Skip to content

ocsp: search CA by key hash instead of ext key id#7934

Merged
douzzer merged 2 commits intowolfSSL:masterfrom
rizlik:ocsp-get-ca-keyhash-fix
Sep 5, 2024
Merged

ocsp: search CA by key hash instead of ext key id#7934
douzzer merged 2 commits intowolfSSL:masterfrom
rizlik:ocsp-get-ca-keyhash-fix

Conversation

@rizlik
Copy link
Copy Markdown
Contributor

@rizlik rizlik commented Sep 2, 2024

Description

Make OCSP code search issuer certificate by key hash, not using the key hash to match the certificate extension key id.
While Key id must depend on the certificate's public key, it can be different than the hash of the key (see let's encrypt e6 certificate).

Fixes zd#18073
Fixes zd#18590

@rizlik
Copy link
Copy Markdown
Contributor Author

rizlik commented Sep 3, 2024

retest this please

julek-wolfssl
julek-wolfssl previously approved these changes Sep 3, 2024
@SparkiDev
Copy link
Copy Markdown
Contributor

Are you saying that the issuer key hash in the extension used a different hash algorithm?
We need a s=new define to turn on this behaviour.

@rizlik
Copy link
Copy Markdown
Contributor Author

rizlik commented Sep 4, 2024

Are you saying that the issuer key hash in the extension used a different hash algorithm? We need a s=new define to turn on this behaviour.

Not really.

In an OCSP response the responder is identified by (RFC 6960)

ResponderID ::= CHOICE {
byName [1] Name,
byKey [2] KeyHash }

KeyHash ::= OCTET STRING -- SHA-1 hash of responder's public key
(excluding the tag and length fields)

In OCSP code we use this KeyHash as the lookup key for the GetCA function, but the GetCA/AddCA functions use the extension Subject Key Id as the lookup key. The subject Key Id (RFC 5280):

For CA certificates, subject key identifiers SHOULD be derived from
the public key or a method that generates unique values.

KeyHash and the Subject Key Identifier match for most certificates, but this happens by chance. Notably new ecc let's encrypt certificates do not use the hash of the public key as their Subject Key Identifier (probably they use method 2 of section 4.2.1.2 RFC 5280).

So the keyHash is not part of extensions. it's a OCSP-specific way to identify the responder and we should use this instead of Subjec Key Identifier to lookup the certificate that signed the OCSP response. We don't need any define as the old behavior was wrong and worked by chance.

@rizlik rizlik assigned SparkiDev and unassigned rizlik Sep 4, 2024
@SparkiDev
Copy link
Copy Markdown
Contributor

So method 2 results in a 64-bit KeyHashId while Method 1 is 160-bits.
We can use this information to decide whether we need to lookup or not.

@SparkiDev SparkiDev assigned rizlik and unassigned SparkiDev Sep 4, 2024
@rizlik
Copy link
Copy Markdown
Contributor Author

rizlik commented Sep 4, 2024

So method 2 results in a 64-bit KeyHashId while Method 1 is 160-bits. We can use this information to decide whether we need to lookup or not.

The fact that let's encrypt uses a suggested method 2 doesn't mean every certificate will use that method. Another cert can use a completely arbitrary and undocumented method for generating a key identifier.

OCSP response reports the keyHash using SHA-1, we should use only the keyhash to find out which certificate signed the response.

@rizlik rizlik assigned SparkiDev and unassigned rizlik Sep 4, 2024
@SparkiDev
Copy link
Copy Markdown
Contributor

By the standard they are meant to use a certain algorithm in a specific way.
Make it a compile option to turn on this behaviour - I want the lookup to be quick if possible.

try to lookup the certificate using the key hash as key identifier
first. If we can't find a certificate, it means that the certificate
uses another method to compute the key identifier so we need to fallback
to linear search.
@rizlik
Copy link
Copy Markdown
Contributor Author

rizlik commented Sep 5, 2024

By the standard they are meant to use a certain algorithm in a specific way.

No, the subject key identifier is an opaque value by the standards.

Found out that let's encrypt switched to sha256-based method described in rfc7093, let's encrypt blog.

There is no way to compute a sha-256 subject key identifier with the sha-1 public key hash provided in the ocsp response.

Modified the PR to search using the key hash as a key (old behaviour) and fall-back to linear search if there are no matches.

I think the old behaviour is wrong and we shouldn't gate this PR behind a define.

This PR changes the look-up of OCSP responses verification only.

Copy link
Copy Markdown
Member

@julek-wolfssl julek-wolfssl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we should try to lookup in the hashmap table but we need to fallback on a direct list lookup. There are multiple ways to generate the keyId and we can't have a lookup for all of them. We already have other areas where we need to go through the entire list like in GetCAByAKID and GetCAByName.

@douzzer douzzer merged commit 9f6a75c into wolfSSL:master Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants