Reject requests that break the RFC 9112 request-target and Host rules

CHANGES.txt

@@ -1,5 +1,43 @@
-Unreleased
-----------
+4.0.0 (unreleased)
+------------------
+
+Backward Incompatibilities
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- An HTTP/1.1 request that does not contain a Host header field is now rejected
+  with a 400 (Bad Request), as required by RFC 9112 section 3.2. A Host header
+  field whose value is not a valid ``uri-host [ ":" port ]`` is rejected the
+  same way, whatever the HTTP version of the request. Note that this means an
+  internationalised domain name has to be punycoded by the client, as it always
+  should have been. See https://github.com/Pylons/waitress/issues/462
+
+- When a request uses the absolute-form of request-target, the authority from
+  the request-target is now used as the value of ``HTTP_HOST`` in the environ,
+  and the Host header field of the request is ignored. RFC 9112 section 3.2.2
+  requires this of an origin server, so that a request such as
+  ``GET http://evil.com/ HTTP/1.1`` with ``Host: victim.com`` can't be
+  interpreted one way by Waitress and another way by anything in front of it.
+  An absolute-form request-target with an empty authority, or with a userinfo
+  subcomponent, is rejected with a 400 (Bad Request). See
+  https://github.com/Pylons/waitress/issues/467
+
+- The request-target of a CONNECT request is now validated as the
+  authority-form that RFC 9112 section 3.2.3 requires it to be, and a CONNECT
+  that targets an empty or invalid port number is rejected with a 400 (Bad
+  Request). Waitress still leaves it to the WSGI application to decide what to
+  do with a CONNECT, this only makes sure the request-target it is handed is
+  well formed. See https://github.com/Pylons/waitress/issues/463
+
+- The two remaining forms of request-target described by RFC 9112 section 3.2
+  are now validated as well. The asterisk-form ``*`` is only accepted for an
+  OPTIONS request, as section 3.2.4 restricts it to a server-wide OPTIONS;
+  sending it with any other method is rejected with a 400 (Bad Request).
+  Anything that is not an absolute-form, an authority-form or the asterisk-form
+  has to be an origin-form, which section 3.2.1 defines as an absolute-path,
+  and a request-target that does not begin with a ``/`` is now rejected the
+  same way rather than reaching the application as a ``PATH_INFO`` that PEP
+  3333 does not allow. A request-target beginning with ``//`` is still treated
+  as a path, as before.
 
 Bugfix
 ~~~~~~
@@ -19,6 +57,19 @@ Bugfix
   https://github.com/Pylons/waitress/pull/475 and
   https://github.com/Pylons/waitress/issues/464
 
+- A request that contains more than one Host header field is now rejected with
+  a 400 (Bad Request), rather than being passed to the WSGI application with
+  the values joined by a comma. See
+  https://github.com/Pylons/waitress/issues/462 and
+  https://github.com/Pylons/waitress/pull/484
+
+- A request that contains more than one Content-Type header field is now
+  rejected with a 400 (Bad Request). Content-Type is a singleton field, and
+  recipients differ in which of the values they pick when it is sent more than
+  once. The same applies to Content-Length. See
+  https://github.com/Pylons/waitress/issues/466 and
+  https://github.com/Pylons/waitress/pull/488
+
 3.0.2 (2024-11-16)
 ------------------
 

src/waitress/parser.py

@@ -24,7 +24,7 @@
 
 from waitress.buffers import OverflowableBuffer
 from waitress.receiver import ChunkedReceiver, FixedStreamReceiver
-from waitress.rfc7230 import HEADER_FIELD_RE, ONLY_DIGIT_RE
+from waitress.rfc7230 import AUTHORITY_FORM_RE, HEADER_FIELD_RE, HOST_RE, ONLY_DIGIT_RE
 from waitress.utilities import (
     BadRequest,
     RequestEntityTooLarge,
@@ -266,13 +266,94 @@ def parse_header(self, header_plus):
         command = command.decode("latin-1")
         self.command = command
         self.version = version
+
+        if command == "CONNECT":
+            # RFC 9112 section 3.2.3: the request-target of a CONNECT is an
+            # authority-form, and a server MUST reject a CONNECT that targets
+            # an empty or invalid port number.
+            #
+            # Waitress leaves it to the WSGI application to decide what to do
+            # with a CONNECT, up to and including implementing a proxy with it,
+            # but the application can only do that safely if the target it is
+            # handed is well formed. This validates the shape of the
+            # request-target, it does not reject the method.
+            #
+            # NB: this has to happen before split_uri() is given a chance to
+            # look at the request-target, as urlsplit() parses the
+            # authority-form "example.com:443" as the scheme "example.com"
+            # followed by the path "443".
+            validate_authority_form(uri)
+
         (
             self.proxy_scheme,
             self.proxy_netloc,
             self.path,
             self.query,
             self.fragment,
         ) = split_uri(uri)
+
+        # NB: a CONNECT is excluded here because its request-target is an
+        # authority-form, which urlsplit() reads as a scheme followed by a path
+        # ("example.com:443" comes back as the scheme "example.com" and the
+        # path "443"). An authority-form is not an absolute-form, and it has
+        # been validated as one above already.
+        if self.proxy_scheme and command != "CONNECT":
+            # This is an absolute-form request-target. RFC 9112 section 3.2.2
+            # says that an origin server MUST ignore the received Host header
+            # field and use the host information from the request-target
+            # instead, so that the two can't disagree about which host the
+            # request was meant for.
+            #
+            # validate_uri_host() rejects a userinfo subcomponent along with
+            # everything else that isn't a valid uri-host, since "@" may not
+            # appear in one. RFC 9110 section 4.2.1 requires that an "http"
+            # URI with an empty host be rejected as invalid, so an authority
+            # is required here rather than merely allowed.
+            if not self.proxy_netloc:
+                raise ParsingError("Empty authority in absolute-form request-target")
+
+            validate_uri_host(self.proxy_netloc, "request-target authority")
+
+            headers["HOST"] = self.proxy_netloc
+
+        elif command != "CONNECT":
+            # Not an absolute-form, and not the authority-form of a CONNECT,
+            # so RFC 9112 section 3.2 leaves only two forms this can be. A
+            # request-target that is neither is malformed and does not name
+            # anything we could route.
+
+            if uri == b"*":
+                # RFC 9112 section 3.2.4: the asterisk-form is only used for a
+                # server-wide OPTIONS request. For any other method it is not
+                # a request-target at all, and passing it through would hand
+                # the application a PATH_INFO of "*" to make sense of.
+
+                if command != "OPTIONS":
+                    raise ParsingError(
+                        "Asterisk-form request-target is only valid for OPTIONS"
+                    )
+            elif not uri.startswith(b"/"):
+                # RFC 9112 section 3.2.1: an origin-form is an absolute-path
+                # optionally followed by a query, and an absolute-path begins
+                # with a "/". PEP 3333 wants the same of PATH_INFO.
+                #
+                # NB: this looks at the request-target as it arrived rather
+                # than at the decoded path, so that a percent encoded "/"
+                # can not stand in for the real one.
+                raise ParsingError("Request-target is not in the origin-form")
+
+        # RFC 9112 section 3.2 requires a 400 (Bad Request) response to any
+        # HTTP/1.1 request that lacks a Host header field, or that has a Host
+        # header field with an invalid field value. Duplicate Host headers are
+        # already rejected as part of SINGLETON_FIELDS above.
+        host = headers.get("HOST")
+
+        if host is None:
+            if version == "1.1":
+                raise ParsingError("HTTP/1.1 request does not contain a Host header")
+        else:
+            validate_uri_host(host, "Host header")
+
         self.url_scheme = self.adj.url_scheme
         connection = headers.get("CONNECTION", "")
 
@@ -370,6 +451,42 @@ def close(self):
             body_rcv.getbuf().close()
 
 
+def validate_uri_host(value, what):
+    """
+    Validate that ``value`` is a "uri-host [ ':' port ]" as required by RFC 7230
+    section 5.4 for the Host header field, and by RFC 3986 section 3.2 for the
+    authority of an absolute-form request-target.
+
+    ``what`` names the thing being validated, for use in the error message.
+    """
+
+    if not HOST_RE.match(value.encode("latin-1")):
+        raise ParsingError(f"Invalid {what}")
+
+
+def validate_authority_form(uri):
+    """
+    Validate that ``uri`` is an "authority-form" request-target as defined by
+    RFC 9112 section 3.2.3, that is a uri-host followed by a non-empty and
+    in-range port number.
+    """
+
+    m = AUTHORITY_FORM_RE.match(uri)
+
+    if m is None:
+        raise ParsingError("Request-target is not in the authority-form")
+
+    host, port = m.group("host", "port")
+
+    if not host:
+        raise ParsingError("Request-target does not contain a host")
+
+    # The regular expression bounds the port to at most 5 digits, so int() is
+    # safe to use on it here
+    if not port or not 0 < int(port) <= 65535:
+        raise ParsingError("Request-target contains an empty or invalid port")
+
+
 def split_uri(uri):
     # urlsplit handles byte input by returning bytes on py3, so
     # scheme, netloc, path, query, and fragment are bytes

src/waitress/rfc7230.py

@@ -62,6 +62,45 @@
     "(?:;(?P<extension>" + CHUNK_EXT_NAME + ")(?:=(?P<value>" + CHUNK_EXT_VAL + "))?)*"
 )
 
+# RFC 3986 Section 3.2.2 "Host", which is what RFC 7230 Section 5.4 uses to
+# define the value of the Host header field, and what RFC 9112 Section 3.2.3
+# uses for the authority-form of a request-target:
+#
+# host          = IP-literal / IPv4address / reg-name
+# IP-literal    = "[" ( IPv6address / IPvFuture  ) "]"
+# IPvFuture     = "v" 1*HEXDIG "." 1*( unreserved / sub-delims / ":" )
+# reg-name      = *( unreserved / pct-encoded / sub-delims )
+# unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
+# sub-delims    = "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "="
+# pct-encoded   = "%" HEXDIG HEXDIG
+#
+# IPv4address is a strict subset of reg-name, so it doesn't need to be matched
+# separately. The contents of an IP-literal are not validated any further than
+# the set of characters that may appear inside of one.
+UNRESERVED = r"[A-Za-z0-9\-._~]"
+SUB_DELIMS = r"[!$&'()*+,;=]"
+PCT_ENCODED = "%" + HEXDIG + HEXDIG
+IPV6_LITERAL = r"\[[A-Fa-f0-9:.]+\]"
+IPVFUTURE_LITERAL = (
+    r"\[[vV]" + HEXDIG + r"+\.(?:" + UNRESERVED + "|" + SUB_DELIMS + "|:)+\\]"
+)
+IP_LITERAL = "(?:" + IPV6_LITERAL + "|" + IPVFUTURE_LITERAL + ")"
+# NB: the alternatives here are disjoint (no character may start more than one
+# of them), so this can't backtrack catastrophically
+REG_NAME = "(?:" + UNRESERVED + "|" + PCT_ENCODED + "|" + SUB_DELIMS + ")*"
+URI_HOST = "(?:" + IP_LITERAL + "|" + REG_NAME + ")"
+
+# RFC 7230 Section 5.4: Host = uri-host [ ":" port ], where port = *DIGIT.
+# Both the uri-host and the port may be empty as far as the grammar goes; the
+# callers apply the stricter rules that their context requires.
+HOST_RE = re.compile(("^" + URI_HOST + "(?::" + DIGIT + "*)?$").encode("latin-1"))
+
+# RFC 9112 Section 3.2.3: authority-form = uri-host ":" port. It is used only
+# for CONNECT, where the port is required to be present and valid.
+AUTHORITY_FORM_RE = re.compile(
+    ("^(?P<host>" + URI_HOST + "):(?P<port>" + DIGIT + "{0,5})$").encode("latin-1")
+)
+
 # Pre-compiled regular expressions for use elsewhere
 ONLY_HEXDIG_RE = re.compile(("^" + HEXDIG + "+$").encode("latin-1"))
 ONLY_DIGIT_RE = re.compile(("^" + DIGIT + "+$").encode("latin-1"))