From 77224fbf7cadb990c700487a280c29897c0088fc Mon Sep 17 00:00:00 2001 From: Boris R Date: Thu, 22 Dec 2022 09:14:24 -0800 Subject: [PATCH 1/3] noHostnameNoPeerVerification --- Sources/NIOSSL/SSLContext.swift | 6 ++-- Sources/NIOSSL/TLSConfiguration.swift | 3 ++ Tests/NIOSSLTests/TLSConfigurationTest.swift | 32 ++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/Sources/NIOSSL/SSLContext.swift b/Sources/NIOSSL/SSLContext.swift index 55995dc6..d5845aa0 100644 --- a/Sources/NIOSSL/SSLContext.swift +++ b/Sources/NIOSSL/SSLContext.swift @@ -569,8 +569,10 @@ extension NIOSSLContext { private static func configureCertificateValidation(context: OpaquePointer, verification: CertificateVerification, trustRoots: NIOSSLTrustRoots?, additionalTrustRoots: [NIOSSLAdditionalTrustRoots], sendCANames: Bool) throws { // If validation is turned on, set the trust roots and turn on cert validation. switch verification { - case .fullVerification, .noHostnameVerification: - CNIOBoringSSL_SSL_CTX_set_verify(context, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nil) + case .fullVerification, .noHostnameVerification, .noHostnameNoPeerVerification: + let flags = verification == .noHostnameNoPeerVerification ? SSL_VERIFY_PEER + : SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT + CNIOBoringSSL_SSL_CTX_set_verify(context, flags, nil) // Also, set TRUSTED_FIRST to work around dumb clients that don't know what they're doing and send // untrusted root certs. X509_VERIFY_PARAM will or-in the flags, so we don't need to load them first. diff --git a/Sources/NIOSSL/TLSConfiguration.swift b/Sources/NIOSSL/TLSConfiguration.swift index f3429b41..a8146796 100644 --- a/Sources/NIOSSL/TLSConfiguration.swift +++ b/Sources/NIOSSL/TLSConfiguration.swift @@ -140,6 +140,9 @@ public enum CertificateVerification: Sendable { /// be checked to see if they are valid for the given hostname. case noHostnameVerification + /// Peer certificate is optional + case noHostnameNoPeerVerification + /// Certificates will be validated against the trust store and checked /// against the hostname of the service we are contacting. case fullVerification diff --git a/Tests/NIOSSLTests/TLSConfigurationTest.swift b/Tests/NIOSSLTests/TLSConfigurationTest.swift index c3acd576..599d0590 100644 --- a/Tests/NIOSSLTests/TLSConfigurationTest.swift +++ b/Tests/NIOSSLTests/TLSConfigurationTest.swift @@ -390,6 +390,38 @@ class TLSConfigurationTest: XCTestCase { try assertPostHandshakeError(withClientConfig: clientConfig, andServerConfig: serverConfig, errorTextContainsAnyOf: ["CERTIFICATE_REQUIRED"]) } + + func testMutualValidationOptionalClientCertificatePreTLS13() throws { + var clientConfig = TLSConfiguration.makeClientConfiguration() + clientConfig.maximumTLSVersion = .tlsv12 + clientConfig.certificateVerification = .none + + var serverConfig = TLSConfiguration.makeServerConfiguration( + certificateChain: [.certificate(TLSConfigurationTest.cert1)], + privateKey: .privateKey(TLSConfigurationTest.key1) + ) + serverConfig.maximumTLSVersion = .tlsv12 + serverConfig.certificateVerification = .noHostnameNoPeerVerification + serverConfig.trustRoots = .certificates([TLSConfigurationTest.cert2]) + + try assertHandshakeSucceeded(withClientConfig: clientConfig, andServerConfig: serverConfig) + } + + func testMutualValidationOptionalClientCertificatePostTLS13() throws { + var clientConfig = TLSConfiguration.makeClientConfiguration() + clientConfig.minimumTLSVersion = .tlsv13 + clientConfig.certificateVerification = .none + + var serverConfig = TLSConfiguration.makeServerConfiguration( + certificateChain: [.certificate(TLSConfigurationTest.cert1)], + privateKey: .privateKey(TLSConfigurationTest.key1) + ) + serverConfig.minimumTLSVersion = .tlsv13 + serverConfig.certificateVerification = .noHostnameNoPeerVerification + serverConfig.trustRoots = .certificates([TLSConfigurationTest.cert2]) + + try assertHandshakeSucceeded(withClientConfig: clientConfig, andServerConfig: serverConfig) + } func testIncompatibleSignatures() throws { var clientConfig = TLSConfiguration.makeClientConfiguration() From 17b7d7e8a127ded9a8bc6ac040b03cf9d07e31aa Mon Sep 17 00:00:00 2001 From: Boris R Date: Thu, 22 Dec 2022 13:41:37 -0800 Subject: [PATCH 2/3] Refactor to have no new enum case --- Sources/NIOSSL/SSLContext.swift | 10 +++++----- Sources/NIOSSL/TLSConfiguration.swift | 4 +--- Tests/NIOSSLTests/TLSConfigurationTest.swift | 6 ++++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Sources/NIOSSL/SSLContext.swift b/Sources/NIOSSL/SSLContext.swift index d5845aa0..cd962769 100644 --- a/Sources/NIOSSL/SSLContext.swift +++ b/Sources/NIOSSL/SSLContext.swift @@ -310,7 +310,8 @@ public final class NIOSSLContext { verification: configuration.certificateVerification, trustRoots: configuration.trustRoots, additionalTrustRoots: configuration.additionalTrustRoots, - sendCANames: configuration.sendCANameList) + sendCANames: configuration.sendCANameList, + certificateRequired: configuration.certificateRequired) // Configure verification algorithms if let verifySignatureAlgorithms = configuration.verifySignatureAlgorithms { @@ -566,12 +567,11 @@ extension NIOSSLContext { // Configuring certificate verification extension NIOSSLContext { - private static func configureCertificateValidation(context: OpaquePointer, verification: CertificateVerification, trustRoots: NIOSSLTrustRoots?, additionalTrustRoots: [NIOSSLAdditionalTrustRoots], sendCANames: Bool) throws { + private static func configureCertificateValidation(context: OpaquePointer, verification: CertificateVerification, trustRoots: NIOSSLTrustRoots?, additionalTrustRoots: [NIOSSLAdditionalTrustRoots], sendCANames: Bool, certificateRequired: Bool) throws { // If validation is turned on, set the trust roots and turn on cert validation. switch verification { - case .fullVerification, .noHostnameVerification, .noHostnameNoPeerVerification: - let flags = verification == .noHostnameNoPeerVerification ? SSL_VERIFY_PEER - : SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT + case .fullVerification, .noHostnameVerification: + let flags = certificateRequired ? SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT : SSL_VERIFY_PEER CNIOBoringSSL_SSL_CTX_set_verify(context, flags, nil) // Also, set TRUSTED_FIRST to work around dumb clients that don't know what they're doing and send diff --git a/Sources/NIOSSL/TLSConfiguration.swift b/Sources/NIOSSL/TLSConfiguration.swift index a8146796..972c9ebe 100644 --- a/Sources/NIOSSL/TLSConfiguration.swift +++ b/Sources/NIOSSL/TLSConfiguration.swift @@ -140,9 +140,6 @@ public enum CertificateVerification: Sendable { /// be checked to see if they are valid for the given hostname. case noHostnameVerification - /// Peer certificate is optional - case noHostnameNoPeerVerification - /// Certificates will be validated against the trust store and checked /// against the hostname of the service we are contacting. case fullVerification @@ -271,6 +268,7 @@ public struct TLSConfiguration { /// Whether to verify remote certificates. public var certificateVerification: CertificateVerification + public var certificateRequired: Bool = true /// The trust roots to use to validate certificates. This only needs to be provided if you intend to validate /// certificates. diff --git a/Tests/NIOSSLTests/TLSConfigurationTest.swift b/Tests/NIOSSLTests/TLSConfigurationTest.swift index 599d0590..4307ec06 100644 --- a/Tests/NIOSSLTests/TLSConfigurationTest.swift +++ b/Tests/NIOSSLTests/TLSConfigurationTest.swift @@ -401,7 +401,8 @@ class TLSConfigurationTest: XCTestCase { privateKey: .privateKey(TLSConfigurationTest.key1) ) serverConfig.maximumTLSVersion = .tlsv12 - serverConfig.certificateVerification = .noHostnameNoPeerVerification + serverConfig.certificateVerification = .noHostnameVerification + serverConfig.certificateRequired = false serverConfig.trustRoots = .certificates([TLSConfigurationTest.cert2]) try assertHandshakeSucceeded(withClientConfig: clientConfig, andServerConfig: serverConfig) @@ -417,7 +418,8 @@ class TLSConfigurationTest: XCTestCase { privateKey: .privateKey(TLSConfigurationTest.key1) ) serverConfig.minimumTLSVersion = .tlsv13 - serverConfig.certificateVerification = .noHostnameNoPeerVerification + serverConfig.certificateVerification = .noHostnameVerification + serverConfig.certificateRequired = false serverConfig.trustRoots = .certificates([TLSConfigurationTest.cert2]) try assertHandshakeSucceeded(withClientConfig: clientConfig, andServerConfig: serverConfig) From e42c2a3a0a37f9aecdbce280cd1f199ecec34b87 Mon Sep 17 00:00:00 2001 From: Boris R Date: Thu, 22 Dec 2022 14:10:14 -0800 Subject: [PATCH 3/3] Remove extra space --- Sources/NIOSSL/SSLContext.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/NIOSSL/SSLContext.swift b/Sources/NIOSSL/SSLContext.swift index cd962769..16a2f608 100644 --- a/Sources/NIOSSL/SSLContext.swift +++ b/Sources/NIOSSL/SSLContext.swift @@ -571,7 +571,7 @@ extension NIOSSLContext { // If validation is turned on, set the trust roots and turn on cert validation. switch verification { case .fullVerification, .noHostnameVerification: - let flags = certificateRequired ? SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT : SSL_VERIFY_PEER + let flags = certificateRequired ? SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT : SSL_VERIFY_PEER CNIOBoringSSL_SSL_CTX_set_verify(context, flags, nil) // Also, set TRUSTED_FIRST to work around dumb clients that don't know what they're doing and send