Skip to content

Commit

Permalink
Actually disable hostname validation on Apple platforms when asked (#502
Browse files Browse the repository at this point in the history
)

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.
  • Loading branch information
Lukasa authored Jan 3, 2025
1 parent 0c49639 commit 94d63e7
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ extension SSLConnection {
preconditionFailure("This callback should only be used if we are using the system-default trust.")
}

let expectedHostname = self.validateHostnames ? self.expectedHostname : nil

// This force-unwrap is safe as we must have decided if we're a client or a server before validation.
var trust: SecTrust? = nil
var result: OSStatus
let policy = SecPolicyCreateSSL(self.role! == .client, self.expectedHostname as CFString?)
let policy = SecPolicyCreateSSL(self.role! == .client, expectedHostname as CFString?)
result = SecTrustCreateWithCertificates(peerCertificates as CFArray, policy, &trust)
guard result == errSecSuccess, let actualTrust = trust else {
throw NIOSSLError.unableToValidateCertificate
Expand Down
62 changes: 62 additions & 0 deletions Tests/NIOSSLTests/SecurityFrameworkVerificationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,68 @@ final class SecurityFrameworkVerificationTests: XCTestCase {
#endif
}

func testDefaultVerificationCanValidateHostname() throws {
#if canImport(Darwin)
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
try! group.syncShutdownGracefully()
}

let p = group.next().makePromise(of: NIOSSLVerificationResult.self)
let context = try NIOSSLContext(configuration: .makeClientConfiguration())
let connection = context.createConnection()!
connection.setConnectState()
connection.expectedHostname = "www.apple.com"

connection.performSecurityFrameworkValidation(promise: p, peerCertificates: Self.appleComCertChain)
let result = try p.futureResult.wait()

XCTAssertEqual(result, .certificateVerified)
#endif
}

func testDefaultVerificationFailsOnInvalidHostname() throws {
#if canImport(Darwin)
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
try! group.syncShutdownGracefully()
}

let p = group.next().makePromise(of: NIOSSLVerificationResult.self)
let context = try NIOSSLContext(configuration: .makeClientConfiguration())
let connection = context.createConnection()!
connection.setConnectState()
connection.expectedHostname = "www.swift-nio.io"

connection.performSecurityFrameworkValidation(promise: p, peerCertificates: Self.appleComCertChain)
let result = try p.futureResult.wait()

XCTAssertEqual(result, .failed)
#endif
}

func testDefaultVerificationIgnoresHostnamesWhenConfiguredTo() throws {
#if canImport(Darwin)
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
try! group.syncShutdownGracefully()
}

let p = group.next().makePromise(of: NIOSSLVerificationResult.self)
var configuration = TLSConfiguration.makeClientConfiguration()
configuration.certificateVerification = .noHostnameVerification
let context = try NIOSSLContext(configuration: configuration)
let connection = context.createConnection()!
connection.setConnectState()
connection.expectedHostname = "www.swift-nio.io"

connection.performSecurityFrameworkValidation(promise: p, peerCertificates: Self.appleComCertChain)
let result = try p.futureResult.wait()

XCTAssertEqual(result, .certificateVerified)
#endif
}

func testDefaultVerificationPlusAdditionalCanUseAdditionalRoot() throws {
#if canImport(Darwin)
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
Expand Down

0 comments on commit 94d63e7

Please sign in to comment.