diff --git a/Sources/NIOSSL/IdentityVerification.swift b/Sources/NIOSSL/IdentityVerification.swift index 4d57220f..7b27166b 100644 --- a/Sources/NIOSSL/IdentityVerification.swift +++ b/Sources/NIOSSL/IdentityVerification.swift @@ -87,23 +87,27 @@ extension UInt8 { /// follow along at home. internal func validIdentityForService(serverHostname: String?, socketAddress: SocketAddress, - leafCertificate: NIOSSLCertificate) throws -> Bool { + leafCertificate: NIOSSLCertificate, + includeCommonName: Bool) throws -> Bool { if let serverHostname = serverHostname { return try serverHostname.withLowercaseASCIIBuffer { try validIdentityForService(serverHostname: $0, socketAddress: socketAddress, - leafCertificate: leafCertificate) + leafCertificate: leafCertificate, + includeCommonName: includeCommonName) } } else { return try validIdentityForService(serverHostname: nil as Array?, socketAddress: socketAddress, - leafCertificate: leafCertificate) + leafCertificate: leafCertificate, + includeCommonName: includeCommonName) } } private func validIdentityForService(serverHostname: Array?, socketAddress: SocketAddress, - leafCertificate: NIOSSLCertificate) throws -> Bool { + leafCertificate: NIOSSLCertificate, + includeCommonName: Bool) throws -> Bool { // Before we begin, we want to locate the first period in our own domain name. We need to do // this because we may need to match a wildcard label. var serverHostnameSlice: ArraySlice? = nil @@ -150,16 +154,15 @@ private func validIdentityForService(serverHostname: Array?, } // In the absence of any matchable subjectAlternativeNames, we can fall back to checking - // the common name. This is a deprecated practice, and in a future release we should - // stop doing this. - guard let commonName = leafCertificate.commonName() else { - // No CN, no match. + // the common name. We only do this if the user has configured a system that trusts the commonName field. + if includeCommonName, let commonName = leafCertificate.commonName() { + // We have a common name. Let's check it against the provided hostname. We never check + // the common name against the IP address. + return matchHostname(ourHostname: serverHostnameSlice, firstPeriodIndex: firstPeriodIndex, dnsName: commonName) + } else { + // We don't trust the common name, or we do but don't have one to look at. return false } - - // We have a common name. Let's check it against the provided hostname. We never check - // the common name against the IP address. - return matchHostname(ourHostname: serverHostnameSlice, firstPeriodIndex: firstPeriodIndex, dnsName: commonName) } diff --git a/Sources/NIOSSL/SSLConnection.swift b/Sources/NIOSSL/SSLConnection.swift index 814debab..dac9c578 100644 --- a/Sources/NIOSSL/SSLConnection.swift +++ b/Sources/NIOSSL/SSLConnection.swift @@ -213,7 +213,8 @@ internal final class SSLConnection { guard try validIdentityForService(serverHostname: self.expectedHostname, socketAddress: address, - leafCertificate: peerCert) else { + leafCertificate: peerCert, + includeCommonName: self.parentContext.configuration.trustCommonName) else { throw NIOSSLExtraError.failedToValidateHostname(expectedName: self.expectedHostname ?? "") } } diff --git a/Sources/NIOSSL/TLSConfiguration.swift b/Sources/NIOSSL/TLSConfiguration.swift index abf5ec57..127a21ef 100644 --- a/Sources/NIOSSL/TLSConfiguration.swift +++ b/Sources/NIOSSL/TLSConfiguration.swift @@ -303,6 +303,18 @@ public struct TLSConfiguration { /// Whether renegotiation is supported. public var renegotiationSupport: NIORenegotiationSupport + /// Whether to trust the commonName field when validating certificate hostnames. + /// + /// The commonName field of certificates has been deprecated for use as a validation source + /// for nearly two decades. As browsers and the WebPKI now forbid the usage of common names + /// in certificates, swift-nio-ssl defaults to this strict level of enforcement as well. + /// + /// However, some older infrastructure does not issue certificates with valid subject alternative + /// name fields. For this reason, we currently allow an override for trusting the common name. This + /// is a temporary measure to allow users to work around these limitations, but we will remove this + /// functionality in future. + public var trustCommonName: Bool + private init(cipherSuiteValues: [NIOTLSCipher] = [], cipherSuites: String = defaultCipherSuites, verifySignatureAlgorithms: [SignatureAlgorithm]?, @@ -317,7 +329,8 @@ public struct TLSConfiguration { shutdownTimeout: TimeAmount, keyLogCallback: NIOSSLKeyLogCallback?, renegotiationSupport: NIORenegotiationSupport, - additionalTrustRoots: [NIOSSLAdditionalTrustRoots]) { + additionalTrustRoots: [NIOSSLAdditionalTrustRoots], + trustCommonName: Bool) { self.cipherSuites = cipherSuites self.verifySignatureAlgorithms = verifySignatureAlgorithms self.signingSignatureAlgorithms = signingSignatureAlgorithms @@ -331,6 +344,7 @@ public struct TLSConfiguration { self.encodedApplicationProtocols = [] self.shutdownTimeout = shutdownTimeout self.renegotiationSupport = renegotiationSupport + self.trustCommonName = trustCommonName self.applicationProtocols = applicationProtocols self.keyLogCallback = keyLogCallback if !cipherSuiteValues.isEmpty { @@ -368,7 +382,8 @@ public struct TLSConfiguration { shutdownTimeout: shutdownTimeout, keyLogCallback: keyLogCallback, renegotiationSupport: .none, // Servers never support renegotiation: there's no point. - additionalTrustRoots: additionalTrustRoots) + additionalTrustRoots: additionalTrustRoots, + trustCommonName: false) } /// Create a TLS configuration for use with server-side contexts. @@ -398,7 +413,8 @@ public struct TLSConfiguration { shutdownTimeout: shutdownTimeout, keyLogCallback: keyLogCallback, renegotiationSupport: .none, // Servers never support renegotiation: there's no point. - additionalTrustRoots: []) + additionalTrustRoots: [], + trustCommonName: false) } /// Create a TLS configuration for use with server-side contexts. @@ -430,7 +446,8 @@ public struct TLSConfiguration { shutdownTimeout: shutdownTimeout, keyLogCallback: keyLogCallback, renegotiationSupport: .none, // Servers never support renegotiation: there's no point. - additionalTrustRoots: []) + additionalTrustRoots: [], + trustCommonName: false) } /// Create a TLS configuration for use with server-side contexts. @@ -463,7 +480,43 @@ public struct TLSConfiguration { shutdownTimeout: shutdownTimeout, keyLogCallback: keyLogCallback, renegotiationSupport: .none, // Servers never support renegotiation: there's no point. - additionalTrustRoots: additionalTrustRoots) + additionalTrustRoots: additionalTrustRoots, + trustCommonName: false) + } + + /// Create a TLS configuration for use with server-side contexts. + /// + /// This provides sensible defaults while requiring that you provide any data that is necessary + /// for server-side function. For client use, try `forClient` instead. + public static func forServer(certificateChain: [NIOSSLCertificateSource], + privateKey: NIOSSLPrivateKeySource, + cipherSuites: String = defaultCipherSuites, + verifySignatureAlgorithms: [SignatureAlgorithm]? = nil, + signingSignatureAlgorithms: [SignatureAlgorithm]? = nil, + minimumTLSVersion: TLSVersion = .tlsv1, + maximumTLSVersion: TLSVersion? = nil, + certificateVerification: CertificateVerification = .none, + trustRoots: NIOSSLTrustRoots = .default, + applicationProtocols: [String] = [], + shutdownTimeout: TimeAmount = .seconds(5), + keyLogCallback: NIOSSLKeyLogCallback? = nil, + additionalTrustRoots: [NIOSSLAdditionalTrustRoots] = [], + trustCommonName: Bool) -> TLSConfiguration { + return TLSConfiguration(cipherSuites: cipherSuites, + verifySignatureAlgorithms: verifySignatureAlgorithms, + signingSignatureAlgorithms: signingSignatureAlgorithms, + minimumTLSVersion: minimumTLSVersion, + maximumTLSVersion: maximumTLSVersion, + certificateVerification: certificateVerification, + trustRoots: trustRoots, + certificateChain: certificateChain, + privateKey: privateKey, + applicationProtocols: applicationProtocols, + shutdownTimeout: shutdownTimeout, + keyLogCallback: keyLogCallback, + renegotiationSupport: .none, // Servers never support renegotiation: there's no point. + additionalTrustRoots: additionalTrustRoots, + trustCommonName: trustCommonName) } /// Creates a TLS configuration for use with client-side contexts. This allows setting the `NIOTLSCipher` property specifically. @@ -497,7 +550,8 @@ public struct TLSConfiguration { shutdownTimeout: shutdownTimeout, keyLogCallback: keyLogCallback, renegotiationSupport: renegotiationSupport, - additionalTrustRoots: additionalTrustRoots) + additionalTrustRoots: additionalTrustRoots, + trustCommonName: false) } /// Creates a TLS configuration for use with client-side contexts. @@ -527,7 +581,8 @@ public struct TLSConfiguration { shutdownTimeout: shutdownTimeout, keyLogCallback: keyLogCallback, renegotiationSupport: .none, // Default value is here for backward-compatibility. - additionalTrustRoots: []) + additionalTrustRoots: [], + trustCommonName: false) } @@ -559,7 +614,8 @@ public struct TLSConfiguration { shutdownTimeout: shutdownTimeout, keyLogCallback: keyLogCallback, renegotiationSupport: renegotiationSupport, - additionalTrustRoots: []) + additionalTrustRoots: [], + trustCommonName: false) } /// Creates a TLS configuration for use with client-side contexts. @@ -592,7 +648,8 @@ public struct TLSConfiguration { shutdownTimeout: shutdownTimeout, keyLogCallback: keyLogCallback, renegotiationSupport: renegotiationSupport, - additionalTrustRoots: []) + additionalTrustRoots: [], + trustCommonName: false) } /// Creates a TLS configuration for use with client-side contexts. @@ -626,7 +683,44 @@ public struct TLSConfiguration { shutdownTimeout: shutdownTimeout, keyLogCallback: keyLogCallback, renegotiationSupport: renegotiationSupport, - additionalTrustRoots: additionalTrustRoots) + additionalTrustRoots: additionalTrustRoots, + trustCommonName: false) + } + + /// Creates a TLS configuration for use with client-side contexts. This allows setting the `NIOTLSCipher` property specifically. + /// + /// This provides sensible defaults, and can be used without customisation. For server-side + /// contexts, you should use `forServer` instead. + public static func forClient(cipherSuites: String = defaultCipherSuites, + verifySignatureAlgorithms: [SignatureAlgorithm]? = nil, + signingSignatureAlgorithms: [SignatureAlgorithm]? = nil, + minimumTLSVersion: TLSVersion = .tlsv1, + maximumTLSVersion: TLSVersion? = nil, + certificateVerification: CertificateVerification = .fullVerification, + trustRoots: NIOSSLTrustRoots = .default, + certificateChain: [NIOSSLCertificateSource] = [], + privateKey: NIOSSLPrivateKeySource? = nil, + applicationProtocols: [String] = [], + shutdownTimeout: TimeAmount = .seconds(5), + keyLogCallback: NIOSSLKeyLogCallback? = nil, + renegotiationSupport: NIORenegotiationSupport = .none, + additionalTrustRoots: [NIOSSLAdditionalTrustRoots] = [], + trustCommonName: Bool) -> TLSConfiguration { + return TLSConfiguration(cipherSuites: cipherSuites, + verifySignatureAlgorithms: verifySignatureAlgorithms, + signingSignatureAlgorithms: signingSignatureAlgorithms, + minimumTLSVersion: minimumTLSVersion, + maximumTLSVersion: maximumTLSVersion, + certificateVerification: certificateVerification, + trustRoots: trustRoots, + certificateChain: certificateChain, + privateKey: privateKey, + applicationProtocols: applicationProtocols, + shutdownTimeout: shutdownTimeout, + keyLogCallback: keyLogCallback, + renegotiationSupport: renegotiationSupport, + additionalTrustRoots: additionalTrustRoots, + trustCommonName: trustCommonName) } } @@ -657,7 +751,8 @@ extension TLSConfiguration { self.encodedApplicationProtocols == comparing.encodedApplicationProtocols && self.shutdownTimeout == comparing.shutdownTimeout && isKeyLoggerCallbacksEqual && - self.renegotiationSupport == comparing.renegotiationSupport + self.renegotiationSupport == comparing.renegotiationSupport && + self.trustCommonName == comparing.trustCommonName } /// Returns a best effort hash of this TLS configuration. @@ -682,5 +777,6 @@ extension TLSConfiguration { hasher.combine(bytes: closureBits) } hasher.combine(renegotiationSupport) + hasher.combine(trustCommonName) } } diff --git a/Tests/NIOSSLTests/IdentityVerificationTest+XCTest.swift b/Tests/NIOSSLTests/IdentityVerificationTest+XCTest.swift index b8a8d11c..29592a29 100644 --- a/Tests/NIOSSLTests/IdentityVerificationTest+XCTest.swift +++ b/Tests/NIOSSLTests/IdentityVerificationTest+XCTest.swift @@ -51,6 +51,8 @@ extension IdentityVerificationTest { ("testDoesNotMatchSANWithEmbeddedNULL", testDoesNotMatchSANWithEmbeddedNULL), ("testFallsBackToCommonName", testFallsBackToCommonName), ("testLowercasesForCommonName", testLowercasesForCommonName), + ("testDoesNotFallBackToCommonName", testDoesNotFallBackToCommonName), + ("testNoLowercasesForCommonName", testNoLowercasesForCommonName), ("testRejectsUnicodeCommonNameWithUnencodedIDNALabel", testRejectsUnicodeCommonNameWithUnencodedIDNALabel), ("testRejectsUnicodeCommonNameWithEncodedIDNALabel", testRejectsUnicodeCommonNameWithEncodedIDNALabel), ("testHandlesMissingCommonName", testHandlesMissingCommonName), diff --git a/Tests/NIOSSLTests/IdentityVerificationTest.swift b/Tests/NIOSSLTests/IdentityVerificationTest.swift index 74aba136..fedddc2e 100644 --- a/Tests/NIOSSLTests/IdentityVerificationTest.swift +++ b/Tests/NIOSSLTests/IdentityVerificationTest.swift @@ -75,7 +75,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiSanCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "localhost", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -83,7 +84,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiSanCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -91,7 +93,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiSanCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "example.com.", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -99,7 +102,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiSanCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "LoCaLhOsT", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -107,7 +111,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiSanCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "httpbin.org", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertFalse(matched) } @@ -115,7 +120,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiSanCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: nil, socketAddress: try .makeAddressResolvingHost("192.168.0.1", port: 443), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -127,7 +133,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiSanCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: nil, socketAddress: ipv6Address, - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -135,7 +142,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiSanCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: nil, socketAddress: try .makeAddressResolvingHost("192.168.0.2", port: 443), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertFalse(matched) } @@ -146,7 +154,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiSanCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: nil, socketAddress: ipv6Address, - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertFalse(matched) } @@ -154,7 +163,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "this.wildcard.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -162,7 +172,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "foo.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -170,7 +181,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "bar.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -178,7 +190,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "baz.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -186,7 +199,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "trailing.period.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -195,7 +209,8 @@ class IdentityVerificationTest: XCTestCase { XCTAssertThrowsError(try validIdentityForService(serverHostname: "straße.unicode.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert)) { error in + leafCertificate: cert, + includeCommonName: false)) { error in XCTAssertEqual(error as? NIOSSLExtraError, .serverHostnameImpossibleToMatch) XCTAssertEqual(String(describing: error), "NIOSSLExtraError.serverHostnameImpossibleToMatch: The server hostname straße.unicode.example.com cannot be matched due to containing non-DNS characters") @@ -207,7 +222,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "xn--strae-oqa.unicode.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -215,7 +231,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "xn--xx-gia.unicode.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertFalse(matched) } @@ -223,7 +240,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "weirdwildcard.nomatch.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertFalse(matched) } @@ -231,7 +249,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "one.two.double.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertFalse(matched) } @@ -240,7 +259,8 @@ class IdentityVerificationTest: XCTestCase { XCTAssertThrowsError(try validIdentityForService(serverHostname: "foo.straße.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert)) { error in + leafCertificate: cert, + includeCommonName: false)) { error in XCTAssertEqual(error as? NIOSSLExtraError, .serverHostnameImpossibleToMatch) XCTAssertEqual(String(describing: error), "NIOSSLExtraError.serverHostnameImpossibleToMatch: The server hostname foo.straße.example.com cannot be matched due to containing non-DNS characters") @@ -251,7 +271,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "foo.xn--strae-oqa.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertTrue(matched) } @@ -260,7 +281,8 @@ class IdentityVerificationTest: XCTestCase { XCTAssertThrowsError(try validIdentityForService(serverHostname: "nul\u{0000}l.example.com", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert)) { error in + leafCertificate: cert, + includeCommonName: false)) { error in XCTAssertEqual(error as? NIOSSLExtraError, .serverHostnameImpossibleToMatch) XCTAssertEqual(String(describing: error), "NIOSSLExtraError.serverHostnameImpossibleToMatch: The server hostname nul\u{0000}l.example.com cannot be matched due to containing non-DNS characters") @@ -271,7 +293,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiCNCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "localhost", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: true) XCTAssertTrue(matched) } @@ -279,16 +302,36 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(multiCNCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "LoCaLhOsT", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: true) XCTAssertTrue(matched) } + func testDoesNotFallBackToCommonName() throws { + let cert = try NIOSSLCertificate(bytes: .init(multiCNCert.utf8), format: .pem) + let matched = try validIdentityForService(serverHostname: "localhost", + socketAddress: try .init(unixDomainSocketPath: "/path"), + leafCertificate: cert, + includeCommonName: false) + XCTAssertFalse(matched) + } + + func testNoLowercasesForCommonName() throws { + let cert = try NIOSSLCertificate(bytes: .init(multiCNCert.utf8), format: .pem) + let matched = try validIdentityForService(serverHostname: "LoCaLhOsT", + socketAddress: try .init(unixDomainSocketPath: "/path"), + leafCertificate: cert, + includeCommonName: false) + XCTAssertFalse(matched) + } + func testRejectsUnicodeCommonNameWithUnencodedIDNALabel() throws { let cert = try NIOSSLCertificate(bytes: .init(unicodeCNCert.utf8), format: .pem) XCTAssertThrowsError(try validIdentityForService(serverHostname: "straße.org", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert)) { error in + leafCertificate: cert, + includeCommonName: false)) { error in XCTAssertEqual(error as? NIOSSLExtraError, .serverHostnameImpossibleToMatch) XCTAssertEqual(String(describing: error), "NIOSSLExtraError.serverHostnameImpossibleToMatch: The server hostname straße.org cannot be matched due to containing non-DNS characters") @@ -299,7 +342,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(unicodeCNCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "xn--strae-oqa.org", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertFalse(matched) } @@ -307,7 +351,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(noCNCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "localhost", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertFalse(matched) } @@ -315,7 +360,8 @@ class IdentityVerificationTest: XCTestCase { let cert = try NIOSSLCertificate(bytes: .init(weirdoPEMCert.utf8), format: .pem) let matched = try validIdentityForService(serverHostname: "httpbin.org", socketAddress: try .init(unixDomainSocketPath: "/path"), - leafCertificate: cert) + leafCertificate: cert, + includeCommonName: false) XCTAssertFalse(matched) } } diff --git a/Tests/NIOSSLTests/NIOSSLIntegrationTest+XCTest.swift b/Tests/NIOSSLTests/NIOSSLIntegrationTest+XCTest.swift index 94a344b6..b4177c4f 100644 --- a/Tests/NIOSSLTests/NIOSSLIntegrationTest+XCTest.swift +++ b/Tests/NIOSSLTests/NIOSSLIntegrationTest+XCTest.swift @@ -64,6 +64,8 @@ extension NIOSSLIntegrationTest { ("testLoadsOfCloses", testLoadsOfCloses), ("testWriteFromFailureOfWrite", testWriteFromFailureOfWrite), ("testTrustedFirst", testTrustedFirst), + ("testCommonNameFails", testCommonNameFails), + ("testCommonNameSucceedsIfOverrideApplied", testCommonNameSucceedsIfOverrideApplied), ] } } diff --git a/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift b/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift index 3c167b41..e33a5213 100644 --- a/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift +++ b/Tests/NIOSSLTests/NIOSSLIntegrationTest.swift @@ -407,6 +407,8 @@ fileprivate func _clientTLSChannel(context: NIOSSLCon class NIOSSLIntegrationTest: XCTestCase { static var cert: NIOSSLCertificate! static var key: NIOSSLPrivateKey! + static var noSANCert: NIOSSLCertificate! + static var noSANKey: NIOSSLPrivateKey! static var encryptedKeyPath: String! override class func setUp() { @@ -416,6 +418,9 @@ class NIOSSLIntegrationTest: XCTestCase { NIOSSLIntegrationTest.cert = cert NIOSSLIntegrationTest.key = key NIOSSLIntegrationTest.encryptedKeyPath = keyInFile(key: NIOSSLIntegrationTest.key, passphrase: "thisisagreatpassword") + let (noSANCert, noSANKey) = generateSelfSignedCert(withSAN: false) + NIOSSLIntegrationTest.noSANCert = noSANCert + NIOSSLIntegrationTest.noSANKey = noSANKey } override class func tearDown() { @@ -2079,4 +2084,70 @@ class NIOSSLIntegrationTest: XCTestCase { let newBuffer = try completionPromise.futureResult.wait() XCTAssertEqual(newBuffer, originalBuffer) } + + func testCommonNameFails() throws { + let config = TLSConfiguration.forServer(certificateChain: [.certificate(NIOSSLIntegrationTest.noSANCert)], + privateKey: .privateKey(NIOSSLIntegrationTest.noSANKey)) + let context = try assertNoThrowWithValue(NIOSSLContext(configuration: config)) + + let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { + XCTAssertNoThrow(try group.syncShutdownGracefully()) + } + + let serverChannel: Channel = try serverTLSChannel(context: context, handlers: [], group: group) + defer { + XCTAssertNoThrow(try serverChannel.close().wait()) + } + + let handshakeResultPromise = group.next().makePromise(of: Void.self) + let handshakeWatcher = WaitForHandshakeHandler(handshakeResultPromise: handshakeResultPromise) + let clientChannel = try clientTLSChannel(context: try configuredClientContext(), + preHandlers: [], + postHandlers: [handshakeWatcher], + group: group, + connectingTo: serverChannel.localAddress!, + serverHostname: "camembert") + defer { + // Ignore errors here, the channel should be closed already by the time this happens. + try? clientChannel.close().wait() + } + + XCTAssertThrowsError(try handshakeResultPromise.futureResult.wait()) + } + + func testCommonNameSucceedsIfOverrideApplied() throws { + let config = TLSConfiguration.forServer(certificateChain: [.certificate(NIOSSLIntegrationTest.noSANCert)], + privateKey: .privateKey(NIOSSLIntegrationTest.noSANKey)) + let context = try assertNoThrowWithValue(NIOSSLContext(configuration: config)) + + let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { + XCTAssertNoThrow(try group.syncShutdownGracefully()) + } + + let serverChannel: Channel = try serverTLSChannel(context: context, handlers: [], group: group) + defer { + XCTAssertNoThrow(try serverChannel.close().wait()) + } + + let clientConfig = TLSConfiguration.forClient(trustRoots: .certificates([NIOSSLIntegrationTest.noSANCert]), + trustCommonName: true) + let clientContext = try assertNoThrowWithValue(NIOSSLContext(configuration: clientConfig)) + + let handshakeResultPromise = group.next().makePromise(of: Void.self) + let handshakeWatcher = WaitForHandshakeHandler(handshakeResultPromise: handshakeResultPromise) + let clientChannel = try clientTLSChannel(context: clientContext, + preHandlers: [], + postHandlers: [handshakeWatcher], + group: group, + connectingTo: serverChannel.localAddress!, + serverHostname: "camembert") + defer { + // Ignore errors here, the channel should be closed already by the time this happens. + try? clientChannel.close().wait() + } + + XCTAssertNoThrow(try handshakeResultPromise.futureResult.wait()) + } } diff --git a/Tests/NIOSSLTests/NIOSSLTestHelpers.swift b/Tests/NIOSSLTests/NIOSSLTestHelpers.swift index 2ae34c19..455107f6 100644 --- a/Tests/NIOSSLTests/NIOSSLTestHelpers.swift +++ b/Tests/NIOSSLTests/NIOSSLTestHelpers.swift @@ -432,7 +432,7 @@ func addExtension(x509: UnsafeMutablePointer, nid: CInt, value: String) { CNIOBoringSSL_X509_EXTENSION_free(ext) } -func generateSelfSignedCert() -> (NIOSSLCertificate, NIOSSLPrivateKey) { +func generateSelfSignedCert(withSAN: Bool = true) -> (NIOSSLCertificate, NIOSSLPrivateKey) { let pkey = generateRSAPrivateKey() let x = CNIOBoringSSL_X509_new()! CNIOBoringSSL_X509_set_version(x, 2) @@ -456,7 +456,7 @@ func generateSelfSignedCert() -> (NIOSSLCertificate, NIOSSLPrivateKey) { CNIOBoringSSL_X509_set_pubkey(x, pkey) - let commonName = "localhost" + let commonName = "camembert" let name = CNIOBoringSSL_X509_get_subject_name(x) commonName.withCString { (pointer: UnsafePointer) -> Void in pointer.withMemoryRebound(to: UInt8.self, capacity: commonName.lengthOfBytes(using: .utf8)) { (pointer: UnsafePointer) -> Void in @@ -473,8 +473,11 @@ func generateSelfSignedCert() -> (NIOSSLCertificate, NIOSSLPrivateKey) { addExtension(x509: x, nid: NID_basic_constraints, value: "critical,CA:FALSE") addExtension(x509: x, nid: NID_subject_key_identifier, value: "hash") - addExtension(x509: x, nid: NID_subject_alt_name, value: "DNS:localhost") addExtension(x509: x, nid: NID_ext_key_usage, value: "critical,serverAuth,clientAuth") + + if withSAN { + addExtension(x509: x, nid: NID_subject_alt_name, value: "DNS:localhost") + } CNIOBoringSSL_X509_sign(x, pkey, CNIOBoringSSL_EVP_sha256()) diff --git a/Tests/NIOSSLTests/SSLCertificateTest.swift b/Tests/NIOSSLTests/SSLCertificateTest.swift index 34ad0189..433dfb57 100644 --- a/Tests/NIOSSLTests/SSLCertificateTest.swift +++ b/Tests/NIOSSLTests/SSLCertificateTest.swift @@ -352,7 +352,7 @@ class SSLCertificateTest: XCTestCase { } func testCommonNameForGeneratedCert() throws { - XCTAssertEqual([UInt8]("localhost".utf8), SSLCertificateTest.dynamicallyGeneratedCert.commonName()!) + XCTAssertEqual([UInt8]("camembert".utf8), SSLCertificateTest.dynamicallyGeneratedCert.commonName()!) } func testMultipleCommonNames() throws {