Fix CRL distribution point scope check logic in crl_crldp_check#3106
Merged
samuel40791765 merged 1 commit intoaws:fips-2024-09-27from Mar 19, 2026
Merged
Conversation
justsmth
approved these changes
Mar 19, 2026
WillChilds-Klein
approved these changes
Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit authored by @nebeid.
Description of changes:
A logic error in
crl_crldp_check()(crypto/x509/x509_vfy.c) prevents CRL distribution point matching from ever running for normal certificates. When a CRL has an Issuing Distribution Point (IDP) extension, the CRL is incorrectly considered out of scope and a revoked certificate escapes detection.Three bugs in one condition:
&&should be||— the comment says skip DPs withreasonsORCRLissuer, but the code only triggers when BOTH are present.return 1should becontinue— when the condition matches, the code declares the CRL in scope instead of skipping the DP.idp_check_dpis in the wrong branch — it only runs for DPs withreasons+CRLissuer, never for normal clean DPs.Fix
Took upstream commit 5386d90.
Testing
Two test scenarios added in
crypto/x509/x509_test.cc:Scenario 1: Cert with a single clean CRLDP + CRL with matching IDP
Leaf has a clean CRLDP (distpoint URI only, no
reasons, noCRLissuer). CRL has a matching IDP and revokes the leaf's serial.idp_check_dpis never called for clean DPs → CRL is out-of-scope.idp_check_dpmatches the distpoints → CRL in scope →CERT_REVOKED.Scenario 2: Cert with two DPs + two CRLs
Leaf has two distribution points:
distpointmatching CRL-B IDP +reasons+CRLissuer(should be skipped)distpoint(matches the revoking CRL-A)CRL-A (matching IDP) revokes the leaf. CRL-B (other IDP) has no revocations.
reasons+CRLissuerso the&&condition is true.idp_check_dpmatches DP1 against CRL-B →return 1→ CRL-B is in scope → no revocations → cert appears valid.&&check and by the fallback because it has an IDP.||catchesreasons).idp_check_dp→CERT_REVOKED.PoC output without fix:
PoC output with fix:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.