-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Inconsistent IDP extension constraint check #11467
base: main
Are you sure you want to change the base?
Conversation
…n a CRL can have only one of onlyContainsUserCerts, onlyContainsCACerts, onlyContainsAttributeCerts set to TRUE. However, extensions.py (lines 1991 : 2003), indirectCRL is also included, which leads to invalid CRL even if the RFC requirement is met. The proposed fix is to drop indirectCRL from the check so it conforms to the RFC.
Cc: @woodruffw this may be a CABF requirement
…On Wed, Aug 21, 2024, 2:30 PM Han Yu ***@***.***> wrote:
Hello, I found an inconsistent IDP extension constraint check per RFC5280
Section 5.2.5. I do not think the indirectCRL needs to be subject to that.
Excerpt from the RFC:
-- at most one of onlyContainsUserCerts, onlyContainsCACerts,
-- and onlyContainsAttributeCerts may be set to TRUE.
Thank you
------------------------------
You can view, comment on, or merge this pull request online at:
#11467
Commit Summary
- 8ccec8d
<8ccec8d>
Per RFC5280 Section 5.2.5, the Issuing Distribution Point extension in a
CRL can have only one of onlyContainsUserCerts, onlyContainsCACerts,
onlyContainsAttributeCerts set to TRUE. However, extensions.py (lines 1991
: 2003), indirectCRL is also included, which leads to invalid CRL even if
the RFC requirement is met. The proposed fix is to drop indirectCRL from
the check so it conforms to the RFC.
File Changes
(1 file <https://github.com/pyca/cryptography/pull/11467/files>)
- *M* src/cryptography/x509/extensions.py
<https://github.com/pyca/cryptography/pull/11467/files#diff-2ad33db9160057b2dc1719647908099d649c68f1dad9f672153d24bda0d21fa3>
(6)
Patch Links:
- https://github.com/pyca/cryptography/pull/11467.patch
- https://github.com/pyca/cryptography/pull/11467.diff
—
Reply to this email directly, view it on GitHub
<#11467>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAGBDYQVPBR5JD6O5IO63ZSTMFDAVCNFSM6AAAAABM4SWWA2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGQ3TQNZTGI3DAOA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Here's the relevant CABF section: https://cabforum.org/working-groups/server/baseline-requirements/requirements/#7221-crl-issuing-distribution-point
So CABF is consistent with 5280 on this, it seems, insofar as (I think this has no effect on the validator, though, since this extension is for CRLs only and not CAs/leafs?) |
Ah, I see... I think your comment about the having no effect on the validator may be true though I am not 100% sure. But, wouldn't this check mean that indirectCRL can never be set to TRUE in the CRL for it to be valid? I basically discovered this when loading the CRL that contains the IDP with indirectCRL, onlyContainsUserCerts set to TRUE. |
@reaperhulk do you remember why we did this? just an accident? Either way, it seems like this change is correct, and if you can fix the tests, we can get this merged. |
Yes this appears to be a mistake, so if the tests get fixed here then this fix looks correct. |
Hello, I found an inconsistent IDP extension constraint check per RFC5280 Section 5.2.5. I do not think the indirectCRL needs to be subject to that. Excerpt from the RFC:
-- at most one of onlyContainsUserCerts, onlyContainsCACerts,
-- and onlyContainsAttributeCerts may be set to TRUE.
Thank you