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

support ServerCertificateContext in quic #53175

Merged
merged 1 commit into from
May 31, 2021
Merged

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented May 24, 2021

This contributes to #49574 and adds support for ServerCertificateContext and fixes certificate chain handling (seems like the certificates come out in revers order that originally expected)
ServerCertificateContext is used primarily by Kestrel as comparing to plain certificate saves repeating operations.
The type does not expose some of the private parts we need so this change uses reflection.
We may keep it as such since this happens only once per server instance or we can decide to expose it via API change later. I wanted to plug the missing functionality and add tests for chain handling in QUIC.

@wfurt wfurt requested a review from a team May 24, 2021 14:58
@wfurt wfurt self-assigned this May 24, 2021
@ghost
Copy link

ghost commented May 24, 2021

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

Issue Details

This contributes to #49574 and adds support for ServerCertificateContext and fixes certificate chain handling (seems like the certificates come out in revers order that originally expected)
ServerCertificateContext is used primarily by Kestrel as comparing to plain certificate saves repeating operations.
The type does not expose some of the private parts we need so this change uses reflection.
We may keep it as such since this happens only once per server instance or we can decide to expose it via API change later. I wanted to plug the missing functionality and add tests for chain handling in QUIC.

Author: wfurt
Assignees: wfurt
Labels:

area-System.Net.Quic

Milestone: -

@@ -121,6 +126,17 @@ private static unsafe SafeMsQuicConfigurationHandle Create(QuicOptions options,
CredentialConfig config = default;
config.Flags = flags; // TODO: consider using LOAD_ASYNCHRONOUS with a callback.

if (certificateContext != null)
{
certificate = (X509Certificate2?) _contextCertificate.GetValue(certificateContext);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it valid for certificate to be reassigned here? if not, can we have some checks (maybe just asserts) to verify it was null?

@wfurt
Copy link
Member Author

wfurt commented May 31, 2021

#53507 logged for the reflection

@wfurt wfurt merged commit 57fc4c2 into dotnet:main May 31, 2021
thaystg added a commit to thaystg/runtime that referenced this pull request Jun 1, 2021
…asm_debugger_and_use_debugger_agent

* upstream/main: (597 commits)
  Fix42292 (dotnet#52463)
  [mono] Remove some obsolete emscripten flags. (dotnet#53486)
  Fixed path to projects (dotnet#53435)
  support ServerCertificateContext in quic (dotnet#53175)
  Socket: delete unix local endpoint filename on Close (dotnet#52103)
  [mono] Fix sgen_gc_info.memory_load_bytes (dotnet#53364)
  Refactor MsQuic's native IP address types. (dotnet#53461)
  Re-enabled optimizations for gtFoldExprConst (dotnet#53347)
  Add diagnostic support into sample app and AppBuilders on Mono. (dotnet#53361)
  Fix issues with virtuals and mdarrays (dotnet#53400)
  Split Variant marshalling tests (dotnet#53035)
  Update clrjit.natvis to cover GT_SIMD and GT_HWINTRINSIC (dotnet#53470)
  remove WSL checks in tests (dotnet#53475)
  Always spawn message loop thread for SystemEvents (dotnet#53467)
  add AcceptAsync cancellation overloads (dotnet#53340)
  Remove unnecessary reference to iOS workload pack in the Mono workload (dotnet#53425)
  Add CookieContainer.GetAllCookies (dotnet#53441)
  Remove DynamicallyAccessedMembers on JsonSerializer  (dotnet#53235)
  [wasm] Re-enable Wasm.Build.Tests (dotnet#53433)
  [libraries] Move library tests Feature Switches defaults to Functional tests (dotnet#53253)
  ...
@ghost ghost locked as resolved and limited conversation to collaborators Jun 30, 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.

3 participants