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

add basic support for client certificate to Quic #54302

Merged
merged 3 commits into from
Jul 9, 2021

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Jun 16, 2021

This change as few separate parts:

  1. We pay attention to ServerAuthenticationOptions.ClientCertificateRequired and we pass it to Quic as QUIC_CREDENTIAL_FLAG_REQUIRE_CLIENT_AUTHENTICATION.
    That has still two problems: It is ignored on Linux as well as on SslStream that is optional e.g. if client does not provide certificate it is possible to ignore that via custom callback but quick terminates the connection if client does not have cert.
    Contributes to HTTP/3: Support SslServerAuthenticationOptions with QUIC #49574

  2. If client certificate is specified we would pass it to MsQuic on Windows.
    contributes to [QUIC] Use client TLS options in QUIC #32069

Once again, Linux is not supported yet.
SslStream has two mechanisms: Either once can pass it via collection of certificates and (over?)complicated process would select once candidate or this could be done via certificate selection callback.
I'm not sure if we could/should replicate the logic for QUIC. For now, we would simply pick first eligible certificate (e.g. X509Certificate2 with key). Feasibility of selecting certificate dynamically is till to be seen.

  1. There is problem with passing ServerAuthenticationOptions to new connection on server.
    One can specify ServerAuthenticationOptions on QuicListener. But by the time new connection is accepted, the TLS handshake is already done and there is no good way how to pass options to it.
    To get ClientCertificte working, this PR will copy part of the options from Listener to new MsQuicConnection to inherit validation session. As part of that, we need Connection in state so we can lookup properties when validation runs. We would reset it afterwards to preserve current relation.

  2. I wanted to do basic validation that the streams are useable as well as keep some of the objects alive to prevent premature disposal.
    I added helper test function to exchange simple message to validate that data can be sent and received.

cc: @nibanks @ThadHouse

@wfurt wfurt self-assigned this Jun 16, 2021
@ghost
Copy link

ghost commented Jun 16, 2021

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

Issue Details

This change as few separate parts:

  1. We pay attention to ServerAuthenticationOptions.ClientCertificateRequired and we pass it to Quic as QUIC_CREDENTIAL_FLAG_REQUIRE_CLIENT_AUTHENTICATION.
    That has still two problems: It is ignored on Linux as well as on SslStream that is optional e.g. if client does not provide certificate it is possible to ignore that via custom callback but quick terminates the connection if client does not have cert.
    Contributes to HTTP/3: Support SslServerAuthenticationOptions with QUIC #49574

  2. If client certificate is specified we would pass it to MsQuic on Windows.
    contributes to [QUIC] Use client TLS options in QUIC #32069

Once again, Linux is not supported yet.
SslStream has two mechanisms: Either once can pass it via collection of certificates and (over?)complicated process would select once candidate or this could be done via certificate selection callback.
I'm not sure if we could/should replicate the logic for QUIC. For now, we would simply pick first eligible certificate (e.g. X509Certificate2 with key). Feasibility of selecting certificate dynamically is till to be seen.

  1. There is problem with passing ServerAuthenticationOptions to new connection on server.
    One can specify ServerAuthenticationOptions on QuicListener. But by the time new connection is accepted, the TLS handshake is already done and there is no good way how to pass options to it.
    To get ClientCertificte working, this PR will copy part of the options from Listener to new MsQuicConnection to inherit validation session. As part of that, we need Connection in state so we can lookup properties when validation runs. We would reset it afterwards to preserve current relation.

  2. I wanted to do basic validation that the streams are useable as well as keep some of the objects alive to prevent premature disposal.
    I added helper test function to exchange simple message to validate that data can be sent and received.

cc: @nibanks @ThadHouse

Author: wfurt
Assignees: wfurt
Labels:

area-System.Net.Quic

Milestone: -

@wfurt
Copy link
Member Author

wfurt commented Jul 7, 2021

Since we agreed Dispose on stream should do the right thing, I think this should be ready for another review.

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

Small comment, otherwise LGTM. Thanks!

@wfurt wfurt merged commit 17859ac into dotnet:main Jul 9, 2021
@karelz karelz added this to the 6.0.0 milestone Jul 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 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.

4 participants