Skip to content

Commit

Permalink
Merge pull request from GHSA-7fj7-39wj-c64f
Browse files Browse the repository at this point in the history
Motivation

HTTP headers are prevented from containing certain characters that can
potentially affect parsing or interpretation. Inadequately policing this
can lead to vulnerabilities in web applications, most notably HTTP
Response Splitting.

NIO was insufficiently policing the correctness of the header fields we
emit in HTTP/1.1. We've therefore added a new handler that is
automatically added to channel pipelines that will police the validity
of header fields.

For projects that are already running the validation themselves, this
can be easily disabled. Note that by default NIO does not validate
content length is correctly calculated, so applications can have their
framing fall out of sync unless they appropriately calculate this
themselves or use chunked transfer encoding.

Modifications

- Add thorough unit testing to confirm we will not emit invalid header
  fields.
- Error if a user attempts to send an invalid header field.

Result

NIO applications are no longer vulnerable to response splitting by CRLF
injection by default.
  • Loading branch information
Lukasa authored Sep 27, 2022
1 parent 5ce13ea commit a16e2f5
Show file tree
Hide file tree
Showing 13 changed files with 1,345 additions and 26 deletions.
5 changes: 4 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ var targets: [PackageDescription.Target] = [
.executableTarget(name: "NIOHTTP1Client",
dependencies: ["NIOPosix", "NIOCore", "NIOHTTP1", "NIOConcurrencyHelpers"],
exclude: ["README.md"]),
.target(name: "CNIOLLHTTP"),
.target(
name: "CNIOLLHTTP",
cSettings: [.define("LLHTTP_STRICT_MODE")]
),
.target(name: "NIOTLS", dependencies: ["NIO", "NIOCore"]),
.executableTarget(name: "NIOChatServer",
dependencies: ["NIOPosix", "NIOCore", "NIOConcurrencyHelpers"],
Expand Down
5 changes: 4 additions & 1 deletion Package@swift-5.4.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ var targets: [PackageDescription.Target] = [
.executableTarget(name: "NIOHTTP1Client",
dependencies: ["NIOPosix", "NIOCore", "NIOHTTP1", "NIOConcurrencyHelpers"],
exclude: ["README.md"]),
.target(name: "CNIOLLHTTP"),
.target(
name: "CNIOLLHTTP",
cSettings: [.define("LLHTTP_STRICT_MODE")]
),
.target(name: "NIOTLS", dependencies: ["NIO", "NIOCore"]),
.executableTarget(name: "NIOChatServer",
dependencies: ["NIOPosix", "NIOCore", "NIOConcurrencyHelpers"],
Expand Down
5 changes: 4 additions & 1 deletion Package@swift-5.5.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ var targets: [PackageDescription.Target] = [
.executableTarget(name: "NIOHTTP1Client",
dependencies: ["NIOPosix", "NIOCore", "NIOHTTP1", "NIOConcurrencyHelpers"],
exclude: ["README.md"]),
.target(name: "CNIOLLHTTP"),
.target(
name: "CNIOLLHTTP",
cSettings: [.define("LLHTTP_STRICT_MODE")]
),
.target(name: "NIOTLS", dependencies: ["NIO", "NIOCore"]),
.executableTarget(name: "NIOChatServer",
dependencies: ["NIOPosix", "NIOCore", "NIOConcurrencyHelpers"],
Expand Down
97 changes: 97 additions & 0 deletions Sources/NIOHTTP1/HTTPHeaderValidator.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2022 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import NIOCore

/// A ChannelHandler to validate that outbound request headers are spec-compliant.
///
/// The HTTP RFCs constrain the bytes that are validly present within a HTTP/1.1 header block.
/// ``NIOHTTPRequestHeadersValidator`` polices this constraint and ensures that only valid header blocks
/// are emitted on the network. If a header block is invalid, then ``NIOHTTPRequestHeadersValidator``
/// will send a ``HTTPParserError/invalidHeaderToken``.
///
/// ``NIOHTTPRequestHeadersValidator`` will also valid that the HTTP trailers are within specification,
/// if they are present.
public final class NIOHTTPRequestHeadersValidator: ChannelOutboundHandler, RemovableChannelHandler {
public typealias OutboundIn = HTTPClientRequestPart
public typealias OutboundOut = HTTPClientRequestPart

public init() { }

public func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
switch self.unwrapOutboundIn(data) {
case .head(let head):
guard head.headers.areValidToSend else {
promise?.fail(HTTPParserError.invalidHeaderToken)
context.fireErrorCaught(HTTPParserError.invalidHeaderToken)
return
}
case .body, .end(.none):
()
case .end(.some(let trailers)):
guard trailers.areValidToSend else {
promise?.fail(HTTPParserError.invalidHeaderToken)
context.fireErrorCaught(HTTPParserError.invalidHeaderToken)
return
}
}

context.write(data, promise: promise)
}
}


/// A ChannelHandler to validate that outbound response headers are spec-compliant.
///
/// The HTTP RFCs constrain the bytes that are validly present within a HTTP/1.1 header block.
/// ``NIOHTTPResponseHeadersValidator`` polices this constraint and ensures that only valid header blocks
/// are emitted on the network. If a header block is invalid, then ``NIOHTTPResponseHeadersValidator``
/// will send a ``HTTPParserError/invalidHeaderToken``.
///
/// ``NIOHTTPResponseHeadersValidator`` will also valid that the HTTP trailers are within specification,
/// if they are present.
public final class NIOHTTPResponseHeadersValidator: ChannelOutboundHandler, RemovableChannelHandler {
public typealias OutboundIn = HTTPServerResponsePart
public typealias OutboundOut = HTTPServerResponsePart

public init() { }

public func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
switch self.unwrapOutboundIn(data) {
case .head(let head):
guard head.headers.areValidToSend else {
promise?.fail(HTTPParserError.invalidHeaderToken)
context.fireErrorCaught(HTTPParserError.invalidHeaderToken)
return
}
case .body, .end(.none):
()
case .end(.some(let trailers)):
guard trailers.areValidToSend else {
promise?.fail(HTTPParserError.invalidHeaderToken)
context.fireErrorCaught(HTTPParserError.invalidHeaderToken)
return
}
}

context.write(data, promise: promise)
}
}

#if swift(>=5.6)
@available(*, unavailable)
extension NIOHTTPRequestHeadersValidator: Sendable {}

@available(*, unavailable)
extension NIOHTTPResponseHeadersValidator: Sendable {}
#endif
205 changes: 205 additions & 0 deletions Sources/NIOHTTP1/HTTPHeaders+Validation.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2022 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

extension String {
/// Validates a given header field value against the definition in RFC 9110.
///
/// The spec in [RFC 9110](https://httpwg.org/specs/rfc9110.html#fields.values) defines the valid
/// characters as the following:
///
/// ```
/// field-value = *field-content
/// field-content = field-vchar
/// [ 1*( SP / HTAB / field-vchar ) field-vchar ]
/// field-vchar = VCHAR / obs-text
/// obs-text = %x80-FF
/// ```
///
/// Additionally, it makes the following note:
///
/// "Field values containing CR, LF, or NUL characters are invalid and dangerous, due to the
/// varying ways that implementations might parse and interpret those characters; a recipient
/// of CR, LF, or NUL within a field value MUST either reject the message or replace each of
/// those characters with SP before further processing or forwarding of that message. Field
/// values containing other CTL characters are also invalid; however, recipients MAY retain
/// such characters for the sake of robustness when they appear within a safe context (e.g.,
/// an application-specific quoted string that will not be processed by any downstream HTTP
/// parser)."
///
/// As we cannot guarantee the context is safe, this code will reject all ASCII control characters
/// directly _except_ for HTAB, which is explicitly allowed.
fileprivate var isValidHeaderFieldValue: Bool {
let fastResult = self.utf8.withContiguousStorageIfAvailable { ptr in
ptr.allSatisfy { $0.isValidHeaderFieldValueByte }
}
if let fastResult = fastResult {
return fastResult
} else {
return self.utf8._isValidHeaderFieldValue_slowPath
}
}

/// Validates a given header field name against the definition in RFC 9110.
///
/// The spec in [RFC 9110](https://httpwg.org/specs/rfc9110.html#fields.values) defines the valid
/// characters as the following:
///
/// ```
/// field-name = token
///
/// token = 1*tchar
///
/// tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*"
/// / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
/// / DIGIT / ALPHA
/// ; any VCHAR, except delimiters
/// ```
///
/// We implement this check directly.
fileprivate var isValidHeaderFieldName: Bool {
let fastResult = self.utf8.withContiguousStorageIfAvailable { ptr in
ptr.allSatisfy { $0.isValidHeaderFieldNameByte }
}
if let fastResult = fastResult {
return fastResult
} else {
return self.utf8._isValidHeaderFieldName_slowPath
}
}
}

extension String.UTF8View {
/// The equivalent of `String.isValidHeaderFieldName`, but a slow-path when a
/// contiguous UTF8 storage is not available.
///
/// This is deliberately `@inline(never)`, as slow paths should be forcibly outlined
/// to encourage inlining the fast-path.
@inline(never)
fileprivate var _isValidHeaderFieldName_slowPath: Bool {
self.allSatisfy { $0.isValidHeaderFieldNameByte }
}

/// The equivalent of `String.isValidHeaderFieldValue`, but a slow-path when a
/// contiguous UTF8 storage is not available.
///
/// This is deliberately `@inline(never)`, as slow paths should be forcibly outlined
/// to encourage inlining the fast-path.
@inline(never)
fileprivate var _isValidHeaderFieldValue_slowPath: Bool {
self.allSatisfy { $0.isValidHeaderFieldValueByte }
}
}

extension UInt8 {
/// Validates a byte of a given header field name against the definition in RFC 9110.
///
/// The spec in [RFC 9110](https://httpwg.org/specs/rfc9110.html#fields.values) defines the valid
/// characters as the following:
///
/// ```
/// field-name = token
///
/// token = 1*tchar
///
/// tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*"
/// / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
/// / DIGIT / ALPHA
/// ; any VCHAR, except delimiters
/// ```
///
/// We implement this check directly.
///
/// We use inline always here to force the check to be inlined, which it isn't always, leading to less optimal code.
@inline(__always)
fileprivate var isValidHeaderFieldNameByte: Bool {
switch self {
case UInt8(ascii: "0")...UInt8(ascii: "9"), // DIGIT
UInt8(ascii: "a")...UInt8(ascii: "z"),
UInt8(ascii: "A")...UInt8(ascii: "Z"), // ALPHA
UInt8(ascii: "!"), UInt8(ascii: "#"),
UInt8(ascii: "$"), UInt8(ascii: "%"),
UInt8(ascii: "&"), UInt8(ascii: "'"),
UInt8(ascii: "*"), UInt8(ascii: "+"),
UInt8(ascii: "-"), UInt8(ascii: "."),
UInt8(ascii: "^"), UInt8(ascii: "_"),
UInt8(ascii: "`"), UInt8(ascii: "|"),
UInt8(ascii: "~"):

return true

default:
return false
}
}

/// Validates a byte of a given header field value against the definition in RFC 9110.
///
/// The spec in [RFC 9110](https://httpwg.org/specs/rfc9110.html#fields.values) defines the valid
/// characters as the following:
///
/// ```
/// field-value = *field-content
/// field-content = field-vchar
/// [ 1*( SP / HTAB / field-vchar ) field-vchar ]
/// field-vchar = VCHAR / obs-text
/// obs-text = %x80-FF
/// ```
///
/// Additionally, it makes the following note:
///
/// "Field values containing CR, LF, or NUL characters are invalid and dangerous, due to the
/// varying ways that implementations might parse and interpret those characters; a recipient
/// of CR, LF, or NUL within a field value MUST either reject the message or replace each of
/// those characters with SP before further processing or forwarding of that message. Field
/// values containing other CTL characters are also invalid; however, recipients MAY retain
/// such characters for the sake of robustness when they appear within a safe context (e.g.,
/// an application-specific quoted string that will not be processed by any downstream HTTP
/// parser)."
///
/// As we cannot guarantee the context is safe, this code will reject all ASCII control characters
/// directly _except_ for HTAB, which is explicitly allowed.
///
/// We use inline always here to force the check to be inlined, which it isn't always, leading to less optimal code.
@inline(__always)
fileprivate var isValidHeaderFieldValueByte: Bool {
switch self {
case UInt8(ascii: "\t"):
// HTAB, explicitly allowed.
return true
case 0...0x1f, 0x7F:
// ASCII control character, forbidden.
return false
default:
// Printable or non-ASCII, allowed.
return true
}
}
}

extension HTTPHeaders {
/// Whether these HTTPHeaders are valid to send on the wire.
var areValidToSend: Bool {
for (name, value) in self.headers {
if !name.isValidHeaderFieldName {
return false
}

if !value.isValidHeaderFieldValue {
return false
}
}

return true
}
}
Loading

0 comments on commit a16e2f5

Please sign in to comment.