> No, because the chunks are removed on the input and recomposed on the
> output (possibly of different sizes) and chunk extensions are entirely
> lost. I was initially confused by Rajat's report because I didn't see
> where the chunk-ext could have been stored to pass it on the other
> side, and Christopher reminded me that I was just stupid, we indeed
> entirely drop them.
I agree with every word of this paragraph except the first one (and
the part where you call yourself stupid) :)
The fact that haproxy removes chunk-exts before forwarding only
ensures that the request is not misinterpreted *after* haproxy
forwards it. But what if the request was *already* misinterpreted when
haproxy received the request?
To be clear, the trailer-injection poc I gave in the last email was
not hypothetical; I tested it against the latest versions of both
pound and haproxy. Does pound have the bigger bug? Certainly. But the
poc only works because of *both* pound's behavior *and* haproxy's
behavior, each of which I am arguing is not permitted by the RFCs. If
haproxy rejected messages with improperly quoted chunk-exts, the pound
bug would not be exploitable. This same payload can't be used against
a `client<->pound<->node.js` server chain, because node.js properly
rejects malformed chunk-exts.
> IMHO it's totally useless. At best we could reject such cases, but
> for no benefit except annoying owners of broken components (i.e. a
> non-negligible portion of the net). Since here it's totally harmless,
> better continue to ignore the extension like the spec suggests.
> No for the reason that we're doing what the spec suggests (like nginx
> apparently).
> Yes exactly. Like any agent that will stop at the semi-colon like permitted
> by the spec ("MUST ignore unknown extensions", and when you neither know nor
> support any extension, you just ignore all of them).
This raises an important question to which I think we disagree on the answer:
What is meant by the word "ignore" in that sentence in the RFC?
I believe that you are of the opinion that "ignore" means "do not look
at," but I think "ignore" in this context instead means "do not
interpret (but still validate)."
I think my interpretation is supported by the rest of the document.
Take, for example, RFC 9110 Section 5.1:
> A proxy MUST forward unrecognized header fields unless the field name is
> listed in the Connection header field (Section 7.6.1) or the proxy is
> specifically configured to block, or otherwise transform, such fields.
> Other recipients SHOULD ignore unrecognized header and trailer fields.
> Adhering to these requirements allows HTTP's functionality to be extended
> without updating or removing deployed intermediaries.
If "ignore" meant "do not look at," then this would seem to suggest
that endpoint servers are not responsible for validating header values
from unrecognized headers. That would mean that, for example, it would
be valid to accept CR, LF, and NUL within header-values of
unrecognized headers. To me, this seems incorrect.
I argue that the word "ignore" in this context instead means the
recipient should ensure that each received header is well-formed, but
well-formed unexpected headers should have no effect. That is, the
result of parsing the validated header field should be ignored.
By my interpretation, this makes haproxy's behavior a (minor)
violation of the spec. Will it cause a problem? Not unless haproxy's
input is coming from pound or something else equally ill-behaved. But
if it did cause a problem, whose fault would it be? I'd argue taht
most of the blame would go to pound, but haproxy is not totally
blameless.
> I disagree with this argument. We need to implement what we *use*. It's
> even particularly dangerous to implement what you don't use because 1)
> you can't test it, and 2) it's often done in too lenient a way to the
> point that issues come from there. How many components have had issues
> with trailers while they are unable to make use of them for example ?
I disagree that you can't test this effectively; the tests need only
show that requests containing well-formed chunk-extensions are emitted
with the chunk-extensions properly removed, and requests containing
malformed chunk-extensions are rejected.
> I'm definitely not worried about the performance cost here, just
> adding more crap to parse something that will not be maintained over
> time,
This makes sense.
> ...that nobody will know how to test and that the spec clearly says
> we must not even look at. Note that I have alreay written the code for
> this (posted earlier in this thread), it's just that it's out of our
> business since we're not supposed to read such extensions.
Again, I don't agree that the spec clearly says chunk-exts should not
be looked at during message validation, just that they should not
affect message interpretation.
> But as you pointed this one is not valid as per the RFC. We could always
> say that "what if..." but the we cannot prevent lenient parsers from
> shooting themselves in the foot.
I've heard this argument many times, but I don't think it's entirely
fair. This basically reduces to "haproxy's spec violation wouldn't be
exploitable if Pound hadn't violated the spec first," and of course
the inverse could be claimed by pound. When an exploit is achievable
only due to multiple bugs in multiple programs, both programs are at
fault (though maybe not equally).
> Maybe time to report it to them ;-) Because it will do the same with
> about every other component since basically noone implements chunk-ext
> in practice.
Already did :)
