Skip to content

Tighten four RFC 9112 rules that Waitress did not enforce - #503

Open
digitalresistor wants to merge 1 commit into
mainfrom
bugfix/rfc9112-strictness
Open

Tighten four RFC 9112 rules that Waitress did not enforce#503
digitalresistor wants to merge 1 commit into
mainfrom
bugfix/rfc9112-strictness

Conversation

@digitalresistor

Copy link
Copy Markdown
Member

Four independent findings from an RFC 9112 audit, all backward incompatible, so the changelog section is retargeted at 4.0.0. Independent of #498 and #502.

Validate the HTTP version, answer 505

RFC 9112 section 2.3 asks a server to answer 505 when the major version is one it does not support, and to process a higher minor version within a major it does implement as the highest minor it is conformant with. Waitress accepted any [0-9].[0-9] and kept it verbatim:

$ printf 'GET / HTTP/9.9\r\nHost: localhost\r\n\r\n' | nc 127.0.0.1 8080
HTTP/1.0 200 OK

This is more than untidiness. Everything version dependent is keyed on the version being exactly 1.0 or 1.1, so an unrecognised one fell through all of it — a request claiming HTTP/2.0 had its Connection and Expect header fields silently ignored. HTTP/1.2 now normalises to 1.1 and honours Connection: close, which it previously did not.

The version-less GET / form that the request-line regex still permits is left alone here; that is a separate decision.

Reject obsolete line folding

RFC 9112 section 5.2 requires a server to either reject an obs-fold with 400 or replace it with SP before interpreting the field value. Waitress did neither, joining the continuation onto the preceding line and keeping the original whitespace:

X-Foo: bar
	Transfer-Encoding: chunked

became HTTP_X_FOO = "bar\tTransfer-Encoding: chunked".

Waitress itself was not desynchronised by this — it never saw a Transfer-Encoding — but it left us reading a field one way while a front end that rejects folding, or unfolds it differently, read it another. Folding has been deprecated since RFC 7230 in 2014.

Drop Connection-named header fields from the environ

RFC 9110 section 7.6.1 makes the field names listed in Connection connection specific: they apply to a single hop and must not be forwarded. Waitress is the end of the connection, so handing them to the application amounts to forwarding them.

Connection: X-Secret
X-Secret: leaked

used to arrive as environ["HTTP_X_SECRET"] == "leaked". A client can name a header there that a proxy in front of Waitress believes only it controls, and the application has no way to tell the two apart.

Skip only empty lines before the request-line

RFC 9112 section 2.2 sanctions ignoring a CRLF received before a request-line, which lets a client send a spare one after a request when pipelining. Waitress used a bare lstrip(), which also ate spaces, tabs, vertical tabs, form feeds and lone CR or LF octets — none of which may appear there, and swallowing them hid them from the checks below.

Test churn

tests/test_parser.py used HTTP/8.4 as its request-line version in 25 places, precisely to show the parser did not care. It does now, so those use a version we support. The five tests that pinned obs-fold being unfolded now pin it being rejected.

Verification

Full default tox envlist green: lint, py39 through py314, pypy39, pypy310, pypy311, coverage at its --fail-under=100 gate, and docs.

Note for review

connection_options() in rfc7230.py is also added by the pending security branch, which ships first. When that lands on main this branch will need a rebase, and the duplicate addition resolves by keeping a single copy.

Validate the HTTP version. RFC 9112 section 2.3 asks a server to answer 505
(HTTP Version Not Supported) when the major version is one it does not
support, and to process a higher minor version within a major it does
implement as the highest minor it is conformant with. Waitress accepted any
[0-9].[0-9] and kept it verbatim. That is more than untidiness: everything
version dependent is keyed on the version being exactly "1.0" or "1.1", so an
unrecognised one fell through all of it and a request claiming HTTP/2.0 had
its Connection and Expect header fields silently ignored.

Reject obsolete line folding. RFC 9112 section 5.2 requires a server to
either reject an obs-fold or replace it with SP before interpreting the field
value; Waitress did neither, joining the continuation onto the line before it
and keeping the original whitespace. That leaves us reading a field one way
while something in front of us that rejects folding, or unfolds differently,
reads it another. Folding has been deprecated since RFC 7230 in 2014.

Drop the header fields named in a request's Connection field before building
the environ. RFC 9110 section 7.6.1 makes them connection specific, applying
to a single hop and not to be forwarded. Waitress is the end of the
connection, so handing them to the application amounts to forwarding: a
client can name a header there that a proxy in front of us believes only it
controls, and the application cannot tell the two apart.

Skip only empty lines before the request-line. RFC 9112 section 2.2 sanctions
ignoring a CRLF there, which lets a client send a spare one after a request
when pipelining. A bare lstrip() also ate spaces, tabs, vertical tabs, form
feeds and lone CR or LF octets, none of which may appear before a
request-line, and swallowing them hid them from the checks below.

The request-line version in tests/test_parser.py was HTTP/8.4 in 25 places,
chosen to show the parser did not care about it. It does now, so those use a
version we support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants