-
Notifications
You must be signed in to change notification settings - Fork 835
If the HTTP/2 :authority
contains /
, HAProxy treats the part after /
as part of the path
#2941
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Interesting, thanks for reporting this. Today we're busy with releases but I'll have a look at this next week. |
That indeed matches my observations. IPv6 address validation is tricky but I recommend doing it, if for no other reason than to match the Envoy proxy. A backend proxy not being consistent with what the frontend proxy does can cause problems, such as unexpected Bad Gateway errors being returned for invalid requests. |
Please note, we take great efforts to interact the least possible with the request or response because there is a wide gap between what is documented as "deprecated" in recent standards and what is found in the field. Every single time we strenghthen a protocol element, we get reports about breakage of legacy applications. The HTTP ecosystem is not just the web with browsers, there are many devices out there, from payment terminals to backup systems, passing by arrival displays in train stations. When such applications respect more or less RFC2616 or at least 2068, you're happy. For example I remember that the userinfo part was still used for FTP transactions encapsulated over HTTP in certain finance and documentation systems. The difficulty with a gateway (and particularly a load balancer) is that people insert it in places where old applications are having difficulties scaling or interoperating with the rest of the world, and purposely breaking them because we decide that their exchanges are not legitimate is not well received. So we try hard to focus only on what we use and not as much on what we see. If users want to use haproxy to increase security at the expense of interoperability, that's perfectly feasible and done by many, but via the http-request rules. So in the case above, the real problem is the '/' in |
That makes sense! I suspect that Envoy is generally not used with such legacy applications, which means that it can get away with stricter validation. |
Yes, also being used as a side car generally implies modern clients at least. Also it has more to defend against developer bugs and has a bigger benefit in immediately rejecting the slightest non-compliance to force developers to fix their code before deployment. In our case, the clients are everywhere all over the net and we can't fix them. |
In the case of |
Yeah I agree that it should be OK. |
What about misplaced |
Still thinking about it. I'd hate to spend cpu cycles on these ones for most requests, but we could pass via a slow parser if we process [] as an exception as well. |
I would add |
As discussed here: httpwg/http2-spec#936 #2941 It's important to take care of some special characters in the :authority pseudo header before reassembling a complete URI, because after assembly it's too late (e.g. the '/'). This patch adds a specific function which was checks all such characters and their ranges on an ist, and benefits from modern compilers optimizations that arrange the comparisons into an evaluation tree for faster match. That's the version that gave the most consistent performance across various compilers, though some hand-crafted versions using bitmaps stored in register could be slightly faster but super sensitive to code ordering, suggesting that the results might vary with future compilers. This one takes on average 1.2ns per character at 3 GHz (3.6 cycles per char on avg). The resulting impact on H2 request processing time (small requests) was measured around 0.3%, from 6.60 to 6.618us per request, which is a bit high but remains acceptable given that the test only focused on req rate. The code was made usable both for H2 and H3.
…eassembly As discussed here: httpwg/http2-spec#936 #2941 It's important to take care of some special characters in the :authority pseudo header before reassembling a complete URI, because after assembly it's too late (e.g. the '/'). This patch does this, both for h2 and h3. The impact on H2 was measured in the worst case at 0.3% of the request rate, while the impact on H3 is around 1%, but H3 was about 1% faster than H2 before and is now on par. It may be backported after a period of observation, and in this case it relies on this previous commit: MINOR: http: add a function to validate characters of :authority Thanks to @DemiMarie for reviving this topic in issue #2941 and bringing new potential interesting cases.
Hi @DemiMarie |
Detailed Description of the Problem
If I send an HTTP/2 request to HAProxy with a
:authority
pseudo-header that contains/
, HAProxy prepends the part including and after the first/
to the:path
pseudo-header. If an upstream proxy performs path-based access control but does not validate:authority
, these checks could be bypassed.Expected Behavior
HAProxy should reject the request because
/
is not allowed in:authority
pseudo-headers.Steps to Reproduce the Behavior
:authority: a/b
.Do you have any idea what may have caused this?
HAProxy forms a request URI by string concatenation, then re-parses it to determine the path to send upstream for HTTP/1.x requests. This is only safe if parsing the URI will result in the same path that was used to form the URI, but in this case it does not.
Do you have an idea how to solve the issue?
HAProxy should strictly validate the
:authority
andHost
headers. In particular, it should reject:[::]
form.[
and]
.These rules come from Envoy (https://envoyproxy.io).
What is your configuration?
Output of
haproxy -vv
Last Outputs and Backtraces
Additional Information
This should also reproduce with HTTP/3 requests in the development branch. Envoy Proxy has a readable implementation of the above-mentioned validation logic in https://github.com/envoyproxy/envoy/blob/72fd99b8c3681e413eb40ecaebfb1c3454810763/source/extensions/http/header_validators/envoy_default/header_validator.cc, but this is licensed under Apache 2.0 and is in C++.
The text was updated successfully, but these errors were encountered: