Acting on incomplete headers

[mnot]

From #416.

Browsers seem to act upon partial response header blocks, which might introduce security issues. In particular, some will follow a redirect without getting the final separating \n, either upon a timeout or connection close.

For example, given:

printf "HTTP/1.1 301 Redirect\nLocation: https://evil.com/\n" | nc -l -p 8002 -c

Firefox 51 and Chrome 55 will redirect to evil.com upon connection close (-c).

Safari 10 will also navigate to evil.com upon connection close, although there appears to be a timer or race condition; you have to omit -c and manually kill nc after the request is sent.

HTTP talks about this here:

If a response terminates in the middle of the header section (before the empty line is received) and the status code might rely on header fields to convey the full meaning of the response, then the client cannot assume that meaning has been conveyed; the client might need to repeat the request in order to determine what action to take next.

cc @mcmanus

[annevk]

https://wpt.fyi/results/fetch/h1-parsing/status-code.window.html also confirms this as my test code was rather sloppy:

def main(request, response):
    output = b"HTTP/1.1 "
    output += request.GET.first(b"input")
    output += b"\nheader-parsing: is sad\n"
    response.writer.write(output)
    response.close_connection = True

All browsers were able to find the header header-parsing in the response, despite the lack of two newlines after it (let's ignore CRLF vs lone LFs here).

@whatwg/security @whatwg/http is this something we think could be fixed or should we enshrine this as a HTTP/1 parsing quirk in #1156?

(@njsmith this and #1156 might be of interest to you as well given your comments in httpwg/http-core.)

[annevk]

I guess there is at least one thing here to act on which is what should happen when you remove the newline after "sad" (i.e., no newlines at all after headers). At that point Firefox starts acting funny and is seemingly non-deterministic, Safari network errors the response, and Chrome keeps trucking. I created tests that expect a network error for this, as Safari already does, and filed bugs:

• HTTP/1 parsing of responses without newlines after headers web-platform-tests/wpt#27456
• https://bugzilla.mozilla.org/show_bug.cgi?id=1690481, follow-up: https://bugzilla.mozilla.org/show_bug.cgi?id=1694361
• https://bugs.chromium.org/p/chromium/issues/detail?id=1173975

Review welcome!

[MattMenke2]

Chrome rejects incomplete headers over HTTPS, as an attack mitigation, but allows them over HTTP. That change resulted in very few bug reports. While the increased prevalence of HTTPS hopefully means the fallout of doing this over HTTP as well wouldn't be too bad, I have my doubts, since I suspect home-brew HTTP servers are much more likely to have this issue, and much less likely to be using HTTPS.

I investigated removing a bunch of HTTP hacks years ago, but ran into issues with the first mitigation, which I believe I chose because it had the lowest use (Removing HTTP/0.9 support over ports other than 80, which is a potential security issue - may have chosen it for the security implications, actually), and gave up. I gathered stats at the time, but don't think we still have them. We may be able to bring back the logging code without too much difficulty, not sure.

[annevk]

To be clear, there are two different cases here, ignoring HTTPS for the moment:

1. Header block ends with LF.
2. Header block does not end with LF.

At the moment I'm only suggesting changes for 2, based on the results in Safari. I guess I should write some HTTPS tests as well though.

[annevk]

It was pointed out to me that

• https://github.com/web-platform-tests/wpt/blob/master/fetch/h1-parsing/lone-cr.window.js
• https://github.com/web-platform-tests/wpt/blob/master/html/cross-origin-embedder-policy/header-parsing.https.html#L46

have conflicting expectations. The latter wants a lone CR to be treated as whitespace.

[MattMenke2]

That's unfortunate. Would be nice if we could align on lone-cr.window.js, but that would potentially cause a lot of breakage.

Looks like Safari is passing a fair number of the tests, mostly with CRs after headers but before the body (https://wpt.fyi/results/fetch/h1-parsing/lone-cr.window.html?label=master&label=experimental&aligned).