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

allow late certificate with disabled renegotiation #53719

Merged
merged 1 commit into from
Jun 7, 2021

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Jun 4, 2021

Motivation: In the past we did not have API to trigger renegotiation so the AllowRenegotiation property really controlled behavior when renegotiation was requested by remote peer. HTTP/2 explicitly prohibits it so Kestrel would set it to false any time when HTTP/2 is possibility. However there is no way how to turn it back on when HTTP/1 is negotiated. That can break the late certificate per specific URL even if HTTP/1 is negotiated.
With this change, we would allow renegotiation if explicitly requested on server side using NegotiateClientCertificateAsync(). Note, that this is only during that particular call e.g. when NegotiateClientCertificateAsync is finished we would again respect the properly and block renegotiation requested by peer as we used to. That should make intro with HTTP/2 easier. Since SslStream does not really care about application protocols it is up to the caller to decide if this is appropriate e.g. if HTTP/2 is used or not. (TLS RFC does not care)

contributes to #49346

cc: @Tratcher

@wfurt wfurt requested review from aik-jahoda and a team June 4, 2021 10:05
@wfurt wfurt self-assigned this Jun 4, 2021
@ghost
Copy link

ghost commented Jun 4, 2021

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

Issue Details

Motivation: In the past we did not have API to trigger renegotiation so the AllowRenegotiation property really controlled behavior when renegotiation was requested by remote peer. HTTP/2 explicitly prohibits it so Kestrel would set it to false any time when HTTP/2 is possibility. However there is no way how to turn it back on when HTTP/1 is negotiated. That can break the late certificate per specific URL even if HTTP/1 is negotiated.
With this change, we would allow renegotiation if explicitly requested on server side using NegotiateClientCertificateAsync(). Note, that this is only during that particular call e.g. when NegotiateClientCertificateAsync is finished we would again respect the properly and block renegotiation requested by peer as we used to. That should make intro with HTTP/2 easier. Since SslStream does not really care about application protocols it is up to the caller to decide if this is appropriate e.g. if HTTP/2 is used or not. (TLS RFC does not care)

contributes to #49346

cc: @Tratcher

Author: wfurt
Assignees: wfurt
Labels:

area-System.Net.Security

Milestone: -

Copy link
Contributor

@aik-jahoda aik-jahoda left a comment

Choose a reason for hiding this comment

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

LGTM

@Tratcher
Copy link
Member

Tratcher commented Jun 4, 2021

Thanks!

@wfurt wfurt merged commit eaa3dff into dotnet:main Jun 7, 2021
@wfurt wfurt deleted the renego_49346 branch June 7, 2021 09:54
@ghost ghost locked as resolved and limited conversation to collaborators Jul 7, 2021
@karelz karelz added this to the 6.0.0 milestone Jul 15, 2021
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.

None yet

4 participants