NIOHTTP2: forbid keep-alive and upgrade connection-specific headers (#552)

RFC 9113 § 8.2.2 enumerates five connection-specific header fields that
a conformant HTTP/2 endpoint must treat as malformed: Connection,
Proxy-Connection, Keep-Alive, Transfer-Encoding, and Upgrade.
HeaderFieldName .legalHeaderField(value:) only rejected three of them,
so keep-alive and upgrade passed validation and were translated verbatim
into HTTP/1.1 messages by the H2->H1 codecs. This is a conformance gap
and a defence-in-depth issue, since downstream HTTP/1.1 parsers may act
on those fields independently.

Modifications:

- Add keep-alive and upgrade to the forbidden connection-specific header
switch in legalHeaderField(value:).
- Update the surrounding comment to cite RFC 9113 § 8.2.2 and its
explicit enumeration in place of the obsoleted RFC 7540 § 8.1.2.2
reference.
- Add request- and response-block validation tests covering all five
connection-specific fields.

Result:

All five connection-specific header fields named by RFC 9113 § 8.2.2 are
rejected during header-block validation.

Resolves #549

Co-authored-by: George Barnett <gbarnett@apple.com>

Author: sour-exploit

Sources/NIOHTTP2/HPACKHeaders+Validation.swift

@@ -273,9 +273,10 @@ extension HeaderFieldName {
     }
 
     func legalHeaderField(value: String) throws {
-        // RFC 7540 § 8.1.2.2 forbids all connection-specific header fields. A connection-specific header field technically
-        // is one that is listed in the Connection header, but could also be proxy-connection & transfer-encoding, even though
-        // those are not usually listed in the Connection header. For defensiveness sake, we forbid those too.
+        // RFC 9113 § 8.2.2 (which obsoletes RFC 7540 § 8.1.2.2) forbids all connection-specific header fields, and
+        // enumerates them explicitly: "Connection, Proxy-Connection, Keep-Alive, Transfer-Encoding, and Upgrade". A
+        // connection-specific header field technically is one that is listed in the Connection header, but the fields
+        // named above could also appear independently, so we forbid them all by name.
         //
         // There is one more wrinkle, which is that the client is allowed to send TE: trailers, and forbidden from sending TE
         // with anything else. We police that separately, as TE is only defined on requests, so we can avoid checking for it
@@ -288,7 +289,7 @@ extension HeaderFieldName {
         }
 
         switch self.baseName {
-        case "connection", "transfer-encoding", "proxy-connection":
+        case "connection", "transfer-encoding", "proxy-connection", "keep-alive", "upgrade":
             throw NIOHTTP2Errors.forbiddenHeaderField(name: String(self.baseName), value: value)
         default:
             return

Tests/NIOHTTP2Tests/HTTP2FramePayloadToHTTP1CodecCRLFTests.swift

@@ -107,6 +107,47 @@ struct HTTP2FramePayloadToHTTP1CodecCRLFTests {
         }
     }
 
+    // MARK: - Validation tests: connection-specific headers are rejected (RFC 9113 § 8.2.2)
+
+    // The five connection-specific header fields that a conformant HTTP/2 endpoint must treat as malformed.
+    // `te` is policed separately (it is permitted with the single value "trailers"), so it is not in this list.
+    static let connectionSpecificHeaders = [
+        "connection", "proxy-connection", "keep-alive", "transfer-encoding", "upgrade",
+    ]
+
+    @Test(
+        "Request validation rejects connection-specific headers",
+        arguments: connectionSpecificHeaders
+    )
+    func requestValidationRejectsConnectionSpecificHeaders(name: String) {
+        let headers = HPACKHeaders([
+            (":method", "GET"),
+            (":path", "/"),
+            (":scheme", "https"),
+            (":authority", "example.com"),
+            (name, "some-value"),
+        ])
+        let error = #expect(throws: NIOHTTP2Errors.ForbiddenHeaderField.self) {
+            try headers.validateRequestBlock(supportsExtendedConnect: false)
+        }
+        #expect(error?.name == name && error?.value == "some-value")
+    }
+
+    @Test(
+        "Response validation rejects connection-specific headers",
+        arguments: connectionSpecificHeaders
+    )
+    func responseValidationRejectsConnectionSpecificHeaders(name: String) {
+        let headers = HPACKHeaders([
+            (":status", "200"),
+            (name, "some-value"),
+        ])
+        let error = #expect(throws: NIOHTTP2Errors.ForbiddenHeaderField.self) {
+            try headers.validateResponseBlock()
+        }
+        #expect(error?.name == name && error?.value == "some-value")
+    }
+
     // MARK: - Server codec tests: rejects control characters in pseudo-headers
 
     @Test(