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

x/crypto/ocsp: create OCSP request without issuer certificate #49355

Closed
joeshaw opened this issue Nov 4, 2021 · 4 comments
Closed

x/crypto/ocsp: create OCSP request without issuer certificate #49355

joeshaw opened this issue Nov 4, 2021 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@joeshaw
Copy link
Contributor

joeshaw commented Nov 4, 2021

What version of Go are you using (go version)?

$ go version
go version go1.17.2 darwin/arm64

x/crypto version: v0.0.0-20210921155107-089bfa567519

Details

The existing ocsp.CreateRequest function takes two certificates: a cert and its issuer. It would be nice to have a variant that didn't require the issuer to be provided.

The two pieces of information needed from an issuer are its RawSubject and its public key: https://github.com/golang/crypto/blob/089bfa5675191fd96a44247682f76ebca03d7916/ocsp/ocsp.go#L641-L661

The Subject info for an issuer is available in the cert's RawIssuer field, and the public key info is available in the AuthorityKeyId field. Go's x509 implementation targets the WebPKI, and the AuthorityKeyIdentifier extension is required for subscriber certificates by the Baseline Requirements, v1.8.0 section 7.1.2.3 item g:

g. authorityKeyIdentifier (required)
This extension MUST be present and MUST NOT be marked critical. It MUST contain a keyIdentifier field and it MUST NOT contain a authorityCertIssuer or authorityCertSerialNumber field.

@gopherbot gopherbot added this to the Unreleased milestone Nov 4, 2021
@thanm thanm added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 4, 2021
@thanm
Copy link
Contributor

thanm commented Nov 4, 2021

@FiloSottile

@joeshaw
Copy link
Contributor Author

joeshaw commented Nov 5, 2021

Looks like in the common case you can set issuerKeyHash := cert.AuthorityKeyId (h/t @dgryski) but RFC 5820 adds a wrinkle:

   Two common methods for generating key
   identifiers from the public key are described in Section 4.2.1.2.

[...]

   For CA certificates, subject key identifiers SHOULD be derived from
   the public key or a method that generates unique values.  Two common
   methods for generating key identifiers from the public key are:

      (1) The keyIdentifier is composed of the 160-bit SHA-1 hash of the
           value of the BIT STRING subjectPublicKey (excluding the tag,
           length, and number of unused bits).

      (2) The keyIdentifier is composed of a four-bit type field with
           the value 0100 followed by the least significant 60 bits of
           the SHA-1 hash of the value of the BIT STRING
           subjectPublicKey (excluding the tag, length, and number of
           unused bits).

   Other methods of generating unique numbers are also acceptable.

(Simple assignment is option 1 above.)

So this might not be suitable and maybe just an optimization we can make when the critera are met.

@AGWA
Copy link

AGWA commented Jul 2, 2024

There's no guarantee in the WebPKI (or PKIX generally) that the AKI extension contains a value that's useful for creating an OCSP request, and there's no way to tell how the AKI was generated. Notably, Let's Encrypt now uses a truncated SHA-256 hash for SKI/AKI. There's no way to distinguish these AKIs from the SHA-1 AKIs that other CAs use.

Go could consider documenting that the only fields accessed in issuer are RawSubject and RawSubjectPublicKeyInfo. This could be useful if the issuer is a non-certificate trust anchor. But under no circumstances should AKI be used.

@joeshaw
Copy link
Contributor Author

joeshaw commented Jul 2, 2024

Thank you for the information! I'm going to close this as it's inappropriate.

@joeshaw joeshaw closed this as completed Jul 2, 2024
@joeshaw joeshaw closed this as not planned Won't fix, can't repro, duplicate, stale Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants