cryptography icon indicating copy to clipboard operation
cryptography copied to clipboard

Cryptography parsed the CRL file with an empty Key Identifier.

Open onepeople158 opened this issue 6 months ago • 7 comments

Version: 45.0.3

Hello Developer, I successfully parsed a CRL file with an empty Key Identifier using Cryptography.When I used GnuTLS to parse this CRL file, it returned an error: error: gnutls_x509_ext_import_authority_key_id: ASN1 parser: Error in DER parsing. Is this considered an error?

Test Case:

crl_empty_key.zip

Code:

from cryptography.x509 import load_pem_x509_crl, load_der_x509_crl
from cryptography.x509 import ExtensionNotFound
import sys

def load_crl(file_path):
    with open(file_path, "rb") as f:
        crl_data = f.read()
    try:
        crl = load_pem_x509_crl(crl_data)
    except ValueError:
        crl = load_der_x509_crl(crl_data)
    return crl

def print_crl_issuer(file_path):
    crl=load_crl(file_path)
    aki_extension = None
    try:
        for ext in crl.extensions:
             if ext.oid == x509.oid.ExtensionOID.AUTHORITY_KEY_IDENTIFIER:
                    aki=ext.value
                    print(aki)
    except Exception as e:
        print(f"Error parsing CRL: {e}")

file_path = 'filename'
print_crl_issuer(file_path)

onepeople158 avatar Jun 07 '25 14:06 onepeople158

Hmm, so an empty AKI is stupid, but it's not clear it violates the RFC?

   AuthorityKeyIdentifier ::= SEQUENCE {
      keyIdentifier             [0] KeyIdentifier           OPTIONAL,
      authorityCertIssuer       [1] GeneralNames            OPTIONAL,
      authorityCertSerialNumber [2] CertificateSerialNumber OPTIONAL  }

(https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.1)

Is there some place that says this violates an RFC?

alex avatar Jun 07 '25 16:06 alex

Hmm, so an empty AKI is stupid, but it's not clear it violates the RFC?

   AuthorityKeyIdentifier ::= SEQUENCE {
      keyIdentifier             [0] KeyIdentifier           OPTIONAL,
      authorityCertIssuer       [1] GeneralNames            OPTIONAL,
      authorityCertSerialNumber [2] CertificateSerialNumber OPTIONAL  }

(https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.1)

Is there some place that says this violates an RFC?

Hello, I created an empty AuthorityKeyIdentifier (AKI) extension where all three subfields are Null. Cryptography accepted this CRL file. Does this violate the following statement in RFC 5280:

The identification can be based on either the key identifier (the subject key identifier in the CRL signer's certificate) or the issuer name and serial number.

empty_aki.zip

onepeople158 avatar Jun 08 '25 08:06 onepeople158

I'm not sure that it does.

alex avatar Jun 08 '25 12:06 alex

I'm not sure that it does.

Hello, the use of an empty sequence for the AKI extension is unreasonable. However, RFC 5280 does not explicitly state that the AKI extension sequence must not be empty—it only specifies that the IDP extension must not be an empty sequence. According to RFC 5280, the AKI extension may be based on the key identifier (the subject key identifier in the issuer certificate) or the issuer name and serial number.

onepeople158 avatar Jun 09 '25 02:06 onepeople158

I think that's a good summary, the fact that they use the ALL CAPS MAY means that MAY carries this definition:

MAY   This word, or the adjective "OPTIONAL", mean that an item is
   truly optional.  One vendor may choose to include the item because a
   particular marketplace requires it or because the vendor feels that
   it enhances the product while another vendor may omit the same item.
   An implementation which does not include a particular option MUST be
   prepared to interoperate with another implementation which does
   include the option, though perhaps with reduced functionality. In the
   same vein an implementation which does include a particular option
   MUST be prepared to interoperate with another implementation which
   does not include the option (except, of course, for the feature the
   option provides.)

Which seems to indicate it's not required to be?

The whole RFC is written in a confusing way. Unless there's a clear reason our behavior either a) violates the spec, b) harms interoperability, c) is a security issue, I'd be inclined to leave it as is.

alex avatar Jun 09 '25 02:06 alex

I think that's a good summary, the fact that they use the ALL CAPS MAY means that MAY carries this definition:

MAY   This word, or the adjective "OPTIONAL", mean that an item is
   truly optional.  One vendor may choose to include the item because a
   particular marketplace requires it or because the vendor feels that
   it enhances the product while another vendor may omit the same item.
   An implementation which does not include a particular option MUST be
   prepared to interoperate with another implementation which does
   include the option, though perhaps with reduced functionality. In the
   same vein an implementation which does include a particular option
   MUST be prepared to interoperate with another implementation which
   does not include the option (except, of course, for the feature the
   option provides.)

Which seems to indicate it's not required to be?

The whole RFC is written in a confusing way. Unless there's a clear reason our behavior either a) violates the spec, b) harms interoperability, c) is a security issue, I'd be inclined to leave it as is.

I understand.

onepeople158 avatar Jun 09 '25 02:06 onepeople158

This issue has been waiting for a reporter response for 3 days. It will be auto-closed if no activity occurs in the next 5 days.

github-actions[bot] avatar Jun 13 '25 00:06 github-actions[bot]

This issue has not received a reporter response and has been auto-closed. If the issue is still relevant please leave a comment and we can reopen it.

github-actions[bot] avatar Jun 19 '25 00:06 github-actions[bot]