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

[API Proposal]: Additional X.509 collection content types in X509CertificateLoader #111547

Open
dongle-the-gadget opened this issue Jan 17, 2025 · 7 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security
Milestone

Comments

@dongle-the-gadget
Copy link

Background and motivation

The new X509CertificateLoader class is a regression compared to the now obsolete X509Certificate2Collection.Import in that it only supports PKCS#12 collections, whereas X509Certificate2Collection.Import supports not only PKCS#12 but also PKCS#7, Authenticode, etc. This means that users depend on an obsolete API just to load those X.509 content types.

API Proposal

namespace System.Security.Cryptography.X509Certificates;

public static class X509CertificateLoader
{
    public static X509Certificate2Collection LoadPkcs7Collection(byte[] data);
    public static X509Certificate2Collection LoadPkcs7Collection(ReadOnlySpan<byte> data);

   // Repeat for other content types.
}

API Usage

byte[] pkcs7Bytes = []; // This will be replaced by actual bytes from a p7b file.
X509Certificate2Collection pkcs7Collection = X509CertificateLoader.LoadPkcs7Collection(pkcs7Bytes);

ReadOnlySpan<byte> pkcs7Bytes = []; // This will be replaced by actual bytes from a p7b file.
X509Certificate2Collection pkcs7Collection = X509CertificateLoader.LoadPkcs7Collection(pkcs7Bytes);

Alternative Designs

No response

Risks

No response

@dongle-the-gadget dongle-the-gadget added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 17, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 17, 2025
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 bartonjs added this to the Future milestone Jan 17, 2025
@bartonjs
Copy link
Member

We left PKCS#7 out mostly because it's an easy workaround that still guarantees a single file format was loaded

SignedCms cms = new();
cms.Decode(pkcs7);
return cms.Certificates;

But, your request is noted.

We will never be seeking parity on that type; but the formats that people ask for do have a chance to come back. (It is basically guaranteed we'll never add SerializedCert to that loader, as it only makes sense on Windows, and I don't know that anyone uses it)

@dongle-the-gadget
Copy link
Author

Unfortunately SignedCms is not a suitable option for my use case, since it causes an exception:

AsnContentException: The provided data is tagged with 'Application' class value '13', but it should have been 'Universal' class value '16'.

@vcsjones
Copy link
Member

AsnContentException: The provided data is tagged with 'Application' class value '13', but it should have been 'Universal' class value '16'.

Well, that itself might be an issue worth looking in to. Do you have an example payload that reproduces that behavior?

@dongle-the-gadget
Copy link
Author

This exception occurs when I try to open any certificates from Trusted Signing. I have provided an example ZIP below:

ZIP of payload certificate

By the way, X509Certificate2Collection.Import can load this file okay, but not SignedCms.

@vcsjones
Copy link
Member

I have provided an example ZIP below:

SignedCms.Decode expects the DER encoded value. This seems to work fine for me on macOS and Linux.

using System;
using System.IO;
using System.Security.Cryptography.Pkcs;

SignedCms cms = new();
cms.Decode(Convert.FromBase64String(File.ReadAllText("text.p7b")));

Console.WriteLine(cms.Certificates.Count); // Prints 4

By the way, X509Certificate2Collection.Import can load this file okay, but not SignedCms.

Assuming your code is something like this:

using System;
using System.IO;
using System.Security.Cryptography.X509Certificates;
using System.Security.Cryptography.Pkcs;

X509Certificate2Collection c = new();
c.Import(File.ReadAllBytes("text.p7b"));

Console.WriteLine(c.Count);

Your code only works on Windows. Windows is more forgiving about the contents that it loads, in this case it is re-interpreting the base64 content bytes as literal text. This works, but on macOS and Linux, it doesn't.

This platform quirk is one of the reasons why the new loader exists in the first place - to address inconsistencies in how the inputs are handled. If new APIs were to be added, I suspect they would almost certainly work the same way that the new APIs do: either you have PEM-text, or binary DER, but not something that is binary text.

@dongle-the-gadget
Copy link
Author

Oh, I see. The service initially gave me a Base64 value and I did try a Base64 decoder. I guess either it's nested or that the Base64 decoder failed somehow.

@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security
Projects
None yet
Development

No branches or pull requests

4 participants