Skip to content

Conversation

@vcsjones
Copy link
Member

@vcsjones vcsjones commented Feb 20, 2025

Older Windowses do not encrypt certificates in PKCS#12 exports, only keys. This change relaxes the counting of certificates in unit tests to include unencrypted certificates on Windows.

Contributes to #112738

Copilot AI review requested due to automatic review settings February 20, 2025 15:01
@ghost ghost added the area-System.Security label Feb 20, 2025
Copy link
Contributor

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.

PR Overview

This PR relaxes the PKCS#12 export unit test requirements by allowing unencrypted certificates on Windows, addressing issues present on older Windows versions.

  • Updated the condition for counting certificates in Pkcs12CertBag to include unencrypted certificates when running on Windows.

Reviewed Changes

File Description
src/libraries/System.Security.Cryptography/tests/X509Certificates/ExportTests.cs Updated certificate counting logic for Windows in PKCS#12 export tests

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

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

@dotnet-policy-service
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.

@bartonjs
Copy link
Member

That must have been a weird intermediate state in Windows 10. Since at least XP the behavior was two top-level collections: an encrypted one that contained certs, and an unencrypted one that contained shrouded keys.

When I was experimenting with PFXes to justify writing a custom loader for macOS (.NET Core 3? 2? 5?), macOS wouldn't even read certs out of an unencrypted safe (or whatever the root things are really called) (or unshrouded keys, regardless of the state of the safe). Perhaps that's why Windows flipped back :)

@vcsjones vcsjones merged commit 306ced2 into dotnet:main Feb 20, 2025
80 of 83 checks passed
@vcsjones vcsjones deleted the fix-112738 branch February 20, 2025 22:01
@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2025
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.

2 participants