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

fix IsMutuallyAuthenticated with NegotiateClientCertificateAsync #88488

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Jul 6, 2023

contributes to #65563 and #75545.

The problem is that with existing code we clean _selectedClientCertificate if the selected certificate was attached to credentials but not used. So later it can be provided via renegotiation or PHA for TLS 1.3 but we already lost the link.

The fix is to keep _selectedClientCertificate untouched once client certificate was selected and used but guard LocalServerCertificate with the extra check that was added by #79898.

@wfurt wfurt added this to the 8.0.0 milestone Jul 6, 2023
@wfurt wfurt requested a review from rzikm July 6, 2023 19:48
@wfurt wfurt self-assigned this Jul 6, 2023
@ghost
Copy link

ghost commented Jul 6, 2023

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

Issue Details

contributes to #65563 and #75545.

The problem is that with existing code we clean _selectedClientCertificate if the selected certificate was attached to credentials but not used. So later it can be provided via renegotiation or PHA for TLS 1.3 but we already lost the link.

The fix is to keep _selectedClientCertificate untouched once client certificate was selected and used but guard LocalServerCertificate with the extra check that was added by #79898.

Author: wfurt
Assignees: wfurt
Labels:

area-System.Net.Security

Milestone: 8.0.0

Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM!

@wfurt wfurt merged commit a41f655 into dotnet:main Jul 10, 2023
@wfurt wfurt deleted the IsAutheNego branch July 10, 2023 17:38
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2023
@rzikm
Copy link
Member

rzikm commented Dec 6, 2023

/backport to 7.0-staging

@github-actions github-actions bot unlocked this conversation Dec 6, 2023
Copy link
Contributor

github-actions bot commented Dec 6, 2023

Started backporting to 7.0-staging: https://github.com/dotnet/runtime/actions/runs/7116625187

Copy link
Contributor

github-actions bot commented Dec 6, 2023

@rzikm an error occurred while backporting to 7.0-staging, please check the run log for details!

The process '/usr/bin/git' failed with exit code 1

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 6, 2023
@rzikm
Copy link
Member

rzikm commented Dec 6, 2023

/backport to release/7.0-staging

@github-actions github-actions bot unlocked this conversation Dec 6, 2023
Copy link
Contributor

github-actions bot commented Dec 6, 2023

Started backporting to release/7.0-staging: https://github.com/dotnet/runtime/actions/runs/7116695721

Copy link
Contributor

github-actions bot commented Dec 6, 2023

@rzikm backporting to release/7.0-staging failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: fix IsMutuallyAuthenticated with NegotiateClientCertificateAsync
Using index info to reconstruct a base tree...
M	src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs
M	src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs
M	src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs
CONFLICT (content): Merge conflict in src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs
Auto-merging src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs
Auto-merging src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs
CONFLICT (content): Merge conflict in src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix IsMutuallyAuthenticated with NegotiateClientCertificateAsync
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

Copy link
Contributor

github-actions bot commented Dec 6, 2023

@rzikm an error occurred while backporting to release/7.0-staging, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants