Skip to content
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

Implement PKCS12 Export in terms of Pkcs12Builder for non-Windows #111823

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

vcsjones
Copy link
Member

Currently, the UnixExportProvider is implemented by putting together ASN.1 data in a bit of an ad-hoc fashion using the serializers.

In order to support #80314 and #111560, we will soon want to offer the capability to export in PKCS12 with different configurations for PBES, like PBES2-AES256-SHA256.

Rather than try to tack on PBES2, PBKDF2, etc in to the ad-hoc encoder, this pulls in Pkcs12Builder in to System.Security.Cryptography. Because it is currently in System.Security.Cryptography.Pkcs, we can't use it directly because it is a package, and even if we ship the package, we have a circular reference problem.

To address that, this puts Pkcs12Builder and its dependencies in Common. This is, essentially:

  1. Move the files. We use the existing BUILDING_PKCS define to change the visibility. If we are building the public API, its visibility is public, otherwise it is internal.
  2. PkcsHelpers needed to be split.
  3. Some things in SignedCms needed to be moved to PkcsHelpers because we don't want to pull in SignedCms.

Contributes to #80314.

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.

#if BUILDING_PKCS
public
#else
#pragma warning disable CA1510, CA1512
Copy link
Member Author

Choose a reason for hiding this comment

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

Why do we need to suppress these if we are in System.Security.Cryptography?

Because S.S.C.Pkcs targets netstandard, the analyzer that tells you to use the throw helpers does not kick in. However when this is pulled in to S.S.Cryptography, the analyzer lights up because it is unaware the source file is being pulled in to a non-net project.

@@ -287,7 +292,7 @@ public IEnumerable<Pkcs12SafeBag> GetBags()

if (_bags == null)
{
return Enumerable.Empty<Pkcs12SafeBag>();
return [];
Copy link
Member Author

Choose a reason for hiding this comment

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

Enumerable.Empty is in System.Linq. S.S.Cryptography does not have a reference to Linq, and it didn't seem worth it for this one case. An empty collection expression is probably "right" these days anyway.

@vcsjones
Copy link
Member Author

Do we need any unit tests for this, Kevin?

I am open to suggestions. Export, directly and indirectly, is used considerably in existing tests to the point where "keep it working" seemed sufficient. I did make a small mistake when bringing this up initially. My computer was probably close to catching on fire from all of the test failures and debug asserts I hit.

@vcsjones vcsjones merged commit c1ed07c into dotnet:main Jan 27, 2025
82 of 85 checks passed
@vcsjones vcsjones deleted the internal-pkcs12builder branch January 27, 2025 20:46
@vcsjones vcsjones added this to the 10.0.0 milestone Jan 27, 2025
@vcsjones vcsjones added the cryptographic-docs-impact Issues impacting cryptographic docs. Cleared and reused after documentation is updated each release. label Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Security cryptographic-docs-impact Issues impacting cryptographic docs. Cleared and reused after documentation is updated each release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants