-
Notifications
You must be signed in to change notification settings - Fork 5k
[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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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)
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
@@ -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; |
There was a problem hiding this comment.
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)
Related to #114640