ocsp: bind responder authorization to CertID issuerKeyHash#10303
ocsp: bind responder authorization to CertID issuerKeyHash#10303julek-wolfssl wants to merge 1 commit intowolfSSL:masterfrom
Conversation
Addresses ZD21675
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR tightens OCSP responder authorization by binding delegated responder checks to the OCSP CertID issuerKeyHash (in addition to issuerNameHash), addressing ZD21675.
Changes:
- Store issuer key hash on
Signerand propagate it from parsed certificates. - Extend
CheckOcspResponder(and chain-check helper) to requireissuerKeyHashmatches for delegated responders. - Adjust OCSP verification call sites to pass the extra key-hash context.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| wolfssl/wolfcrypt/asn.h | Adds issuerKeyHash to Signer for OCSP authorization binding. |
| wolfssl/ocsp.h | Updates CheckOcspResponder signature to include subject/issuer key hashes. |
| wolfcrypt/src/asn.c | Populates issuerKeyHash and updates OCSP responder checks to pass new params. |
| src/ocsp.c | Enforces name+key matching (CertID halves) for delegated OCSP responders and chain building. |
Comments suppressed due to low confidence (1)
wolfcrypt/src/asn.c:1
OcspRespCheck()currently passesresponder->subjectKeyHashas the first argument toCheckOcspResponder, but that parameter is compared againstsingle->issuerHash(the CertID issuer name hash). This makes the issuer check compare a key-hash to a name-hash and will incorrectly fail/behave. Passresponder->subjectNameHashassubjectHashandresponder->subjectKeyHashassubjectKeyHash.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (XMEMCMP(parent->subjectNameHash, issuerHash, | ||
| OCSP_DIGEST_SIZE) == 0 && | ||
| XMEMCMP(parent->subjectKeyHash, issuerKeyHash, | ||
| OCSP_DIGEST_SIZE) == 0) { |
There was a problem hiding this comment.
Key-hash comparisons are using OCSP_DIGEST_SIZE, but subjectKeyHash/issuerKeyHash are key identifier hashes (typically KEYID_SIZE). If OCSP_DIGEST_SIZE differs from KEYID_SIZE (e.g., non-SHA1 OCSP digests), this will compare the wrong number of bytes and can cause false accept/reject. Use KEYID_SIZE (or sizeof(parent->subjectKeyHash) / sizeof(single->issuerKeyHash)) for key-hash comparisons.
| OCSP_DIGEST_SIZE) == 0) { | |
| sizeof(parent->subjectKeyHash)) == 0) { |
| if (subjectKeyHash != NULL && | ||
| XMEMCMP(subjectHash, single->issuerHash, | ||
| OCSP_DIGEST_SIZE) == 0 && | ||
| XMEMCMP(subjectKeyHash, single->issuerKeyHash, | ||
| OCSP_DIGEST_SIZE) == 0) { |
There was a problem hiding this comment.
Same issue as above: single->issuerKeyHash is a key hash, but it's compared with OCSP_DIGEST_SIZE. This should use KEYID_SIZE (or sizeof(single->issuerKeyHash)) to avoid mismatched lengths when OCSP digest size differs from keyid size.
| WOLFSSL_LOCAL int CheckOcspResponder(OcspResponse *bs, byte* subjectHash, | ||
| byte extExtKeyUsage, byte* issuerHash, void* vp); | ||
| byte* subjectKeyHash, byte extExtKeyUsage, byte* issuerHash, | ||
| byte* issuerKeyHash, void* vp); |
There was a problem hiding this comment.
With the new logic, subjectHash is used as the responder certificate subject name hash (compared to CertID issuerNameHash), while subjectKeyHash is the responder's subject key hash. Renaming subjectHash/issuerHash to subjectNameHash/issuerNameHash (and similarly for key hashes) would reduce the risk of call-site mixups like the one in OcspRespCheck().
|
| #endif | ||
| #ifdef HAVE_OCSP | ||
| byte subjectKeyHash[KEYID_SIZE]; | ||
| byte issuerKeyHash[KEYID_SIZE]; /* key hash of verifying parent CA */ |
There was a problem hiding this comment.
is this the issuer of the cert we are verifying? is this the root CA of the chain whose leaf certificate we are verifying?
Addresses ZD21675