[release/9.0] Permit unencrypted key exports from CNG #109134
Merged
+471
−66
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
CNG, by default, loads PKCS#12 certificate private keys as "AllowExport", not "AllowsPlaintextExport". When users attempt to export the private key from a loaded PKCS#12, they will receive an error that the operation is not permitted because they are expected to perform an encrypted export. This is counter-intuitive to some people, as the general expectation is that they can export private keys they just loaded. Starting in .NET 9, we are loading more PKCS#12 private keys in CNG instead of the legacy CSP, meaning users will hit this problem more. This is also a regression from .NET 8. The default provider changed, meaning keys that were once exportable no longer are. This pull request makes a change similar to what we do for macOS. If a user asks for an unencrypted export of the private key, and the key does not permit that, we will ask CNG for an encrypted export of the private key and decrypt it for them. This makes the unencrypted exports "just work", as they do on other platforms.
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
bartonjs
added
the
cryptographic-docs-impact
bartonjs
approved these changes
Oct 23, 2024
jeffhandley
approved these changes
Oct 23, 2024
jeffhandley
added
Servicing-approved
bartonjs
added
the
tracking
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #109119 to release/9.0
/cc @vcsjones
Customer Impact
Reported by a customer at #109059.
Customers that upgrade to .NET 9 may receive a
CryptographicException: The requested operation is not supported.exception when exporting a private key from a certificate on Windows.This is because certificate loading on .NET 9 changed to prefer using CNG instead of CSP for the private key storage when no storage engine is specified in the PKCS#12/PFX. When a PKCS#12 is imported into the CNG provider, it is set to only permit encrypted exports. Most of the .NET APIs that expose the private key are not encrypted, so the private key is effectively not exportable.
Regression
Yes. This regression was introduced by #107005.
Testing
Customer provided steps to reproduce which allowed us to add thorough unit tests. The tests are added in this pull request to prevent future regressions. Existing tests cover existing scenarios, and this is a generally well-tested area.
Risk
Process: Medium
CredScan or other release validation tooling might get angry at the particulars of how we're doing it. We have tried to run the tools on this change already, and they passed; but that doesn't mean we won't get "surprised" post-merge.
Functional: Low
Tests say that existing scenarios where the export routines work (as of 9 RC2) still work, and the new tests say that the PFX-based ones work, too. So functional regression risk is low.
The worst abstract risk is that the if is wrong and we run all exports through the complicated path when the simple one would work. That makes the routines slower, but they still function.