Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLSConfiguration.certificateRequired attribute #413

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

TLSConfiguration.certificateRequired attribute #413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
77224fb
noHostnameNoPeerVerification
Dec 22, 2022
17b7d7e
Refactor to have no new enum case
Dec 22, 2022
e42c2a3
Remove extra space
Dec 22, 2022
1d8c1d8
Merge branch 'main' into contrib
borisreitman Mar 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions Sources/NIOSSL/SSLContext.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -566,11 +567,12 @@ 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:
CNIOBoringSSL_SSL_CTX_set_verify(context, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nil)
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
// untrusted root certs. X509_VERIFY_PARAM will or-in the flags, so we don't need to load them first.
Expand Down
1 change: 1 addition & 0 deletions Sources/NIOSSL/TLSConfiguration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ public struct TLSConfiguration {

/// Whether to verify remote certificates.
public var certificateVerification: CertificateVerification
public var certificateRequired: Bool = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a doc comment to this explaining what it does and how it interacts with the above?


/// The trust roots to use to validate certificates. This only needs to be provided if you intend to validate
/// certificates.
Expand Down
34 changes: 34 additions & 0 deletions Tests/NIOSSLTests/TLSConfigurationTest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,40 @@ 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 = .noHostnameVerification
serverConfig.certificateRequired = false
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 = .noHostnameVerification
serverConfig.certificateRequired = false
serverConfig.trustRoots = .certificates([TLSConfigurationTest.cert2])

try assertHandshakeSucceeded(withClientConfig: clientConfig, andServerConfig: serverConfig)
}

func testIncompatibleSignatures() throws {
var clientConfig = TLSConfiguration.makeClientConfiguration()
Expand Down