NIOHTTP2: reject SP and control characters in pseudo-header values (#555)

Stacks on #554 — the diff will show that commit too until it merges.

## Motivation

`isValidPseudoHeaderValue` rejects CR, LF and NUL, which is narrower
than the HTTP/1.1 request line actually needs.

`:path` ends up as the request-target in:

```
METHOD SP request-target SP HTTP-version CRLF
```

so `:path = "/a HTTP/1.1"` serializes to `GET /a HTTP/1.1 HTTP/1.1`.
Whether a recipient reads the first or the last SP-delimited token as
the version decides what the request-target actually is, which is the
usual parser-differential setup. RFC 9112 § 3.2 wants SP in a
request-target percent-encoded, and bare HTAB and the other CTLs aren't
valid there either.

None of the HTTP/2 pseudo-headers have a grammar that allows SP or a
CTL: `:method` and `:protocol` are tokens, `:scheme` is a URI scheme,
`:authority` is host[:port], `:path` is a request-target, `:status` is
3DIGIT. So rejecting them doesn't reject anything well-formed.

Same severity caveat as #554: NIOHTTP1's
`uriOnlyContainsAllowedCharacters` already rejects SP in a
request-target by default, so this is hardening rather than something
exploitable on a stock pipeline.

## Modifications

`isValidPseudoHeaderValue` goes from `{0x00, 0x0A, 0x0D}` to `<= 0x20 ||
== 0x7F`, so the whole CTL range plus SP and DEL.

Bytes >= 0x80 stay allowed on purpose. They aren't delimiters in a
request line or header block, and rejecting them would break anyone
sending unencoded UTF-8 in `:path`.

### One behaviour change worth flagging

`validateRequestBlock` runs on send as well as receive, so a client
passing a URI with a raw space in it will now throw where it didn't
before. I'd argue that's the right outcome, since NIOHTTP1's validator
already rejects that URI by default and this makes the two layers agree,
but it is a behaviour change and it's the bit most likely to need
discussion. That's why I split it out of #554 instead of bundling them.

## Result

Pseudo-header values containing SP or any control character are
rejected.

## Testing

Added to `HTTP2FramePayloadToHTTP1CodecCRLFTests.swift`:

- `:path` with SP (`/a HTTP/1.1`), HTAB, VT, FF, DEL, SOH
- SP in `:authority`, `:method`, `:scheme`
- covered at the predicate, request-validation and server-codec levels

The existing positive cases in that file still pass untouched — `/`,
`/foo/bar`, `/foo?q=1&r=2`, `/foo#fragment`, `*`, `/foo%20bar`, `200` —
which is the check that this doesn't reject anything legitimate.

Built and ran on Linux, Swift 6.1 aarch64. `swift test -Xswiftc
-warnings-as-errors` is clean with no new warnings, the new tests pass,
and `swift format lint --strict Sources/ Tests/` is clean.

---------

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

Author: abhinav9-coder

Sources/NIOHTTP2/HTTP2ToHTTP1Codec.swift

@@ -796,7 +796,28 @@ extension HPACKHeaders {
         // Pseudo-header values must not contain CR, LF, or NUL bytes. These characters could
         // enable HTTP/2-to-HTTP/1.1 request smuggling when the value is placed into an HTTP/1.1
         // message (e.g. :path becomes the request-target).
-        !value.utf8.contains(where: { $0 == 0x0A || $0 == 0x0D || $0 == 0x00 })
+        //
+        // We reject the entire CTL range (0x00-0x1F), SP (0x20), and DEL (0x7F), rather than
+        // only CR/LF/NUL, for two reasons:
+        //
+        // 1. `:path` becomes the HTTP/1.1 request-target, which is serialized into the request
+        //    line `METHOD SP request-target SP HTTP-version CRLF`. A bare SP inside `:path`
+        //    therefore produces an ambiguous request line (e.g. `GET /a HTTP/1.1 HTTP/1.1`),
+        //    which HTTP/1.1 parsers may split differently depending on whether they take the
+        //    first or last SP-delimited token as the version. RFC 9112 § 3.2 requires any SP
+        //    in a request-target to be percent-encoded. Bare HTAB and the other CTLs are
+        //    likewise not permitted in a request-target.
+        //
+        // 2. No pseudo-header defined for HTTP/2 has a grammar admitting SP or a CTL:
+        //    `:method` and `:protocol` are tokens (RFC 9110 § 5.6.2), `:scheme` is a URI scheme
+        //    (RFC 3986 § 3.1), `:authority` is host[:port], `:path` is a request-target, and
+        //    `:status` is 3DIGIT. Rejecting these bytes is therefore conformant and does not
+        //    reject any well-formed value.
+        //
+        // Bytes >= 0x80 are deliberately still permitted: they are not delimiters in an
+        // HTTP/1.1 request line or header block, and rejecting them would break peers that
+        // send unencoded UTF-8 in `:path`.
+        !value.utf8.contains(where: { $0 <= 0x20 || $0 == 0x7F })
     }
 
     /// Whether this is a valid value for a regular (non-pseudo) HTTP/2 header field.

Tests/NIOHTTP2Tests/HTTP2FramePayloadToHTTP1CodecCRLFTests.swift

@@ -315,4 +315,68 @@ struct HTTP2FramePayloadToHTTP1CodecCRLFTests {
     // `appendRegularHeaders(from:)` without inspecting them. Regular field values are policed by
     // the connection state machine, which calls `validateRequestBlock` before the codec ever sees
     // the block, so a codec-only `EmbeddedChannel` has no validation in it to exercise.
+
+    // MARK: - Validation tests: SP and other CTLs in pseudo-header values
+
+    // `:path` becomes the HTTP/1.1 request-target in the request line
+    // `METHOD SP request-target SP HTTP-version CRLF`. A bare SP inside `:path` makes that line
+    // ambiguous (`GET /a HTTP/1.1 HTTP/1.1`), so RFC 9112 § 3.2 requires it to be
+    // percent-encoded. No HTTP/2 pseudo-header has a grammar admitting SP or a CTL.
+    static let pseudoHeaderDelimiterInjections: [PseudoHeaderInjection] = [
+        .init(pseudoHeaderName: ":path", maliciousValue: "/a HTTP/1.1", label: "SP request-line split"),
+        .init(pseudoHeaderName: ":path", maliciousValue: "/a\tb", label: "HTAB"),
+        .init(pseudoHeaderName: ":path", maliciousValue: "/a\u{0B}b", label: "vertical tab"),
+        .init(pseudoHeaderName: ":path", maliciousValue: "/a\u{0C}b", label: "form feed"),
+        .init(pseudoHeaderName: ":path", maliciousValue: "/a\u{7F}b", label: "DEL"),
+        .init(pseudoHeaderName: ":path", maliciousValue: "/a\u{01}b", label: "SOH"),
+        .init(pseudoHeaderName: ":authority", maliciousValue: "example.com evil.com", label: "SP"),
+        .init(pseudoHeaderName: ":method", maliciousValue: "GET /admin HTTP/1.1", label: "SP"),
+        .init(pseudoHeaderName: ":scheme", maliciousValue: "https evil", label: "SP"),
+    ]
+
+    @Test(
+        "pseudo-header values containing SP or CTLs are rejected",
+        arguments: pseudoHeaderDelimiterInjections.map(\.maliciousValue)
+    )
+    func invalidPseudoHeaderDelimiterValue(value: String) {
+        #expect(!HPACKHeaders.isValidPseudoHeaderValue(value))
+    }
+
+    @Test(
+        "Request validation rejects SP and CTLs in pseudo-headers",
+        arguments: pseudoHeaderDelimiterInjections
+    )
+    func requestValidationRejectsDelimiters(injection: PseudoHeaderInjection) {
+        let headers = HPACKHeaders([
+            (":method", injection.pseudoHeaderName == ":method" ? injection.maliciousValue : "GET"),
+            (":path", injection.pseudoHeaderName == ":path" ? injection.maliciousValue : "/"),
+            (":scheme", injection.pseudoHeaderName == ":scheme" ? injection.maliciousValue : "https"),
+            (":authority", injection.pseudoHeaderName == ":authority" ? injection.maliciousValue : "example.com"),
+        ])
+        let error = #expect(throws: NIOHTTP2Errors.InvalidPseudoHeaderValue.self) {
+            try headers.validateRequestBlock(supportsExtendedConnect: false)
+        }
+        #expect(error?.name == injection.pseudoHeaderName && error?.value == injection.maliciousValue)
+    }
+
+    @Test(
+        "Server codec rejects SP and CTLs in pseudo-headers",
+        arguments: pseudoHeaderDelimiterInjections
+    )
+    func serverCodecRejectsDelimiters(injection: PseudoHeaderInjection) throws {
+        let channel = EmbeddedChannel()
+        try channel.pipeline.syncOperations.addHandler(HTTP2FramePayloadToHTTP1ServerCodec())
+
+        let requestHeaders = HPACKHeaders([
+            (":method", injection.pseudoHeaderName == ":method" ? injection.maliciousValue : "GET"),
+            (":path", injection.pseudoHeaderName == ":path" ? injection.maliciousValue : "/"),
+            (":scheme", injection.pseudoHeaderName == ":scheme" ? injection.maliciousValue : "https"),
+            (":authority", injection.pseudoHeaderName == ":authority" ? injection.maliciousValue : "example.com"),
+        ])
+        #expect(throws: NIOHTTP2Errors.InvalidPseudoHeaderValue.self) {
+            try channel.writeInbound(
+                HTTP2Frame.FramePayload.headers(.init(headers: requestHeaders, endStream: true))
+            )
+        }
+    }
 }