Skip to content

[NOREVIEW] Test support for certificates with ephemeral keys #114767

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rzikm
Copy link
Member

@rzikm rzikm commented Apr 17, 2025

Related to #114640

@Copilot Copilot AI review requested due to automatic review settings April 17, 2025 09:11
@rzikm rzikm changed the title [NOREVIEW] Testsupport for certificates with ephemeral keys [NOREVIEW] Test support for certificates with ephemeral keys Apr 17, 2025
@rzikm rzikm marked this pull request as draft April 17, 2025 09:11
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Windows.cs:303

  • Confirm that combining both PKCS12_NAMED_NO_PERSIST_KEY and PKCS12_NO_PERSIST_KEY is intentional and that their combined effect does not lead to unintended behavior.
pfxCertStoreFlags |= Interop.Crypt32.PfxCertStoreFlags.PKCS12_NAMED_NO_PERSIST_KEY | Interop.Crypt32.PfxCertStoreFlags.PKCS12_NO_PERSIST_KEY | Interop.Crypt32.PfxCertStoreFlags.PKCS12_ALWAYS_CNG_KSP;

src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs:955

  • Verify that the new test behavior—expecting successful authentication rather than an exception—fully covers the intended ephemeral key handling scenarios.
public async Task SslStream_EphemeralKey_DoesNotThrow()

src/libraries/Common/tests/System/Net/Configuration.Certificates.Dynamic.cs:165

  • Re-assess the logic change that now re-imports certificates on Windows irrespective of the ephemeralKey flag, ensuring that both ephemeral and non-ephemeral scenarios are handled correctly.
if (PlatformDetection.IsWindows)

Copy link
Contributor

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

@@ -300,7 +300,7 @@ private static Interop.Crypt32.PfxCertStoreFlags MapKeyStorageFlags(X509KeyStora
if ((keyStorageFlags & X509KeyStorageFlags.EphemeralKeySet) == X509KeyStorageFlags.EphemeralKeySet)
{
pfxCertStoreFlags &= ~Interop.Crypt32.PfxCertStoreFlags.PKCS12_PREFER_CNG_KSP;
pfxCertStoreFlags |= Interop.Crypt32.PfxCertStoreFlags.PKCS12_NO_PERSIST_KEY | Interop.Crypt32.PfxCertStoreFlags.PKCS12_ALWAYS_CNG_KSP;
pfxCertStoreFlags |= Interop.Crypt32.PfxCertStoreFlags.PKCS12_NAMED_NO_PERSIST_KEY | Interop.Crypt32.PfxCertStoreFlags.PKCS12_NO_PERSIST_KEY | Interop.Crypt32.PfxCertStoreFlags.PKCS12_ALWAYS_CNG_KSP;
Copy link
Member

Choose a reason for hiding this comment

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

I didn't get clarity from Windows on if it would be bad to do this "generally". It also doesn't solve the problem of loading the key from file and using CopyWithPrivateKey, still requires routing through PFX import/export (or doing something similar for the other path... but I haven't seen how this flag applies for non-PFX)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants