-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Improve compat for low rights users loading CAPI machine keys via PFX #119124
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
Conversation
1) Windows exhibits different behaviors between no provider attribute with PKCS12_PREFER_CNG_KSP than with specifying the KSP, so always specify the KSP. 2) Let's stop messing with provider attributes on non-Windows, since this change is making them more expensive. 3) If PreserveStorageProvider is false, the key would go to a machine store, the user can't write to the CNG machine store, and the key is from a known CAPI provider, don't upgrade it to CNG.
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
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.
Pull Request Overview
This PR improves compatibility for low-privilege users loading CAPI machine keys via PFX files on Windows by modifying the cryptographic provider selection logic. The changes address permission issues where users cannot write to CNG machine stores but can write to CAPI machine stores.
Key changes:
- Remove default PKCS12_PREFER_CNG_KSP flag initialization and let Windows handle provider selection naturally
- Add intelligent provider selection logic that detects CAPI providers and user permissions
- Restrict provider attribute processing to Windows-only platforms for performance
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| X509CertificateLoader.Windows.cs | Removes default CNG preference flag to allow natural Windows provider selection |
| X509CertificateLoader.Pkcs12.cs | Adds comprehensive provider selection logic, permission checking, and Windows-specific attribute processing |
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Outdated
Show resolved
Hide resolved
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Show resolved
Hide resolved
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Outdated
Show resolved
Hide resolved
|
Tested with {Default, User, Machine, User|Machine} x {0, Exportable} with all RSA type providers. The low rights user fails for MS_DEFAULT_PROV because it prohibits RSA "Exchange" keys over 1024-bit. (Windows says Access Denied, not us) Admin, with change: DetailsLow-rights user, with change: DetailsAdmin user, 10-p7: DetailsLow-rights user, 10-p7: DetailsSummary: Low rights machine key imports end up in MS_ENH_RSA_AES_PROV, rather than failing. |
vcsjones
left a comment
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.
Is there any way we can add test coverage for this?
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Outdated
Show resolved
Hide resolved
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Outdated
Show resolved
Hide resolved
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Outdated
Show resolved
Hide resolved
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Show resolved
Hide resolved
Not reliably for our CI, I don't think. We could add a test that imports a MS_STRONG_PROV key with MachineKeySet and see that it succeeds, and that the only two acceptable providers are MS_KSP and PROV_RSA_AES_ENH... but I don't know that we have a way to make CI run a test as both an admin and an unprivileged user. (If you do, I'm happy to learn) |
Not really. In theory it might be possible to teach RemoteExecutor how to do it by creating a process with |
|
We also have a few other tests conditioned on "IsElevated" or something like that. Some of the CNG tests have conditions for that for machine key writing. So we could write a test conditioned on that, but I don't know the odds that it will actually run in CI. |
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Outdated
Show resolved
Hide resolved
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Outdated
Show resolved
Hide resolved
I added a test that shows that the expected provider is returned in all 16 cases. Technically there are 32 cases (all the ones I added, but then again without a key having the machine key attribute), but I declare equivalency. The PfxTests class passes as a low-rights user, meaning the new test passes as a low rights user. |
...ies/Common/src/System/Security/Cryptography/X509Certificates/X509CertificateLoader.Pkcs12.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/tests/X509Certificates/PfxTests.cs
Outdated
Show resolved
Hide resolved
vcsjones
left a comment
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.
Looks good minus some nonblocking suggestions.
vcsjones
left a comment
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.
Now I got nothing.
|
/backport to release/10.0 |
|
Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/17304209626 |
Windows exhibits different behaviors between no provider attribute with PKCS12_PREFER_CNG_KSP than with specifying the KSP, so always specify the KSP instead of deleting the attribute.
Let's stop messing with provider attributes on non-Windows, since this change is making them more expensive.
If PreserveStorageProvider is false, the key would go to a machine store, the user can't write to the CNG machine store, and the key is from a known CAPI provider, don't upgrade it to CNG.
Fixes #117798.