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.

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

Conversation

borisreitman
Copy link

@borisreitman borisreitman commented Dec 22, 2022

I have implemented a new attribute .certificateRequired in TLSConfiguration. When set to false, it makes presence of client certificates optional. If the client certificate is provided, then the customVerificationCallback will be called. By default, the attribute is true, which is the current behaviour (failure without client certificate).

From an article referenced below, here is a quote:

"[We] tell it to accept requests with no valid certificate. We need this to handle invalid connections as well (for example to display an error message), otherwise, they would just get a cryptic HTTPS error message from the browser (ERR_BAD_SSL_CLIENT_AUTH_CERT to be precise)"

Reference: https://medium.com/@sevcsik/authentication-using-https-client-certificates-3c9d270e8326

@swift-server-bot
Copy link

Can one of the admins verify this patch?

9 similar comments
@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@borisreitman
Copy link
Author

@dnadoba Can you check it again, I pushed a new changeset.

@borisreitman borisreitman changed the title .noHostnameNoPeerVerification TLSConfiguration.certificateRequired attribute Dec 23, 2022
@@ -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?

@Lukasa
Copy link
Contributor

Lukasa commented Mar 21, 2023

@swift-server-bot test this please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants