Skip to content

Add easy way to create a certificate from just a cert-PEM #43590

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

Closed
shadow-cs opened this issue Oct 19, 2020 · 8 comments · Fixed by #43907
Closed

Add easy way to create a certificate from just a cert-PEM #43590

shadow-cs opened this issue Oct 19, 2020 · 8 comments · Fixed by #43907
Labels
api-approved API was approved in API review, it can be implemented area-System.Security
Milestone

Comments

@shadow-cs
Copy link
Contributor

shadow-cs commented Oct 19, 2020

Background and Motivation

#31944 was a great addition but it is sort of a shame we cannot use it just to create a cert from PEM without private key.

All that is needed is to expose ExtractCertificateFromPem publicly.

Proposed API

namespace System.Security.Cryptography.X509Certificates
{
    partial class X509Certificate2
    {
        public static X509Certificate2 CreateFromPem(ReadOnlySpan<char> certPem);
    }
}

Usage Examples

Cases where I need a single certificate from PEM data that is already in a System.String or ReadOnlySpan<char>.

Alternatives

  • Using a X509Certificate2Collection.ImportFromPem and extracting the single certificate.
  • Stripping the header and footer, decoding base64 and feeding to constructor.
  • new X509Certificate2(Encoding.ASCII.GetBytes(certPem))
@shadow-cs shadow-cs added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 19, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Security untriaged New issue has not been triaged by the area owner labels Oct 19, 2020
@ghost
Copy link

ghost commented Oct 19, 2020

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @jeffhandley
See info in area-owners.md if you want to be subscribed.

@vcsjones
Copy link
Member

The first case should be covered by the constructor that takes a path. For that one I think we wanted to avoid introducing a public method that does the same thing as the constructor.

For the latter where you have PEM contents, I would agree that there is not a super straight forward API that does that.

Stripping the header and footer, decoding base64 and feeding to constructor.

There is also the new PemEncoding functionality to parse and iterate over PEM files.

@bartonjs
Copy link
Member

The only thing this API would add that the existing ctor doesn't do is support reading the PEM content from a string without passing it through Encoding.ASCII/Encoding.UTF8. (I guess it also would enforce that it's a plain PEM certificate, vs any of the zillions of other formats that the ctors support).

I agree that it's not a negligible thing to have a PEM cert already in a string, but I'm not sure that it's worth creating confusion on the CreateFromPemFile method. Adding just public static X509Certificate2 CreateFromPem(ReadOnlySpan<char> certPem) seems slightly wrong from a symmetry point of view, but its mirror already exists as new X509Certificate2(string path).

Is the scenario you want to solve actually "I have a PEM-encoded certificate in a string and want an X509Certificate2 object", or am I mis-projecting?

@shadow-cs
Copy link
Contributor Author

shadow-cs commented Oct 19, 2020

The first case should be covered by the constructor that takes a path. For that one I think we wanted to avoid introducing a public method that does the same thing as the constructor.

Right @vcsjones! I added it just to be complete with the new static methods and didn't realize it was in the ctor also.

Is the scenario you want to solve actually "I have a PEM-encoded certificate in a string and want an X509Certificate2 object", or am I mis-projecting?

No @bartonjs you're exactly right. We're implementing a PKI with a custom CTLs (edit: and .NET5 is bringing a lot of improvements in handling chain verification etc. 😉) so parsing a single cert from string is what I need at this point. I did found some other helpers (thanks again @vcsjones for pointing at more). And I do agree that stripping the headers and decoding is simple enough but the method we need is sitting just right there.

@bartonjs
Copy link
Member

So

namespace System.Security.Cryptography.X509Certificates
{
    partial class X509Certificate2
    {
        public static X509Certificate2 CreateFromPem(ReadOnlySpan<char> certPem);
    }
}

seems to be all that's required for your scenario. It's pretty low development cost (rename the existing method, make it public) and has a scenario.

Any dissenters?

@shadow-cs
Copy link
Contributor Author

👍 @bartonjs that's just what we need.

@vcsjones
Copy link
Member

Any dissenters?

Seems good to me.

@bartonjs bartonjs added this to the 6.0.0 milestone Oct 19, 2020
@bartonjs bartonjs added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Oct 19, 2020
@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Oct 27, 2020
@terrajobst
Copy link
Contributor

terrajobst commented Oct 27, 2020

Video

  • Looks good as proposed
namespace System.Security.Cryptography.X509Certificates
{
    partial class X509Certificate2
    {
        public static X509Certificate2 CreateFromPem(ReadOnlySpan<char> certPem);
    }
}

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants