Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate that connection-specific headers aren't sent. #95

Merged
merged 1 commit into from
Apr 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 42 additions & 14 deletions Sources/NIOHTTP2/HPACKHeaders+Validation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ extension HeaderBlockValidator {
for (name, value, _) in block {
let fieldName = try HeaderFieldName(name)
try blockSection.validField(fieldName)
try fieldName.legalHeaderField(value: value)

let thisPseudoHeaderFieldType = try seenPseudoHeaders.seenNewHeaderField(fieldName)

Expand Down Expand Up @@ -142,22 +143,28 @@ extension RequestBlockValidator: HeaderBlockValidator {
// This is a bit awkward.
//
// For now we don't support extended-CONNECT, but when we do we'll need to update the logic here.
guard let pseudoHeaderType = pseudoHeaderType else {
// Nothing to do here.
return
}
if let pseudoHeaderType = pseudoHeaderType {
assert(name.fieldType == .pseudoHeaderField)

switch pseudoHeaderType {
case .method:
// This is a method pseudo-header. Check if the value is CONNECT.
self.isConnectRequest = value == "CONNECT"
case .path:
// This is a path pseudo-header. It must not be empty.
if value.utf8.count == 0 {
throw NIOHTTP2Errors.EmptyPathHeader()
}
default:
break
}
} else {
assert(name.fieldType == .regularHeaderField)

switch pseudoHeaderType {
case .method:
// This is a method pseudo-header. Check if the value is CONNECT.
self.isConnectRequest = value == "CONNECT"
case .path:
// This is a path pseudo-header. It must not be empty.
if value.utf8.count == 0 {
throw NIOHTTP2Errors.EmptyPathHeader()
// We want to check that if the TE header field is present, it only contains "trailers".
if name.baseName == "te" && value != "trailers" {
throw NIOHTTP2Errors.ForbiddenHeaderField(name: String(name.baseName), value: value)
}
default:
break
}
}

Expand Down Expand Up @@ -249,6 +256,27 @@ extension HeaderFieldName {
throw NIOHTTP2Errors.InvalidHTTP2HeaderFieldName(fieldName)
}
}

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.
//
// 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
// on responses and trailers.
guard self.fieldType == .regularHeaderField else {
// Pseudo-headers are never connection-specific.
return
}

switch self.baseName {
case "connection", "transfer-encoding", "proxy-connection":
throw NIOHTTP2Errors.ForbiddenHeaderField(name: String(self.baseName), value: value)
default:
return
}
}
}


Expand Down
12 changes: 12 additions & 0 deletions Sources/NIOHTTP2/HTTP2Error.swift
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,18 @@ public enum NIOHTTP2Errors {
self.fieldName = fieldName
}
}

/// Connection-specific header fields are forbidden in HTTP/2: this error is raised when one is
/// sent or received.
public struct ForbiddenHeaderField: NIOHTTP2Error {
public var name: String
public var value: String

public init(name: String, value: String) {
self.name = name
self.value = value
}
}
}


Expand Down
9 changes: 9 additions & 0 deletions Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,15 @@ extension ConnectionStateMachineTests {
("testAllowSimpleConnectRequest", testAllowSimpleConnectRequest),
("testRejectRequestHeadersWithoutAuthorityFieldOnCONNECT", testRejectRequestHeadersWithoutAuthorityFieldOnCONNECT),
("testAllowRequestHeadersWithoutAuthorityFieldOnCONNECTValidationDisabled", testAllowRequestHeadersWithoutAuthorityFieldOnCONNECTValidationDisabled),
("testRejectHeadersWithConnectionHeader", testRejectHeadersWithConnectionHeader),
("testAllowHeadersWithConnectionHeaderWhenValidationDisabled", testAllowHeadersWithConnectionHeaderWhenValidationDisabled),
("testRejectHeadersWithTransferEncodingHeader", testRejectHeadersWithTransferEncodingHeader),
("testAllowHeadersWithTransferEncodingHeaderWhenValidationDisabled", testAllowHeadersWithTransferEncodingHeaderWhenValidationDisabled),
("testRejectHeadersWithProxyConnectionHeader", testRejectHeadersWithProxyConnectionHeader),
("testAllowHeadersWithProxyConnectionHeaderWhenValidationDisabled", testAllowHeadersWithProxyConnectionHeaderWhenValidationDisabled),
("testRejectHeadersWithTEHeaderNotTrailers", testRejectHeadersWithTEHeaderNotTrailers),
("testAllowHeadersWithTEHeaderNotTrailersWhenValidationDisabled", testAllowHeadersWithTEHeaderNotTrailersWhenValidationDisabled),
("testAllowHeadersWithTEHeaderSetToTrailers", testAllowHeadersWithTEHeaderSetToTrailers),
]
}
}
Expand Down
Loading