Skip to content

Commit 76b2883

Browse files
Lukasaweissi
authored andcommitted
Set SSL_VERIFY_FAIL_IF_NO_PEER_CERT. (#107)
Motivation: When server users turn on certificate verification, they usually expect that they'll actually require the certificate from their peer. Sadly, this is not the BoringSSL default behaviour. Modifications: Set `SSL_VERIFY_FAIL_IF_NO_PEER_CERT` when requesting certificate validation. Result: Better certificate validation.
1 parent c46e701 commit 76b2883

File tree

3 files changed

+25
-1
lines changed

3 files changed

+25
-1
lines changed

Sources/NIOSSL/SSLContext.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ extension NIOSSLContext {
345345
// If validation is turned on, set the trust roots and turn on cert validation.
346346
switch verification {
347347
case .fullVerification, .noHostnameVerification:
348-
CNIOBoringSSL_SSL_CTX_set_verify(context, SSL_VERIFY_PEER, nil)
348+
CNIOBoringSSL_SSL_CTX_set_verify(context, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nil)
349349

350350
switch trustRoots {
351351
case .some(.default), .none:

Tests/NIOSSLTests/TLSConfigurationTest+XCTest.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ extension TLSConfigurationTest {
3232
("testServerCannotValidateClientPreTLS13", testServerCannotValidateClientPreTLS13),
3333
("testServerCannotValidateClientPostTLS13", testServerCannotValidateClientPostTLS13),
3434
("testMutualValidation", testMutualValidation),
35+
("testMutualValidationRequiresClientCertificatePreTLS13", testMutualValidationRequiresClientCertificatePreTLS13),
36+
("testMutualValidationRequiresClientCertificatePostTLS13", testMutualValidationRequiresClientCertificatePostTLS13),
3537
("testNonexistentFileObject", testNonexistentFileObject),
3638
("testComputedApplicationProtocols", testComputedApplicationProtocols),
3739
]

Tests/NIOSSLTests/TLSConfigurationTest.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,28 @@ class TLSConfigurationTest: XCTestCase {
246246
try flushFuture.wait()
247247
}
248248

249+
func testMutualValidationRequiresClientCertificatePreTLS13() throws {
250+
let clientConfig = TLSConfiguration.forClient(maximumTLSVersion: .tlsv12, certificateVerification: .none)
251+
let serverConfig = TLSConfiguration.forServer(certificateChain: [.certificate(TLSConfigurationTest.cert1)],
252+
privateKey: .privateKey(TLSConfigurationTest.key1),
253+
maximumTLSVersion: .tlsv12,
254+
certificateVerification: .noHostnameVerification,
255+
trustRoots: .certificates([TLSConfigurationTest.cert2]))
256+
257+
try assertHandshakeError(withClientConfig: clientConfig, andServerConfig: serverConfig, errorTextContainsAnyOf: ["ALERT_HANDSHAKE_FAILURE"])
258+
}
259+
260+
func testMutualValidationRequiresClientCertificatePostTLS13() throws {
261+
let clientConfig = TLSConfiguration.forClient(minimumTLSVersion: .tlsv13, certificateVerification: .none)
262+
let serverConfig = TLSConfiguration.forServer(certificateChain: [.certificate(TLSConfigurationTest.cert1)],
263+
privateKey: .privateKey(TLSConfigurationTest.key1),
264+
minimumTLSVersion: .tlsv13,
265+
certificateVerification: .noHostnameVerification,
266+
trustRoots: .certificates([TLSConfigurationTest.cert2]))
267+
268+
try assertPostHandshakeError(withClientConfig: clientConfig, andServerConfig: serverConfig, errorTextContainsAnyOf: ["CERTIFICATE_REQUIRED"])
269+
}
270+
249271
func testNonexistentFileObject() throws {
250272
let clientConfig = TLSConfiguration.forClient(trustRoots: .file("/thispathbetternotexist/bogus.foo"))
251273

0 commit comments

Comments
 (0)