digicert / pkilint

A framework for verifying PKI structures
MIT License
86 stars 18 forks source link

Scope of reasons for revocation #104

Open zzzsz opened 2 weeks ago

zzzsz commented 2 weeks ago

Why are there only keyCompromise, affiliationChanged, superseded, cessationOfOperation, and privilegeWithdrawn reasons for revocation? I did not find such a revocation range restriction in RFC 5280.

And why can revocation reason cACompromise only exist in CA crl? My point is that if a leaf certificate is revoked because the CA is compromised, then its revocation reason should also be cACompromise.

def create_reason_code_validator(
        crl_type: crl.CertificateRevocationListType
):
    allowed_reasons = [
        rfc5280.CRLReason.namedValues[r]
        for r in [
            'keyCompromise',
            'affiliationChanged',
            'superseded',
            'cessationOfOperation',
            'privilegeWithdrawn',
        ]
    ]

    if crl_type == crl.CertificateRevocationListType.ARL:
        allowed_reasons.append(rfc5280.CRLReason.namedValues['cACompromise'])

    return crl_extension.CrlReasonCodeAllowlistValidator(
        allowed_reason_codes=allowed_reasons
    )
CBonnell commented 1 week ago

The restriction is from X.509 (08/2005), clause 8.5.2.2:

cACompromise is used in revoking a CA-certificate; it indicates that it is known or suspected that the subject's private key, or other aspects of the subject validated in the certificate, have been compromised;

Thus, the cACompromise reason code is only appropriate for revoking CA certificates.

zzzsz commented 1 week ago

I have a question. What is the range of pkix in the error message pkix.xxx written in your code? For example pkix.crl_prohibited_reason_code.

At first I thought the scope of pkix refers to rfc, but now you say that the scope of cACompromise comes from itu's x509, which confuses me. Also, why not just set the error message of CrlReasonCodeAllowlistValidator to itu.xxx?

CBonnell commented 1 week ago

Your understanding of the finding code classifications is correct. This finding code should be prefixed with "itu" instead of "pkix".

I'll mark this as a bug and look into getting this fixed.

CBonnell commented 1 week ago

Keeping this open so the finding code can be fixed.