Skip to content

Commit 94d63e7

Browse files
authored
Actually disable hostname validation on Apple platforms when asked (#502)
Motivation: swift-nio-ssl has long had a flag to disable its own certificate validation behaviour. This flag works fine, and had the desired effect. However, this flag did not take account of the fact that Security.framework _also_ validates the hostname. Due to an implementation bug, setting .noHostnameVerification disabled only one of the two checks. Modifications: - Correctly check the flag before passing a hostname to Security.framework - Write some tests to make sure the behaviour works in all modes. Result: Better, more consistent behaviour.
1 parent 0c49639 commit 94d63e7

File tree

2 files changed

+65
-1
lines changed

2 files changed

+65
-1
lines changed

Sources/NIOSSL/SecurityFrameworkCertificateVerification.swift

+3-1
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,12 @@ extension SSLConnection {
3030
preconditionFailure("This callback should only be used if we are using the system-default trust.")
3131
}
3232

33+
let expectedHostname = self.validateHostnames ? self.expectedHostname : nil
34+
3335
// This force-unwrap is safe as we must have decided if we're a client or a server before validation.
3436
var trust: SecTrust? = nil
3537
var result: OSStatus
36-
let policy = SecPolicyCreateSSL(self.role! == .client, self.expectedHostname as CFString?)
38+
let policy = SecPolicyCreateSSL(self.role! == .client, expectedHostname as CFString?)
3739
result = SecTrustCreateWithCertificates(peerCertificates as CFArray, policy, &trust)
3840
guard result == errSecSuccess, let actualTrust = trust else {
3941
throw NIOSSLError.unableToValidateCertificate

Tests/NIOSSLTests/SecurityFrameworkVerificationTests.swift

+62
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,68 @@ final class SecurityFrameworkVerificationTests: XCTestCase {
7373
#endif
7474
}
7575

76+
func testDefaultVerificationCanValidateHostname() throws {
77+
#if canImport(Darwin)
78+
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
79+
defer {
80+
try! group.syncShutdownGracefully()
81+
}
82+
83+
let p = group.next().makePromise(of: NIOSSLVerificationResult.self)
84+
let context = try NIOSSLContext(configuration: .makeClientConfiguration())
85+
let connection = context.createConnection()!
86+
connection.setConnectState()
87+
connection.expectedHostname = "www.apple.com"
88+
89+
connection.performSecurityFrameworkValidation(promise: p, peerCertificates: Self.appleComCertChain)
90+
let result = try p.futureResult.wait()
91+
92+
XCTAssertEqual(result, .certificateVerified)
93+
#endif
94+
}
95+
96+
func testDefaultVerificationFailsOnInvalidHostname() throws {
97+
#if canImport(Darwin)
98+
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
99+
defer {
100+
try! group.syncShutdownGracefully()
101+
}
102+
103+
let p = group.next().makePromise(of: NIOSSLVerificationResult.self)
104+
let context = try NIOSSLContext(configuration: .makeClientConfiguration())
105+
let connection = context.createConnection()!
106+
connection.setConnectState()
107+
connection.expectedHostname = "www.swift-nio.io"
108+
109+
connection.performSecurityFrameworkValidation(promise: p, peerCertificates: Self.appleComCertChain)
110+
let result = try p.futureResult.wait()
111+
112+
XCTAssertEqual(result, .failed)
113+
#endif
114+
}
115+
116+
func testDefaultVerificationIgnoresHostnamesWhenConfiguredTo() throws {
117+
#if canImport(Darwin)
118+
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
119+
defer {
120+
try! group.syncShutdownGracefully()
121+
}
122+
123+
let p = group.next().makePromise(of: NIOSSLVerificationResult.self)
124+
var configuration = TLSConfiguration.makeClientConfiguration()
125+
configuration.certificateVerification = .noHostnameVerification
126+
let context = try NIOSSLContext(configuration: configuration)
127+
let connection = context.createConnection()!
128+
connection.setConnectState()
129+
connection.expectedHostname = "www.swift-nio.io"
130+
131+
connection.performSecurityFrameworkValidation(promise: p, peerCertificates: Self.appleComCertChain)
132+
let result = try p.futureResult.wait()
133+
134+
XCTAssertEqual(result, .certificateVerified)
135+
#endif
136+
}
137+
76138
func testDefaultVerificationPlusAdditionalCanUseAdditionalRoot() throws {
77139
#if canImport(Darwin)
78140
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)

0 commit comments

Comments
 (0)