From 584c0d06c46233d3e1759695e5eff495b12edcd2 Mon Sep 17 00:00:00 2001 From: Johannes Weiss Date: Mon, 23 Mar 2020 16:26:05 +0000 Subject: [PATCH] universal bootstrap support (#185) --- .gitignore | 1 + Package.swift | 2 +- Sources/NIOSSL/NIOSSLClientHandler.swift | 39 ++++++-- Sources/NIOSSL/SSLErrors.swift | 12 +++ .../NIOSSL/UniversalBootstrapSupport.swift | 68 +++++++++++++ Tests/NIOSSLTests/ClientSNITests+XCTest.swift | 5 + Tests/NIOSSLTests/ClientSNITests.swift | 69 ++++++++++++- Tests/NIOSSLTests/NIOSSLIntegrationTest.swift | 98 +++++++++++++------ scripts/sanity.sh | 4 +- 9 files changed, 253 insertions(+), 45 deletions(-) create mode 100644 Sources/NIOSSL/UniversalBootstrapSupport.swift diff --git a/.gitignore b/.gitignore index bd2d8f8d..3961b9db 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ Package.resolved /docs DerivedData /.idea +.swiftpm diff --git a/Package.swift b/Package.swift index 64881ab7..b804598d 100644 --- a/Package.swift +++ b/Package.swift @@ -35,7 +35,7 @@ let package = Package( MANGLE_END */ ], dependencies: [ - .package(url: "https://github.com/apple/swift-nio.git", from: "2.14.0"), + .package(url: "https://github.com/apple/swift-nio.git", from: "2.15.0"), ], targets: [ .target(name: "CNIOBoringSSL"), diff --git a/Sources/NIOSSL/NIOSSLClientHandler.swift b/Sources/NIOSSL/NIOSSLClientHandler.swift index 1694543f..047026fa 100644 --- a/Sources/NIOSSL/NIOSSLClientHandler.swift +++ b/Sources/NIOSSL/NIOSSLClientHandler.swift @@ -14,8 +14,8 @@ import NIO -private extension String { - func isIPAddress() -> Bool { +extension String { + private func isIPAddress() -> Bool { // We need some scratch space to let inet_pton write into. var ipv4Addr = in_addr() var ipv6Addr = in6_addr() @@ -25,6 +25,21 @@ private extension String { inet_pton(AF_INET6, ptr, &ipv6Addr) == 1 } } + + func validateSNIServerName() throws { + guard !self.isIPAddress() else { + throw NIOSSLExtraError.cannotUseIPAddressInSNI(ipAddress: self) + } + + // no 0 bytes + guard !self.utf8.contains(0) else { + throw NIOSSLExtraError.invalidSNIHostname + } + + guard (1 ... 255).contains(self.utf8.count) else { + throw NIOSSLExtraError.invalidSNIHostname + } + } } /// A channel handler that wraps a channel in TLS using NIOSSL. @@ -43,12 +58,14 @@ public final class NIOSSLClientHandler: NIOSSLHandler { connection.setConnectState() if let serverHostname = serverHostname { - if serverHostname.isIPAddress() { - throw NIOSSLExtraError.cannotUseIPAddressInSNI(ipAddress: serverHostname) - } + try serverHostname.validateSNIServerName() // IP addresses must not be provided in the SNI extension, so filter them. - try connection.setServerName(name: serverHostname) + do { + try connection.setServerName(name: serverHostname) + } catch { + preconditionFailure("Bug in NIOSSL (please report): \(Array(serverHostname.utf8)) passed NIOSSL's hostname test but failed in BoringSSL.") + } } if let verificationCallback = verificationCallback { @@ -71,12 +88,14 @@ public final class NIOSSLClientHandler: NIOSSLHandler { connection.setConnectState() if let serverHostname = serverHostname { - if serverHostname.isIPAddress() { - throw NIOSSLExtraError.cannotUseIPAddressInSNI(ipAddress: serverHostname) - } + try serverHostname.validateSNIServerName() // IP addresses must not be provided in the SNI extension, so filter them. - try connection.setServerName(name: serverHostname) + do { + try connection.setServerName(name: serverHostname) + } catch { + preconditionFailure("Bug in NIOSSL (please report): \(Array(serverHostname.utf8)) passed NIOSSL's hostname test but failed in BoringSSL.") + } } if let verificationCallback = optionalCustomVerificationCallback { diff --git a/Sources/NIOSSL/SSLErrors.swift b/Sources/NIOSSL/SSLErrors.swift index bee86ac8..b0ec5dfc 100644 --- a/Sources/NIOSSL/SSLErrors.swift +++ b/Sources/NIOSSL/SSLErrors.swift @@ -164,6 +164,7 @@ extension NIOSSLExtraError { case failedToValidateHostname case serverHostnameImpossibleToMatch case cannotUseIPAddressInSNI + case invalidSNIHostname } } @@ -178,6 +179,17 @@ extension NIOSSLExtraError { /// IP addresses may not be used in SNI. public static let cannotUseIPAddressInSNI = NIOSSLExtraError(baseError: .cannotUseIPAddressInSNI, description: nil) + /// The SNI hostname requirements have not been met. + /// + /// - note: Should the provided SNI hostname be an IP address instead, `.cannotUseIPAddressInSNI` is thrown instead + /// of this error. + /// + /// Reasons a hostname might not meet the requirements: + /// - hostname in UTF8 is more than 255 bytes + /// - hostname is the empty string + /// - hostname contains the `0` unicode scalar (which would be encoded as the `0` byte which is unsupported). + public static let invalidSNIHostname = NIOSSLExtraError(baseError: .invalidSNIHostname, description: nil) + @inline(never) internal static func failedToValidateHostname(expectedName: String) -> NIOSSLExtraError { let description = "Couldn't find \(expectedName) in certificate from peer" diff --git a/Sources/NIOSSL/UniversalBootstrapSupport.swift b/Sources/NIOSSL/UniversalBootstrapSupport.swift new file mode 100644 index 00000000..f8590767 --- /dev/null +++ b/Sources/NIOSSL/UniversalBootstrapSupport.swift @@ -0,0 +1,68 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the SwiftNIO open source project +// +// Copyright (c) 2020 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 NIO + +/// A TLS provider to bootstrap TLS-enabled connections with `NIOClientTCPBootstrap`. +/// +/// Example: +/// +/// // TLS setup. +/// let configuration = TLSConfiguration.forClient() +/// let sslContext = try NIOSSLContext(configuration: configuration) +/// +/// // Creating the "universal bootstrap" with the `NIOSSLClientTLSProvider`. +/// let tlsProvider = NIOSSLClientTLSProvider(context: sslContext, serverHostname: "example.com") +/// let bootstrap = NIOClientTCPBootstrap(ClientBootstrap(group: group), tls: tlsProvider) +/// +/// // Bootstrapping a connection using the "universal bootstrapping mechanism" +/// let connection = bootstrap.enableTLS() +/// .connect(to: "example.com") +/// .wait() +public struct NIOSSLClientTLSProvider: NIOClientTLSProvider { + public typealias Bootstrap = Bootstrap + + let context: NIOSSLContext + let serverHostname: String? + let customVerificationCallback: NIOSSLCustomVerificationCallback? + + /// Construct the TLS provider with the necessary configuration. + public init(context: NIOSSLContext, + serverHostname: String?, + customVerificationCallback: NIOSSLCustomVerificationCallback? = nil) throws { + try serverHostname.map { + try $0.validateSNIServerName() + } + self.context = context + self.serverHostname = serverHostname + self.customVerificationCallback = customVerificationCallback + } + + /// Enable TLS on the bootstrap. This is not a function you will typically call as a user, it is called by + /// `NIOClientTCPBootstrap`. + public func enableTLS(_ bootstrap: Bootstrap) -> Bootstrap { + // NIOSSLClientHandler.init only throws because of `malloc` error and invalid SNI hostnames. We want to crash + // on malloc error and we pre-checked the SNI hostname in `init` so that should be impossible here. + return bootstrap.protocolHandlers { + if let customVerificationCallback = self.customVerificationCallback { + return [try! NIOSSLClientHandler(context: self.context, + serverHostname: self.serverHostname, + customVerificationCallback: customVerificationCallback)] + } else { + return [try! NIOSSLClientHandler(context: self.context, + serverHostname: self.serverHostname)] + } + } + } +} diff --git a/Tests/NIOSSLTests/ClientSNITests+XCTest.swift b/Tests/NIOSSLTests/ClientSNITests+XCTest.swift index d997d804..6707dc81 100644 --- a/Tests/NIOSSLTests/ClientSNITests+XCTest.swift +++ b/Tests/NIOSSLTests/ClientSNITests+XCTest.swift @@ -31,6 +31,11 @@ extension ClientSNITests { ("testNoSNILeadsToNoExtension", testNoSNILeadsToNoExtension), ("testSNIIsRejectedForIPv4Addresses", testSNIIsRejectedForIPv4Addresses), ("testSNIIsRejectedForIPv6Addresses", testSNIIsRejectedForIPv6Addresses), + ("testSNIIsRejectedForEmptyHostname", testSNIIsRejectedForEmptyHostname), + ("testSNIIsRejectedForTooLongHostname", testSNIIsRejectedForTooLongHostname), + ("testSNIIsRejectedFor0Byte", testSNIIsRejectedFor0Byte), + ("testSNIIsNotRejectedForAnyOfTheFirst1000CodeUnits", testSNIIsNotRejectedForAnyOfTheFirst1000CodeUnits), + ("testSNIIsNotRejectedForVeryWeirdCharacters", testSNIIsNotRejectedForVeryWeirdCharacters), ] } } diff --git a/Tests/NIOSSLTests/ClientSNITests.swift b/Tests/NIOSSLTests/ClientSNITests.swift index 3b2550b7..c0e05a77 100644 --- a/Tests/NIOSSLTests/ClientSNITests.swift +++ b/Tests/NIOSSLTests/ClientSNITests.swift @@ -79,8 +79,12 @@ class ClientSNITests: XCTestCase { func testSNIIsRejectedForIPv4Addresses() throws { let context = try configuredSSLContext() - - XCTAssertThrowsError(try NIOSSLClientHandler(context: context, serverHostname: "192.168.0.1")){ error in + + let testString = "192.168.0.1" + XCTAssertThrowsError(try NIOSSLClientTLSProvider(context: context, serverHostname: testString)) { error in + XCTAssertEqual(.cannotUseIPAddressInSNI, error as? NIOSSLExtraError) + } + XCTAssertThrowsError(try NIOSSLClientHandler(context: context, serverHostname: testString)){ error in XCTAssertEqual(.cannotUseIPAddressInSNI, error as? NIOSSLExtraError) } } @@ -88,8 +92,67 @@ class ClientSNITests: XCTestCase { func testSNIIsRejectedForIPv6Addresses() throws { let context = try configuredSSLContext() - XCTAssertThrowsError(try NIOSSLClientHandler(context: context, serverHostname: "fe80::200:f8ff:fe21:67cf")){ error in + let testString = "fe80::200:f8ff:fe21:67cf" + XCTAssertThrowsError(try NIOSSLClientTLSProvider(context: context, serverHostname: testString)) { error in XCTAssertEqual(.cannotUseIPAddressInSNI, error as? NIOSSLExtraError) } + XCTAssertThrowsError(try NIOSSLClientHandler(context: context, serverHostname: testString)){ error in + XCTAssertEqual(.cannotUseIPAddressInSNI, error as? NIOSSLExtraError) + } + + } + + func testSNIIsRejectedForEmptyHostname() throws { + let context = try configuredSSLContext() + + let testString = "" + XCTAssertThrowsError(try NIOSSLClientTLSProvider(context: context, serverHostname: testString)) { error in + XCTAssertEqual(.invalidSNIHostname, error as? NIOSSLExtraError) + } + XCTAssertThrowsError(try NIOSSLClientHandler(context: context, serverHostname: testString)){ error in + XCTAssertEqual(.invalidSNIHostname, error as? NIOSSLExtraError) + } + } + + func testSNIIsRejectedForTooLongHostname() throws { + let context = try configuredSSLContext() + + let testString = String(repeating: "x", count: 256) + XCTAssertThrowsError(try NIOSSLClientTLSProvider(context: context, serverHostname: testString)) { error in + XCTAssertEqual(.invalidSNIHostname, error as? NIOSSLExtraError) + } + XCTAssertThrowsError(try NIOSSLClientHandler(context: context, serverHostname: testString)){ error in + XCTAssertEqual(.invalidSNIHostname, error as? NIOSSLExtraError) + } + } + + func testSNIIsRejectedFor0Byte() throws { + let context = try configuredSSLContext() + + let testString = String(UnicodeScalar(0)!) + XCTAssertThrowsError(try NIOSSLClientTLSProvider(context: context, serverHostname: testString)) { error in + XCTAssertEqual(.invalidSNIHostname, error as? NIOSSLExtraError) + } + XCTAssertThrowsError(try NIOSSLClientHandler(context: context, serverHostname: testString)) { error in + XCTAssertEqual(.invalidSNIHostname, error as? NIOSSLExtraError) + } + } + + func testSNIIsNotRejectedForAnyOfTheFirst1000CodeUnits() throws { + let context = try configuredSSLContext() + + for testString in (1...Int(1000)).compactMap({ UnicodeScalar($0).map { String($0) } }) { + XCTAssertNoThrow(try NIOSSLClientHandler(context: context, serverHostname: testString)) + XCTAssertNoThrow(try NIOSSLClientTLSProvider(context: context, serverHostname: testString)) + } + } + + func testSNIIsNotRejectedForVeryWeirdCharacters() throws { + let context = try configuredSSLContext() + + let testString = "😎🥶💥🏴󠁧󠁢󠁥󠁮󠁧󠁿👩‍💻" + XCTAssertLessThanOrEqual(testString.utf8.count, 255) // just to check we didn't make this too large. + XCTAssertNoThrow(try NIOSSLClientHandler(context: context, serverHostname: testString)) + XCTAssertNoThrow(try NIOSSLClientTLSProvider(context: context, serverHostname: testString)) } } diff --git a/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift b/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift index 71054681..4f879ad0 100644 --- a/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift +++ b/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift @@ -295,13 +295,40 @@ internal func clientTLSChannel(context: NIOSSLContext, serverHostname: String? = nil, file: StaticString = #file, line: UInt = #line) throws -> Channel { - func handlerFactory() throws -> NIOSSLClientHandler { - return try NIOSSLClientHandler(context: context, serverHostname: serverHostname) + func tlsFactory() -> NIOSSLClientTLSProvider { + return try! .init(context: context, serverHostname: serverHostname) } - return try _clientTLSChannel(context: context, preHandlers: preHandlers, postHandlers: postHandlers, group: group, connectingTo: connectingTo, handlerFactory: handlerFactory) + return try _clientTLSChannel(context: context, preHandlers: preHandlers, postHandlers: postHandlers, group: group, connectingTo: connectingTo, tlsFactory: tlsFactory) } +@available(*, deprecated, message: "just for testing the deprecated functionality") +private struct DeprecatedTLSProviderForTests: NIOClientTLSProvider { + public typealias Bootstrap = Bootstrap + + let context: NIOSSLContext + let serverHostname: String? + let verificationCallback: NIOSSLVerificationCallback + + @available(*, deprecated, renamed: "init(context:serverHostname:customVerificationCallback:)") + public init(context: NIOSSLContext, serverHostname: String?, verificationCallback: @escaping NIOSSLVerificationCallback) { + self.context = context + self.serverHostname = serverHostname + self.verificationCallback = verificationCallback + } + + public func enableTLS(_ bootstrap: Bootstrap) -> Bootstrap { + return bootstrap.protocolHandlers { + // NIOSSLClientHandler.init only throws because of `malloc` error and invalid SNI hostnames. We want to crash + // on malloc error and we pre-checked the SNI hostname in `init` so that should be impossible here. + return [try! NIOSSLClientHandler(context: self.context, + serverHostname: self.serverHostname, + verificationCallback: self.verificationCallback)] + } + } +} + + @available(*, deprecated, renamed: "clientTLSChannel(context:preHandlers:postHandlers:group:connectingTo:serverHostname:customVerificationCallback:file:line:)") internal func clientTLSChannel(context: NIOSSLContext, @@ -313,11 +340,15 @@ internal func clientTLSChannel(context: NIOSSLContext, verificationCallback: @escaping NIOSSLVerificationCallback, file: StaticString = #file, line: UInt = #line) throws -> Channel { - func handlerFactory() throws -> NIOSSLClientHandler { - return try NIOSSLClientHandler(context: context, serverHostname: serverHostname, verificationCallback: verificationCallback) + func tlsFactory() -> DeprecatedTLSProviderForTests { + return .init(context: context, serverHostname: serverHostname, verificationCallback: verificationCallback) } - return try _clientTLSChannel(context: context, preHandlers: preHandlers, postHandlers: postHandlers, group: group, connectingTo: connectingTo, handlerFactory: handlerFactory) + return try _clientTLSChannel(context: context, + preHandlers: preHandlers, + postHandlers: postHandlers, + group: group, connectingTo: connectingTo, + tlsFactory: tlsFactory) } @@ -330,33 +361,42 @@ internal func clientTLSChannel(context: NIOSSLContext, customVerificationCallback: @escaping NIOSSLCustomVerificationCallback, file: StaticString = #file, line: UInt = #line) throws -> Channel { - func handlerFactory() throws -> NIOSSLClientHandler { - return try NIOSSLClientHandler(context: context, serverHostname: serverHostname, customVerificationCallback: customVerificationCallback) - } - - return try _clientTLSChannel(context: context, preHandlers: preHandlers, postHandlers: postHandlers, group: group, connectingTo: connectingTo, handlerFactory: handlerFactory) + func tlsFactory() -> NIOSSLClientTLSProvider { + return try! .init(context: context, + serverHostname: serverHostname, + customVerificationCallback: customVerificationCallback) + } + + return try _clientTLSChannel(context: context, + preHandlers: preHandlers, + postHandlers: postHandlers, + group: group, + connectingTo: connectingTo, + tlsFactory: tlsFactory) } -fileprivate func _clientTLSChannel(context: NIOSSLContext, - preHandlers: [ChannelHandler], - postHandlers: [ChannelHandler], - group: EventLoopGroup, - connectingTo: SocketAddress, - handlerFactory: @escaping () throws -> NIOSSLClientHandler, - file: StaticString = #file, - line: UInt = #line) throws -> Channel { - return try assertNoThrowWithValue(ClientBootstrap(group: group) +fileprivate func _clientTLSChannel(context: NIOSSLContext, + preHandlers: [ChannelHandler], + postHandlers: [ChannelHandler], + group: EventLoopGroup, + connectingTo: SocketAddress, + tlsFactory: @escaping () -> TLS, + file: StaticString = #file, + line: UInt = #line) throws -> Channel where TLS.Bootstrap == ClientBootstrap { + let bootstrap = NIOClientTCPBootstrap(ClientBootstrap(group: group), + tls: tlsFactory()) + return try assertNoThrowWithValue(bootstrap .channelInitializer { channel in - let results = preHandlers.map { channel.pipeline.addHandler($0) } - return EventLoopFuture.andAllSucceed(results, on: results.first?.eventLoop ?? group.next()).flatMapThrowing { - try handlerFactory() - }.flatMap { - channel.pipeline.addHandler($0) - }.flatMap { - let results = postHandlers.map { channel.pipeline.addHandler($0) } - return EventLoopFuture.andAllSucceed(results, on: results.first?.eventLoop ?? group.next()) + channel.pipeline.addHandlers(postHandlers) + } + .enableTLS() + .connect(to: connectingTo) + .flatMap { channel in + channel.pipeline.addHandlers(preHandlers, position: .first).map { + channel } - }.connect(to: connectingTo).wait(), file: file, line: line) + } + .wait(), file: file, line: line) } class NIOSSLIntegrationTest: XCTestCase { diff --git a/scripts/sanity.sh b/scripts/sanity.sh index 731f1c79..a196d683 100755 --- a/scripts/sanity.sh +++ b/scripts/sanity.sh @@ -18,7 +18,7 @@ here="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" function replace_acceptable_years() { # this needs to replace all acceptable forms with 'YEARS' - sed -e 's/201[78]-201[89]/YEARS/g' -e 's/2019/YEARS/g' + sed -e 's/20[12][78901]-20[12][8901]/YEARS/' -e 's/20[12][8901]/YEARS/' } printf "=> Checking linux tests... " @@ -130,6 +130,7 @@ EOF cd "$here/.." find . \ \( \! -path './.build/*' -a \ + \! -path './.xcode/*' -a \ \( "${matching_files[@]}" \) -a \ \( \! \( "${exceptions[@]}" \) \) \) | while read line; do if [[ "$(cat "$line" | replace_acceptable_years | head -n $expected_lines | shasum)" != "$expected_sha" ]]; then @@ -143,4 +144,3 @@ EOF done rm "$tmp" -