Skip to content

Add SslClientAuthenticationOptions.ClientCertificateContext - #80182

Merged
rzikm merged 16 commits into
dotnet:mainfrom
rzikm:71194-add-SslStreamCertificateContext-to-SslClientAuthenticationOptions
Jan 9, 2023
Merged

Add SslClientAuthenticationOptions.ClientCertificateContext#80182
rzikm merged 16 commits into
dotnet:mainfrom
rzikm:71194-add-SslStreamCertificateContext-to-SslClientAuthenticationOptions

Conversation

@rzikm

@rzikm rzikm commented Jan 4, 2023

Copy link
Copy Markdown
Member

Closes #71194.

We already convert certs from ClientCertificates and result of the LocalCertificateSelectionCallback to the cert context, so the only necessary thing was to make sure we don't overwrite the user-supplied context.

@ghost

ghost commented Jan 4, 2023

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost ghost assigned rzikm Jan 4, 2023
@ghost

ghost commented Jan 4, 2023

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/ncl, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Closes #71194.

We already convert certs from ClientCertificates and result of the LocalCertificateSelectionCallback to the cert context, so the only necessary thing was to make sure we don't overwrite the user-supplied context.

Author: rzikm
Assignees: -
Labels:

area-System.Net.Security, new-api-needs-documentation

Milestone: -
Comment thread src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs Outdated

@wfurt wfurt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice!

@wfurt

wfurt commented Jan 4, 2023

Copy link
Copy Markdown
Member

There are many tests disabled with note about "Self-signed certificates" on Android. Can we perhaps use other untrusted certificates @simonrozsival ? Most of the tests do not depend on the self-signed part.

@simonrozsival

Copy link
Copy Markdown
Member

@wfurt at the moment we can't bypass Android's internal validation of certificates that currently rejects any untrusted certificates. Once #77386 is merged, Android will behave a lot like the other platforms.

@wfurt wfurt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@rzikm

rzikm commented Jan 6, 2023

Copy link
Copy Markdown
Member Author

Test failures are unrelated

@rzikm
rzikm requested a review from wfurt January 6, 2023 19:24
@rzikm
rzikm merged commit cd5bbab into dotnet:main Jan 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 8, 2023
@karelz karelz added this to the 8.0.0 milestone Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.