From 7fe465e029eb91b7043bacaad61d17f43b9f411a Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Fri, 19 Mar 2021 17:09:02 +0000 Subject: [PATCH 1/7] Generate trust roots SecCertificate for Transport Services --- .dockerignore | 2 + .../TLSConfiguration.swift | 60 ++++++++++++++++--- 2 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 .dockerignore diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 000000000..2fb33437e --- /dev/null +++ b/.dockerignore @@ -0,0 +1,2 @@ +.build +.git \ No newline at end of file diff --git a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift index 6dcebb52a..c1e6cffb2 100644 --- a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift +++ b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift @@ -109,6 +109,11 @@ preconditionFailure("TLSConfiguration.keyLogCallback is not supported. \(useMTELGExplainer)") } + // the certificate chain + if self.certificateChain.count > 0 { + preconditionFailure("TLSConfiguration.certificateChain is not supported") + } + // private key if self.privateKey != nil { preconditionFailure("TLSConfiguration.privateKey is not supported. \(useMTELGExplainer)") @@ -117,20 +122,58 @@ // renegotiation support key is unsupported // trust roots - if let trustRoots = self.trustRoots { - guard case .default = trustRoots else { - preconditionFailure("TLSConfiguration.trustRoots != .default is not supported. \(useMTELGExplainer)") + var secTrustRoots: [SecCertificate]? + switch trustRoots { + case .some(.certificates(let certificates)): + do { + secTrustRoots = try certificates.compactMap { certificate in + return try SecCertificateCreateWithData(nil, Data(certificate.toDERBytes()) as CFData) + } + } catch { + // failed to load } + case .some(.file): + preconditionFailure("TLSConfiguration.trustRoots.file is not supported") + break + + case .some(.default), .none: + break } - switch self.certificateVerification { - case .none: + precondition(self.certificateVerification != .noHostnameVerification, "TLSConfiguration.certificateVerification = .noHostnameVerification is not supported") + + if certificateVerification != .fullVerification || trustRoots != nil { // add verify block to control certificate verification sec_protocol_options_set_verify_block( options.securityProtocolOptions, - { _, _, sec_protocol_verify_complete in - sec_protocol_verify_complete(true) - }, TLSConfiguration.tlsDispatchQueue + { sec_metadata, sec_trust, sec_protocol_verify_complete in + guard self.certificateVerification != .none else { + sec_protocol_verify_complete(true) + return + } + + let trust = sec_trust_copy_ref(sec_trust).takeRetainedValue() + if let trustRootCertificates = secTrustRoots { + SecTrustSetAnchorCertificates(trust, trustRootCertificates as CFArray) + } + if #available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) { + SecTrustEvaluateAsyncWithError(trust, Self.tlsDispatchQueue) { (trust, result, error) in + if let error = error { + print("Trust failed: \(error.localizedDescription)") + } + sec_protocol_verify_complete(result) + } + } else { + SecTrustEvaluateAsync(trust, Self.tlsDispatchQueue) { (trust, result) in + switch result { + case .proceed, .unspecified: + sec_protocol_verify_complete(true) + default: + sec_protocol_verify_complete(false) + } + } + } + }, Self.tlsDispatchQueue ) case .noHostnameVerification: @@ -140,7 +183,6 @@ case .fullVerification: break } - return options } } From c6f048d2a9395ee4f89bb7f75d111a32607065e9 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Sun, 21 Mar 2021 10:56:56 +0000 Subject: [PATCH 2/7] swift format --- .../NIOTransportServices/TLSConfiguration.swift | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift index c1e6cffb2..960c57caa 100644 --- a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift +++ b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift @@ -127,14 +127,13 @@ case .some(.certificates(let certificates)): do { secTrustRoots = try certificates.compactMap { certificate in - return try SecCertificateCreateWithData(nil, Data(certificate.toDERBytes()) as CFData) + try SecCertificateCreateWithData(nil, Data(certificate.toDERBytes()) as CFData) } } catch { // failed to load } case .some(.file): preconditionFailure("TLSConfiguration.trustRoots.file is not supported") - break case .some(.default), .none: break @@ -146,7 +145,7 @@ // add verify block to control certificate verification sec_protocol_options_set_verify_block( options.securityProtocolOptions, - { sec_metadata, sec_trust, sec_protocol_verify_complete in + { _, sec_trust, sec_protocol_verify_complete in guard self.certificateVerification != .none else { sec_protocol_verify_complete(true) return @@ -157,20 +156,20 @@ SecTrustSetAnchorCertificates(trust, trustRootCertificates as CFArray) } if #available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) { - SecTrustEvaluateAsyncWithError(trust, Self.tlsDispatchQueue) { (trust, result, error) in + SecTrustEvaluateAsyncWithError(trust, Self.tlsDispatchQueue) { _, result, error in if let error = error { print("Trust failed: \(error.localizedDescription)") } sec_protocol_verify_complete(result) } } else { - SecTrustEvaluateAsync(trust, Self.tlsDispatchQueue) { (trust, result) in - switch result { - case .proceed, .unspecified: + SecTrustEvaluateAsync(trust, Self.tlsDispatchQueue) { _, result in + switch result { + case .proceed, .unspecified: sec_protocol_verify_complete(true) - default: + default: sec_protocol_verify_complete(false) - } + } } } }, Self.tlsDispatchQueue From af884fbca4d2028d35c83bc3006208aac648a6b4 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Mon, 22 Mar 2021 16:46:33 +0000 Subject: [PATCH 3/7] Allow errors thrown by SecCertificateCreateWithData to passed up the callstack --- .../NIOTransportServices/TLSConfiguration.swift | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift index 960c57caa..1a4efdc59 100644 --- a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift +++ b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift @@ -60,7 +60,7 @@ /// /// - Parameter queue: Dispatch queue to run `sec_protocol_options_set_verify_block` on. /// - Returns: Equivalent NWProtocolTLS Options - func getNWProtocolTLSOptions() -> NWProtocolTLS.Options { + func getNWProtocolTLSOptions() throws -> NWProtocolTLS.Options { let options = NWProtocolTLS.Options() let useMTELGExplainer = """ @@ -125,12 +125,8 @@ var secTrustRoots: [SecCertificate]? switch trustRoots { case .some(.certificates(let certificates)): - do { - secTrustRoots = try certificates.compactMap { certificate in - try SecCertificateCreateWithData(nil, Data(certificate.toDERBytes()) as CFData) - } - } catch { - // failed to load + secTrustRoots = try certificates.compactMap { certificate in + try SecCertificateCreateWithData(nil, Data(certificate.toDERBytes()) as CFData) } case .some(.file): preconditionFailure("TLSConfiguration.trustRoots.file is not supported") From ad02945155b0326e15641c3736626d1bef7c08f5 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 22 Apr 2021 15:07:57 +0100 Subject: [PATCH 4/7] Support TLSConfiguration.trustRoots(.file)... Support TLSConfiguration.trustRoots(.file) when converting to a `NWProtocolTLS.Options` --- .../NIOTransportServices/TLSConfiguration.swift | 7 +++++-- .../HTTPClientNIOTSTests.swift | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift index 1a4efdc59..6c6cbe488 100644 --- a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift +++ b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift @@ -128,8 +128,11 @@ secTrustRoots = try certificates.compactMap { certificate in try SecCertificateCreateWithData(nil, Data(certificate.toDERBytes()) as CFData) } - case .some(.file): - preconditionFailure("TLSConfiguration.trustRoots.file is not supported") + case .some(.file(let file)): + let certificates = try NIOSSLCertificate.fromPEMFile(file) + secTrustRoots = try certificates.compactMap { certificate in + try SecCertificateCreateWithData(nil, Data(certificate.toDERBytes()) as CFData) + } case .some(.default), .none: break diff --git a/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests.swift index a8d2088d7..244aeadb1 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests.swift @@ -111,4 +111,19 @@ class HTTPClientNIOTSTests: XCTestCase { } #endif } + + func testTrustRootCertificateLoadFail() { + guard isTestingNIOTS() else { return } + #if canImport(Network) + let tlsConfig = TLSConfiguration.forClient(trustRoots: .file("not/a/certificate")) + XCTAssertThrowsError(try tlsConfig.getNWProtocolTLSOptions()) { error in + switch error { + case let error as NIOSSL.NIOSSLError where error == .failedToLoadCertificate: + break + default: + XCTFail("\(error)") + } + } + #endif + } } From 869ec003f6dc59262e521d99565aa90ff96ec186 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 22 Apr 2021 15:13:10 +0100 Subject: [PATCH 5/7] Linux tests --- .../HTTPClientNIOTSTests+XCTest.swift | 1 + .../HTTPClientNIOTSTests.swift | 18 +++++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests+XCTest.swift index c0d86085f..cc33f6aee 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests+XCTest.swift @@ -29,6 +29,7 @@ extension HTTPClientNIOTSTests { ("testTLSFailError", testTLSFailError), ("testConnectionFailError", testConnectionFailError), ("testTLSVersionError", testTLSVersionError), + ("testTrustRootCertificateLoadFail", testTrustRootCertificateLoadFail), ] } } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests.swift index 244aeadb1..9a0070229 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests.swift @@ -111,19 +111,19 @@ class HTTPClientNIOTSTests: XCTestCase { } #endif } - + func testTrustRootCertificateLoadFail() { guard isTestingNIOTS() else { return } #if canImport(Network) - let tlsConfig = TLSConfiguration.forClient(trustRoots: .file("not/a/certificate")) - XCTAssertThrowsError(try tlsConfig.getNWProtocolTLSOptions()) { error in - switch error { - case let error as NIOSSL.NIOSSLError where error == .failedToLoadCertificate: - break - default: - XCTFail("\(error)") + let tlsConfig = TLSConfiguration.forClient(trustRoots: .file("not/a/certificate")) + XCTAssertThrowsError(try tlsConfig.getNWProtocolTLSOptions()) { error in + switch error { + case let error as NIOSSL.NIOSSLError where error == .failedToLoadCertificate: + break + default: + XCTFail("\(error)") + } } - } #endif } } From 29ea5c3c0bee16d013d888ea22c99f7138a8bef6 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 13 May 2021 13:09:53 +0100 Subject: [PATCH 6/7] Create NWProtocolTLS.Options on DispatchQueue --- .../TLSConfiguration.swift | 29 +++++++++++++------ Sources/AsyncHTTPClient/Utils.swift | 8 +++-- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift index 6c6cbe488..816a1fb3f 100644 --- a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift +++ b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift @@ -16,6 +16,7 @@ import Foundation import Network + import NIO import NIOSSL import NIOTransportServices @@ -58,7 +59,23 @@ /// create NWProtocolTLS.Options for use with NIOTransportServices from the NIOSSL TLSConfiguration /// - /// - Parameter queue: Dispatch queue to run `sec_protocol_options_set_verify_block` on. + /// - Parameter eventLoop: EventLoop to wait for creation of options on + /// - Returns: Future holding NWProtocolTLS Options + func getNWProtocolTLSOptions(on eventLoop: EventLoop) -> EventLoopFuture { + let promise = eventLoop.makePromise(of: NWProtocolTLS.Options.self) + Self.tlsDispatchQueue.async { + do { + let options = try self.getNWProtocolTLSOptions() + promise.succeed(options) + } catch { + promise.fail(error) + } + } + return promise.futureResult + } + + /// create NWProtocolTLS.Options for use with NIOTransportServices from the NIOSSL TLSConfiguration + /// /// - Returns: Equivalent NWProtocolTLS Options func getNWProtocolTLSOptions() throws -> NWProtocolTLS.Options { let options = NWProtocolTLS.Options() @@ -138,7 +155,8 @@ break } - precondition(self.certificateVerification != .noHostnameVerification, "TLSConfiguration.certificateVerification = .noHostnameVerification is not supported") + precondition(self.certificateVerification != .noHostnameVerification, + "TLSConfiguration.certificateVerification = .noHostnameVerification is not supported. \(useMTELGExplainer)") if certificateVerification != .fullVerification || trustRoots != nil { // add verify block to control certificate verification @@ -173,13 +191,6 @@ } }, Self.tlsDispatchQueue ) - - case .noHostnameVerification: - precondition(self.certificateVerification != .noHostnameVerification, - "TLSConfiguration.certificateVerification = .noHostnameVerification is not supported. \(useMTELGExplainer)") - - case .fullVerification: - break } return options } diff --git a/Sources/AsyncHTTPClient/Utils.swift b/Sources/AsyncHTTPClient/Utils.swift index 1af7899b2..6069222b1 100644 --- a/Sources/AsyncHTTPClient/Utils.swift +++ b/Sources/AsyncHTTPClient/Utils.swift @@ -180,9 +180,11 @@ extension NIOClientTCPBootstrap { // if eventLoop is compatible with NIOTransportServices create a NIOTSConnectionBootstrap if #available(OSX 10.14, iOS 12.0, tvOS 12.0, watchOS 6.0, *), let tsBootstrap = NIOTSConnectionBootstrap(validatingGroup: eventLoop) { // create NIOClientTCPBootstrap with NIOTS TLS provider - let parameters = tlsConfiguration.getNWProtocolTLSOptions() - let tlsProvider = NIOTSClientTLSProvider(tlsOptions: parameters) - return eventLoop.makeSucceededFuture(NIOClientTCPBootstrap(tsBootstrap, tls: tlsProvider)) + return tlsConfiguration.getNWProtocolTLSOptions(on: eventLoop) + .map { parameters in + let tlsProvider = NIOTSClientTLSProvider(tlsOptions: parameters) + return NIOClientTCPBootstrap(tsBootstrap, tls: tlsProvider) + } } #endif From 508233a18d3984ba1ff70f469db7b81353a78168 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 13 May 2021 13:20:33 +0100 Subject: [PATCH 7/7] Added dispatchPreCondition --- .../NIOTransportServices/TLSConfiguration.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift index 816a1fb3f..86867c6f9 100644 --- a/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift +++ b/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift @@ -128,7 +128,7 @@ // the certificate chain if self.certificateChain.count > 0 { - preconditionFailure("TLSConfiguration.certificateChain is not supported") + preconditionFailure("TLSConfiguration.certificateChain is not supported. \(useMTELGExplainer)") } // private key @@ -173,6 +173,7 @@ SecTrustSetAnchorCertificates(trust, trustRootCertificates as CFArray) } if #available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) { + dispatchPrecondition(condition: .onQueue(Self.tlsDispatchQueue)) SecTrustEvaluateAsyncWithError(trust, Self.tlsDispatchQueue) { _, result, error in if let error = error { print("Trust failed: \(error.localizedDescription)")