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: X509AuthorityKeyIdentifierExtension #50488

Closed
Tracked by #64488
qmfrederik opened this issue Mar 31, 2021 · 8 comments
Closed
Tracked by #64488

API Proposal: X509AuthorityKeyIdentifierExtension #50488

qmfrederik opened this issue Mar 31, 2021 · 8 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security
Milestone

Comments

@qmfrederik
Copy link
Contributor

qmfrederik commented Mar 31, 2021

Background and Motivation

The .NET API includes the X509SubjectKeyIdentifierExtension which allows you to read the subject key identifier extension of a certificate.

The SKI is usually used together with the Authority Key Identifier (AKI) so you can identify the public key to be used to verify the signature on a certificate.

There's no built-in class to read the AKI.

Proposed API

+    public class X509AuthorityKeyIdentifierExtension: System.Security.Cryptography.X509Certificates.X509Extension
+    {
+        public X509AuthorityKeyIdentifierExtension() { }
+        public X509AuthorityKeyIdentifierExtension(byte[] authorityKeyIdentifier, bool critical = false);
+        public X509AuthorityKeyIdentifierExtension(System.ReadOnlySpan<byte> authorityKeyIdentifier, bool critical = false) { }
+        public X509AuthorityKeyIdentifierExtension(System.Security.Cryptography.AsnEncodedData encodedAuthorityKeyIdentifier, bool critical) { }
+    }

Usage Examples

Create a new certificate

X509Certificate2 signingCertificate;
var issuerSubjectKey = signingCertificate.Extensions.OfType<X509SubjectKeyIdentifierExtension>().Single().RawData;

CertificateRequest hostRequest = new CertificateRequest(...);
request.CertificateExtensions.Add(new X509AuthorityKeyIdentifierExtension(issuerSubjectKey.Slice(2)));

Alternative Designs

The API proposal follows the API shape of the other X509*Extension classes.

Risks

New API, so risk should be low.

@qmfrederik qmfrederik added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 31, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Security untriaged New issue has not been triaged by the area owner labels Mar 31, 2021
@ghost
Copy link

ghost commented Mar 31, 2021

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

Issue Details

Background and Motivation

The .NET API includes the X509SubjectKeyIdentifierExtension which allows you to read the subject key identifier extension of a certificate.

The SKI is usually used together with the Authority Key Identifier (AKI) so you can identify the public key to be used to verify the signature on a certificate.

There's no built-in class to read the AKI.

Proposed API

+    public class X509AuthorityKeyIdentifierExtension: System.Security.Cryptography.X509Certificates.X509Extension
+    {
+        public X509AuthorityKeyIdentifierExtension() { }
+        public X509AuthorityKeyIdentifierExtension(System.ReadOnlySpan<byte> authorityKeyIdentifier, bool critical) { }
+        public X509AuthorityKeyIdentifierExtension(System.Security.Cryptography.AsnEncodedData encodedAuthorityKeyIdentifier, bool critical) { }
+    }

Usage Examples

Create a new certificate

X509Certificate2 signingCertificate;
var issuerSubjectKey = signingCertificate.Extensions.OfType<X509SubjectKeyIdentifierExtension>().Single().RawData;

CertificateRequest hostRequest = new CertificateRequest(...);
request.CertificateExtensions.Add(new X509SubjectKeyIdentifierExtension(issuerSubjectKey.Slice(2)));

Alternative Designs

The API proposal follows the API shape of the other X509*Extension classes.

Risks

New API, so risk should be low.

Author: qmfrederik
Assignees: -
Labels:

api-suggestion, area-System.Security, untriaged

Milestone: -

@vcsjones
Copy link
Member

I think we typically have byte[] overloads for ReadOnlySpan<byte> things as well, so I would suggest adding:

public X509AuthorityKeyIdentifierExtension(byte[] authorityKeyIdentifier, bool critical);

X509SubjectKeyIdentifierExtension also has constructor overloads that take a PublicKey. Would it make sense for the AKI to, as well?

public X509AuthorityKeyIdentifierExtension(System.Security.Cryptography.X509Certificates.PublicKey key, bool critical);

@bartonjs
Copy link
Member

var issuerSubjectKey = signingCertificate.Extensions.OfType<X509SubjectKeyIdentifierExtension>().Single().RawData;
request.CertificateExtensions.Add(new X509SubjectKeyIdentifierExtension(issuerSubjectKey.Slice(2)));

Needing to do Slice(2) seems like the API is missing something 😄. It's assuming (a) that a KeyIdentifier value is present, (b) the length of the KeyIdentifier hasn't exceeded 127 bytes (while 20 bytes (SHA-1) is typical, it's not a limit), and (c) that the issuer+serialnumber format isn't used.

So it definitely needs some extra properties. And then probably needs more constructors:

  • keyIdentifier
  • issuer, serial
  • keyIdentifier, issuer, serial

Though issuer is a GeneralNames, which we already have a problem of not having a way to represent.

And, even though we haven't done it on the existing extension types, the critical parameter should be defaulted... in this case, to false.

@qmfrederik
Copy link
Contributor Author

@vcsjones

X509SubjectKeyIdentifierExtension also has constructor overloads that take a PublicKey. Would it make sense for the AKI to, as well?

I don't think so; there are multiple ways to derive a key identifier from a public key. So you pass a PublicKey, but there's no way to deterministically derive the key identifier from the key.

@qmfrederik
Copy link
Contributor Author

@bartonjs

Well, my reading of the RFC (but I'm known to be wrong at times 😄) is that section 4.2.1.1 says that it's either the key identifier or the issuer name and serial number. So that would leave two overloads.

Regarding GeneralNames, it seems authorityCertIssuer can only ever be the issuer name, which is defined in section 4.1.2.4 as a Name, so I guess the constructor could just take a X500DistinguishedName, and you can sidestep the GeneralNames issue?

So something like:

+        public X509AuthorityKeyIdentifierExtension(byte[] keyIdentifier, bool critical = false);
+        public X509AuthorityKeyIdentifierExtension(X500DistringuishedName issuerName, int serialNumber, bool critical = false);

It could be helpful if X509SubjectKeyIdentifierExtension could expose the KeyIdentifier as a byte[] array rather in addition to just a string.

@bartonjs
Copy link
Member

bartonjs commented Apr 1, 2021

my reading of the RFC ... says that it's [either/or]

I can see that reading, but I feel like I've seen some Issuing CA certs that had both, and OpenSSL's chain processing code definitely handles both.

The X.509 standard says it's ok (emphasis mine):

The key may be identified by an explicit key identifier in the keyIdentifier component, by the identification of a public-key certificate for the key (giving certificate issuer in the authorityCertIssuer component and public-key certificate serial number in the authorityCertSerialNumber component), or by both explicit key identifier and identification of a public-key certificate for the key.

X.509 also says (in 18.3.2.2 Use of authority key identifier):

The GeneralNames of the authorityCertIssuer component should be used to name the CA which issued the public-key certificate and also to optionally identify where the public-key certificate can be found when it is available via http, ftp, or ldap. In the latter case, one of the GeneralNames should be a uniformResourceIdentifier as specified ...

I don't think I've seen authorityKeyIdentifier used as a fetch source... or a cert that populated that data there... but that's what the standard says.

@bartonjs bartonjs removed the untriaged New issue has not been triaged by the area owner label Jul 2, 2021
@bartonjs bartonjs added this to the Future milestone Jul 2, 2021
@bartonjs bartonjs modified the milestones: Future, 7.0.0 Sep 16, 2021
@mregen
Copy link

mregen commented May 8, 2022

if this helps, here is a MIT licensed implementation based on the ASN.1 library.
https://github.com/OPCFoundation/UA-.NETStandard/blob/master/Libraries/Opc.Ua.Security.Certificates/Extensions/X509AuthorityKeyIdentifierExtension.cs
The field helper support is just limited to whats needed for OPC UA certificates

@bartonjs
Copy link
Member

There were two issues for this, apparently, and I've made an API proposal out of the older one (#24931). Closing this one in favor of that one.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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