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

Provide a way to load a CertificateRequest from a byte[] #29547

Closed
Tracked by #64488
jeremyVignelles opened this issue May 14, 2019 · 12 comments · Fixed by #73023
Closed
Tracked by #64488

Provide a way to load a CertificateRequest from a byte[] #29547

jeremyVignelles opened this issue May 14, 2019 · 12 comments · Fixed by #73023
Labels
api-approved API was approved in API review, it can be implemented area-System.Security blocking Marks issues that we want to fast track in order to unblock other important work
Milestone

Comments

@jeremyVignelles
Copy link

jeremyVignelles commented May 14, 2019

Rationale

I am using the CertificateRequest.CreateSigningRequest() to get a CSR byte array that I can send to the Certificate Authority.

As far as I know, there is currently no way, on the CA side, to "deserialize" this byte array into a CertificateRequest.

Use case

I am currently building a CA in .NET, that can sign certificate for .net client applications.

The .net client generates a PKCS 10 CSR, and send it over HTTPS to the CA, which will sign it.

Proposed API

Policies and best practices dictate that we want callers to be able to provide basic certificate revocation via CRL (Certificate Revocation Lists) prior to enabling them to load a PKCS#10 signing request, so CRL building is included in this proposal.

A CRL is, essentially, a signed list of (byte[] SerialNumber, DateTimeOffset RevocationTime, X509Extension[] Extensions).

Build a CRL from nothing

namespace System.Security.Cryptography.X509Certificates
{
    public sealed partial class CertificateRevocationListBuilder
    {
        public CertificateRevocationListBuilder() { }

        public void AddEntry(
            byte[] serialNumber,
            DateTimeOffset? revocationTime = default,
            X509RevocationReason? reason = default) { }

        public void AddEntry(
            ReadOnlySpan<byte> serialNumber,
            DateTimeOffset? revocationTime = default,
            X509RevocationReason? reason = default) { }
        
        // Helper for AddEntry(certificate.SerialNumberBytes)
        public void AddEntry(
            X509Certificate2 certificate,
            DateTimeOffset? revocationTime = default,
            X509RevocationReason? reason = default) { }
       
        // Calls similar overload with thisUpdate: DateTimeOffset.UtcNow
        public byte[] Build(
            X509Certificate2 issuerCertificate,
            BigInteger crlNumber,
            DateTimeOffset nextUpdate,
            HashAlgorithmName hashAlgorithm,
            RSASignaturePadding? rsaSignaturePadding = null) { throw null; }

        // Sanity checks issuerCertificate,
        // builds a X509SignatureGenerator from the cert and (for RSA) rsaSignaturePadding,
        // builds an appropriate X509AuthorityKeyIdentifierExtension,
        // then calls the most complicated overload of Build
        public byte[] Build(
            X509Certificate2 issuerCertificate,
            BigInteger crlNumber,
            DateTimeOffset nextUpdate,
            DateTimeOffset thisUpdate,
            HashAlgorithmName hashAlgorithm,
            RSASignaturePadding? rsaSignaturePadding = null) { throw null; }

        // Calls similar overload with thisUpdate: DateTimeOffset.UtcNow
        public byte[] Build(
            X500DistinguishedName issuerName, 
            X509SignatureGenerator generator,
            BigInteger crlNumber,
            DateTimeOffset nextUpdate,
            HashAlgorithmName hashAlgorithm,
            X509AuthorityKeyIdentifierExtension akid) { throw null; }

        public byte[] Build(
            X500DistinguishedName issuerName, 
            X509SignatureGenerator generator,
            BigInteger crlNumber,
            DateTimeOffset nextUpdate,
            DateTimeOffset thisUpdate,
            HashAlgorithmName hashAlgorithm,
            X509AuthorityKeyIdentifierExtension akid) { throw null; }
    }

    // Names/numbers come from X.509 specification
    public enum X509RevocationReason
    {
        Unspecified = 0,
        KeyCompromise = 1,
        // Alternative: CertificateAuthorityCompromise
        CACompromise = 2,
        AffiliationChanged = 3,
        Superseded = 4,
        CessationOfOperation = 5,
        CertificateHold = 6,

        RemoveFromCrl = 8,
        PrivilegeWithdrawn = 9,
        // Alternative: AttributeAuthorityCompromise
        AACompromise = 10,
        WeakAlgorithmOrKey = 11,
    }
}

Load an existing CRL, modify it, save it back

namespace System.Security.Cryptography.X509Certificates
{
    public sealed partial class CertificateRevocationListBuilder
    {
        public static CertificateRevocationListBuilder Load(
            byte[] currentCrl,
            out BigInteger currentCrlNumber) { throw null; }

        public static CertificateRevocationListBuilder Load(
            ReadOnlySpan<byte> currentCrl,
            out BigInteger currentCrlNumber,
            out int bytesConsumed) { throw null; }

        public static CertificateRevocationListBuilder LoadPem(
            string currentCrl,
            out BigInteger currentCrlNumber) { throw null; }

        public static CertificateRevocationListBuilder LoadPem(
            ReadOnlySpan<char> currentCrl,
            out BigInteger currentCrlNumber) { throw null; }


        // As proposed, this type doesn't allow enumerating the contents, but this method
        // permits a blanket removal policy, e.g.
        //   builder.ExpireEntries(DateTimeOffset.UtcNow - MaximumValidity)
        // to purge off all old entries (if desired)
        public void ExpireEntries(DateTimeOffset oldestRevocationTimeToKeep) { }

        public void RemoveEntry(byte[] serialNumber) { }
        public void RemoveEntry(ReadOnlySpan<byte> serialNumber) { }
    }
}

Creating a CRL Distribution Points Extension for Generated Certificates

At this time we don't think it's important to expose a way to read these back, and we don't support generating them in their full complexity, so we provide a helper-builder on the closest appropriate type (without burning the best type name for if we add a rich type later).

namespace System.Security.Cryptography.X509Certificates
{
    public sealed partial class CertificateRevocationListBuilder
    {
        public static X509Extension BuildCrlDistributionPointExtension(
            IEnumerable<string> uris,
            bool critical = false) { throw null; }
    }
}

Support PKCS#10 attributes, load a PKCS#10

namespace System.Security.Cryptography.X509Certificates
{
    public sealed partial class CertificateRequest
    {
        // add a new ctor with a (defaulted) RSASignaturePadding
        public CertificateRequest(
            X500DistinguishedName subjectName,
            X509Certificates.PublicKey publicKey,
            HashAlgorithmName hashAlgorithm,
            RSASignaturePadding? rsaSignaturePadding = default) { }

        // PKCS#10 Challenge Password, and friends
        public Collection<AsnEncodedData> OtherRequestAttributes { get; }

        public static CertificateRequest LoadSigningRequest(
            byte[] pkcs10,
            HashAlgorithmName signerHashAlgorithm,
            bool skipSignatureValidation = false,
            bool unsafeLoadCertificateExtensions = false,
            RSASignaturePadding? signerSignaturePadding = null) { throw null; }

        public static CertificateRequest LoadSigningRequest(
           ReadOnlySpan<byte> pkcs10,
           HashAlgorithmName signerHashAlgorithm,
           out int bytesConsumed,
           bool skipSignatureValidation = false,
           bool unsafeLoadCertificateExtensions = false,
           RSASignaturePadding? signerSignaturePadding = null) { throw null; }

        public static CertificateRequest LoadSigningRequestPem(
            ReadOnlySpan<char> pkcs10Pem,
            HashAlgorithmName signerHashAlgorithm,
            bool skipSignatureValidation = false,
            bool unsafeLoadCertificateExtensions = false,
            RSASignaturePadding? signerSignaturePadding = null) { throw null; }

        public static CertificateRequest LoadSigningRequestPem(
            string pkcs10Pem,
            HashAlgorithmName signerHashAlgorithm,
            bool skipSignatureValidation = false,
            bool unsafeLoadCertificateExtensions = false,
            RSASignaturePadding? signerSignaturePadding = null) { throw null; }

        public string CreateSigningRequestPem() { throw null; }
        public string CreateSigningRequestPem(X509SignatureGenerator signatureGenerator) { throw null; }
    }
}

Usability/Understanding Improvement in X509BasicConstraintsExtension

namespace System.Security.Cryptography.X509Certificates
{
    partial class X509BasicConstraintsExtension
    {
        // Named wrapper for
        //   new X509BasicConstraintsExtension(
        //     true,
        //     pathLengthConstraint.HasValue,
        //     pathLengthConstraint.GetValueOrDefault(),
        //     true);
        public static X509BasicConstraintsExtension CreateForCertificateAuthority(int? pathLengthConstraint = default(int?)) { throw null; }

        // Named wrapper for
        //   new X509BasicConstraintsExtension(
        //     false,
        //     false,
        //     0,
        //     critical);
        public static X509BasicConstraintsExtension CreateForEndEntity(bool critical = false) { throw null; }
    }
}
@bartonjs
Copy link
Member

There are N things that I think are important before API is added here:

  1. The ability to create (and probably read) CRLs
  2. The ability to encode CRL Distribution Point values into cert extensions
  3. The ability to encode the Authority Information Access certificate extension (for OCSP).
  4. Maybe the ability to read OCSP requests and write OCSP responses.
  5. Figuring out what to do with the PKCS#10 extensions (like challenge request).
  6. Being able to reason over the SAN entries
  7. Certificate Transparency might be good, too.

That still leaves out the fundamentally hard problems like "knowing if it's reasonable to sign the SAN entries", but that's a CA policy thing. The important part, to me, from the platform perspective is to make it such that anyone writing any form of CA be capable of executing on the main parts of policy. Effectively, if one were to walk through the CA Browser Forum's Baseline Requirements document, would they be capable of writing a conforming CA.... if not, I'd rather not have the ability to read the CSR.

If the only needs from your CA solution are "I want your public key, and I'm going to make up the rest from some external context data", that's best conveyed by the client sending their key with the new (in Core 3.0) AsymmetricAlgorithm.ExportSubjectPublicKeyInfo() method (family), then the CA side hydrating it into an appropriate key (it should probably be hydratable directly to a PublicKey object, but I missed that)... then use the CertficateRequest to line up the rest of the extensions (which have no surprises, since you added them instead of decoded them) and sign it.

So, effectively, this request is the very end of a long chain of feature requests for a "I want to make a Certificate Authority with .NET". The goal of the API was just to be able to submit requests to a good CA, or to build certificates for testing scenarios... with the other half of the equation a giant TBD 😄.

@jeremyVignelles
Copy link
Author

Thanks for your answer, that makes sense.
I guess I'll have to do it another way I guess.
The PKCS#10 was a way for me to have the client apps generate alternative names based on local IPs and send them to the CA. The client is already authenticated by other means, so I don't really need a fully compliant CA.

@bartonjs
Copy link
Member

a way for me to have the client apps generate alternative names based on local IPs and send them to the CA

You could, if it fits into your security model, have the client send a list of IP addresses, or take the encoded output of the SubjectAlternativeNameBuilder, and send that in your protocol (perhaps alongside the SubjectPublicKeyInfo).

Yeah, it asymptotically approaches "just build/send and receive/read a PKCS#10", but doing things piecemeal, like not allowing the client to assert that it is, itself, a subordinate CA.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@marcbarry
Copy link

+1 to deserialize a CertificateRequest byte array back into a CertificateRequest object for delayed signing.

Agree this is conceptually at the long tail end of implementing a CA which is CA/B compliant and might look a bit like a public authority, but short of this there are there not myriad private and internal use-cases where signing a received CSR is a useful mechanic?

@bartonjs bartonjs modified the milestones: 5.0.0, Future Jul 7, 2020
@bartonjs bartonjs removed the untriaged New issue has not been triaged by the area owner label Jul 7, 2020
@ericwj
Copy link

ericwj commented Nov 8, 2020

Suppose I serialize it and write it to disk. Now I want to read it back in?

I certainly don't want to have to dive into openssl or, worse, BouncyCastle. Or the other way around, depending on preference.

@ArthurHNL
Copy link

We are currently developing a solution that needs this feature. I agree that numerous things must be added before a full-fledged CA can be developed using .NET. But when I'm able to manually create and serialize a certificate request, why shouldn't I be able to deserialize a certificate request? Now I am probably going to have to interop with OpenSSL :(.

@vcsjones
Copy link
Member

vcsjones commented Sep 16, 2021

@bartonjs

Sounds like there is some interest in this, so I would propose something like this, inspired from what we have an X509Certificate2.

namespace System.Security.Cryptography.X509Certificates {
    public partial class CertificateRequest {
+       public CertificateRequest(byte[] derContents);
+       public CertificateRequest(ReadOnlySpan<byte> derContents);
+       public static CertificateRequest CreateFromPem(ReadOnlySpan<char> csrPem);
    }
}

@bartonjs
Copy link
Member

The API proposal isn't the hard part 😄. I'm still opposed to adding this functionality until at least numbers 1-3 in #29547 (comment) are tackled. But, getting to the point where this can be added to .NET 7 (then adding it) is my top personal goal for .NET 7.

@bartonjs bartonjs modified the milestones: Future, 7.0.0 Sep 16, 2021
@amin1best

This comment has been minimized.

@bartonjs
Copy link
Member

bartonjs commented Jul 1, 2022

@vcsjones I still need to work in X509RevocationReason. Did you envision it as a nullable parameter to AddEntry (e.g. AddEntry(ReadOnlySpan<byte> serialNumber, DateTimeOffset revocationTime, X509RevocationReason? reason = default)), or just double the overloads tacking it on the end as non-nullable... or overloads with non-defaulted nullable? (I slightly worry an overload with non-nullable might suggest we're providing a value in the other overloads, but that it's too complicated to represent with a default)

@vcsjones
Copy link
Member

vcsjones commented Jul 1, 2022

@bartonjs I envisioned something like this, both revocationTime and reason as nullable, since I think your original draft had overloads with and without revocationTime, I just switched everything to nullable to avoid an overloadsplosion.

// Absolutely not tied to this name.
public enum CertificateRevocationListRevokeReason {
    Unspecified = 0,
    KeyCompromise = 1,
    CACompromise = 2,
    AffiliationChanged = 3,
    Superseded = 4,
    CessationOfOperation = 5,
    CertificateHold = 6,
    // No 7, per RFC 5280,
    RemoveFromCRL = 8,
    PrivilegeWithdrawn = 9,
    AACompromise = 10,
}

// CRLBuilder AddEntry:

public void AddEntry(
    byte[] serialNumber,
    DateTimeOffset? revocationTime = null,
    CertificateRevocationListRevokeReason? reason = null);

public void AddEntry(
    ReadOnlySpan<byte> serialNumber,
    DateTimeOffset? revocationTime = null,
    CertificateRevocationListRevokeReason? reason = null);

public void AddEntry(
    X509Certificate2 certificate,
    DateTimeOffset? revocationTime = null,
    CertificateRevocationListRevokeReason? reason = null);

null reason does not mean CertificateRevocationListRevokeReason.Unspecified, it means the extension is omitted entirely. Hence the distinction between making it a nullable enum vs. the default just being "Unspecified".

@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 labels Jul 1, 2022
@bartonjs bartonjs added the blocking Marks issues that we want to fast track in order to unblock other important work label Jul 13, 2022
@terrajobst
Copy link
Member

terrajobst commented Jul 19, 2022

Video

  • CertificateRevocationListBuilder
    • Build: let's make thisUpdate optional
    • Let's remove ExpireEntries until someone asks for it
  • CertificateRequest let's not use unsafe parameters because they can disappear from the call sites. Suggestion from YouTube was to use an enum, which we liked.
namespace System.Security.Cryptography.X509Certificates;

public sealed partial class CertificateRevocationListBuilder
{
    public CertificateRevocationListBuilder();

    public void AddEntry(byte[] serialNumber,
                         DateTimeOffset? revocationTime = default,
                         X509RevocationReason? reason = default);
    public void AddEntry(ReadOnlySpan<byte> serialNumber,
                         DateTimeOffset? revocationTime = default,
                         X509RevocationReason? reason = default);

    public void AddEntry(X509Certificate2 certificate,
                         DateTimeOffset? revocationTime = default,
                         X509RevocationReason? reason = default);
    
    public byte[] Build(X509Certificate2 issuerCertificate,
                        BigInteger crlNumber,
                        DateTimeOffset nextUpdate,
                        HashAlgorithmName hashAlgorithm,
                        RSASignaturePadding? rsaSignaturePadding = null,
                        DateTimeOffset? thisUpdate = null);

    public byte[] Build(X500DistinguishedName issuerName,
                        X509SignatureGenerator generator,
                        BigInteger crlNumber,
                        DateTimeOffset nextUpdate,
                        HashAlgorithmName hashAlgorithm,
                        X509AuthorityKeyIdentifierExtension akid,
                        DateTimeOffset? thisUpdate = null));


    public static CertificateRevocationListBuilder Load(byte[] currentCrl,
                                                        out BigInteger currentCrlNumber);

    public static CertificateRevocationListBuilder Load(ReadOnlySpan<byte> currentCrl,
                                                        out BigInteger currentCrlNumber,
                                                        out int bytesConsumed);

    public static CertificateRevocationListBuilder LoadPem(string currentCrl,
                                                           out BigInteger currentCrlNumber);

    public static CertificateRevocationListBuilder LoadPem(ReadOnlySpan<char> currentCrl,
                                                           out BigInteger currentCrlNumber);

    public bool RemoveEntry(byte[] serialNumber);
    public bool RemoveEntry(ReadOnlySpan<byte> serialNumber);

    public static X509Extension BuildCrlDistributionPointExtension(IEnumerable<string> uris,
                                                                   bool critical = false);
}

public enum X509RevocationReason
{
    Unspecified = 0,
    KeyCompromise = 1,
    CACompromise = 2,
    AffiliationChanged = 3,
    Superseded = 4,
    CessationOfOperation = 5,
    CertificateHold = 6,
    RemoveFromCrl = 8,
    PrivilegeWithdrawn = 9,
    AACompromise = 10,
    WeakAlgorithmOrKey = 11,
}

public partial class CertificateRequest
{
    public CertificateRequest(X500DistinguishedName subjectName,
                              PublicKey publicKey,
                              HashAlgorithmName hashAlgorithm,
                              RSASignaturePadding? rsaSignaturePadding = default);

    public Collection<AsnEncodedData> OtherRequestAttributes { get; }

    public static CertificateRequest LoadSigningRequest(byte[] pkcs10,
                                                        HashAlgorithmName signerHashAlgorithm,
                                                        CertificateRequestLoadOptions options = default,
                                                        RSASignaturePadding? signerSignaturePadding = null);

    public static CertificateRequest LoadSigningRequest(ReadOnlySpan<byte> pkcs10,
                                                        HashAlgorithmName signerHashAlgorithm,
                                                        out int bytesConsumed,
                                                        CertificateRequestLoadOptions options = default,
                                                        RSASignaturePadding? signerSignaturePadding = null);

    public static CertificateRequest LoadSigningRequestPem(ReadOnlySpan<char> pkcs10Pem,
                                                           HashAlgorithmName signerHashAlgorithm,
                                                           CertificateRequestLoadOptions options = default,
                                                           RSASignaturePadding? signerSignaturePadding = null);

    public static CertificateRequest LoadSigningRequestPem(string pkcs10Pem,
                                                           HashAlgorithmName signerHashAlgorithm,
                                                           CertificateRequestLoadOptions options = default,
                                                           RSASignaturePadding? signerSignaturePadding = null);

    public string CreateSigningRequestPem();
    public string CreateSigningRequestPem(X509SignatureGenerator signatureGenerator);
}

[Flags]
public enum CertificateRequestLoadOptions
{
    Default = 0x0,
    SkipSignatureValidation = 0x01,
    UnsafeLoadCertificateExtensions = 0x02
}

public partial class X509BasicConstraintsExtension
{
    public static X509BasicConstraintsExtension CreateForCertificateAuthority(int? pathLengthConstraint = null);
    public static X509BasicConstraintsExtension CreateForEndEntity(bool critical = false);
}

@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 Jul 19, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 28, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 3, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 3, 2022
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 blocking Marks issues that we want to fast track in order to unblock other important work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants