Use pyasn1-fasder for ASN.1 DER decoding (#98)

* Use pyasn1-fasder decoding by default * Delete references to removed validations * Change update template * Update CHANGELOG.md * Add trailing newline
digicert · Aug 28, 2024 · 5076b96 · 5076b96
1 parent df0c41d
commit 5076b96
Show file tree

Hide file tree

Showing 26 changed files with 120 additions and 269 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 All notable changes to this project from version 0.9.3 onwards are documented in this file.
 
+## 0.11.4 - 2024-08-28
+
+### New features/enhancements
+
+- Use pyasn1-fasder for ASN.1 DER decoding by default (#98)
+
 ## 0.11.3 - 2024-07-17
 
 ### Fixes

diff --git a/README.md b/README.md
@@ -429,6 +429,7 @@ pkilint is built on several open source packages. In particular, these packages
 | publicsuffixlist   | Mozilla Public License 2.0 (MPL 2.0) | ko-zu                                                          | https://github.com/ko-zu/psl                      |
 | pyasn1             | BSD License                          | Christian Heimes and Simon Pichugin                            | https://github.com/pyasn1/pyasn1                  |
 | pyasn1-alt-modules | BSD License                          | Russ Housley                                                   | https://github.com/russhousley/pyasn1-alt-modules |
+| pyasn1-fasder      | MIT License                          | Corey Bonnell                                                  | https://github.com/cbonnell/pyasn1-fasder         |
 | python-dateutil    | Apache Software License; BSD License | Gustavo Niemeyer                                               | https://github.com/dateutil/dateutil              |
 | python-iso639      | Apache Software License              | Jackson L. Lee                                                 | https://github.com/jacksonllee/iso639             |
 | validators         | MIT License                          | Konsta Vesterinen                                              | https://github.com/kvesteri/validators            |

diff --git a/VERSION.txt b/VERSION.txt
@@ -1 +1 @@
-0.11.3
+0.11.4
diff --git a/pkilint/cabf/serverauth/finding_metadata.csv b/pkilint/cabf/serverauth/finding_metadata.csv
@@ -184,8 +184,6 @@ ERROR,cabf.serverauth.subscriber_missing_reserved_policy_oid,Validates that the
 ERROR,cabf.serverauth.subscriber_prohibited_ku_present,Validates that the content of the key usage extension conforms with BR 7.1.2.7.11.
 ERROR,cabf.serverauth.subscriber_required_ku_missing,Validates that the content of the key usage extension conforms with BR 7.1.2.7.11.
 ERROR,cabf.serverauth.subscriber_stateprovince_and_locality_missing,"Validates that the stateOrProvinceName and/or localityName subject attributes are present, as per EVG 9.2.6, BR 7.1.2.7.3, and BR 7.1.2.7.4."
-ERROR,itu.bitstring_not_der_encoded,"X.690 2002-07, clause 11.2.2: ""Where ITU-T Rec. X.680 &#124; ISO/IEC 8824-1, 21.7, applies, the bitstring shall have all trailing 0 bits removed before it is encoded"""
-ERROR,itu.invalid_printablestring_character,"X.680 2002-07, clause 37.4: ""Table 8 lists the characters which can appear in the PrintableString type and PrintableString character abstract syntax"""
 ERROR,pkix.aki_with_cert_issuer_but_serial_number_absent,"RFC 5280 4.2.1.1: ""The identification MAY be based on either the key identifier (the subject key identifier in the issuer's certificate) or the issuer name and serial number"""
 ERROR,pkix.aki_with_serial_number_but_cert_issuer_absent,"RFC 5280 4.2.1.1: ""The identification MAY be based on either the key identifier (the subject key identifier in the issuer's certificate) or the issuer name and serial number"""
 ERROR,pkix.authority_information_access_extension_critical,"RFC 5280 4.2.2.1: ""Conforming CAs MUST mark this extension as non-critical."""

diff --git a/pkilint/cabf/smime/finding_metadata.csv b/pkilint/cabf/smime/finding_metadata.csv
@@ -63,8 +63,6 @@ ERROR,cabf.smime.unsupported_public_key_type,SMBR 7.1.3.1,
 ERROR,cabf.smime.usernotice_has_noticeref,SMBR 7.1.2.3 (a),"""If a qualifier of type id-qt-unotice (OID: 1.3.6.1.5.5.7.2.2) is included, then it SHALL contain explicitText and SHALL NOT contain noticeRef"""
 ERROR,iso.lei.invalid_lei_checksum,ISO 17442,LEI checksum character is incorrect
 ERROR,iso.lei.invalid_lei_format,ISO 17442,LEI value format is not correct
-ERROR,itu.bitstring_not_der_encoded,"X.690 2002-07, clause 11.2.2","""Where ITU-T Rec. X.680 &#124; ISO/IEC 8824-1, 21.7, applies, the bitstring shall have all trailing 0 bits removed before it is encoded"""
-ERROR,itu.invalid_printablestring_character,"X.680 2002-07, clause 37.4","""Table 8 lists the characters which can appear in the PrintableString type and PrintableString character abstract syntax"""
 ERROR,msft.invalid_user_principal_name_syntax,https://learn.microsoft.com/en-us/windows/win32/ad/naming-properties,"""A UPN is an Internet-style login name for a user based on the Internet standard RFC 822"""
 ERROR,pkix.aki_with_cert_issuer_but_serial_number_absent,RFC 5280 4.2.1.1,"""The identification MAY be based on either the key identifier (the subject key identifier in the issuer's certificate) or the issuer name and serial number"""
 ERROR,pkix.aki_with_serial_number_but_cert_issuer_absent,RFC 5280 4.2.1.1,"""The identification MAY be based on either the key identifier (the subject key identifier in the issuer's certificate) or the issuer name and serial number"""

diff --git a/pkilint/document.py b/pkilint/document.py
@@ -10,20 +10,12 @@
 from pyasn1.type.univ import (ObjectIdentifier, SequenceOfAndSetOfBase, SequenceAndSetBase,
                               Choice, BitString
                               )
+from pyasn1_fasder import decode_der
 
 logger = logging.getLogger(__name__)
 
 PATH_REGEX = re.compile(r'^((?P<doc_name>[^:]*):)?(?P<node_path>([^.]+\.)*[^.]+)?$')
 
-try:
-    # noinspection PyUnresolvedReferences
-    from pyasn1_fasder import decode_der
-
-    logging.info('Using pyasn1-fasder for ASN.1 DER decoding')
-    _USE_PYASN1_FASDER = True
-except ImportError:
-    _USE_PYASN1_FASDER = False
-
 
 class PDUNavigationFailedError(Exception):
     """Represents the failure to find the requested node in a document."""
@@ -299,42 +291,12 @@ def decode_substrate(source_document: Document, substrate: bytes,
                      )
         return next(iter(parent_node.children.values()))
 
-    if _USE_PYASN1_FASDER:
-        try:
-            decoded, _ = decode_der(substrate, asn1Spec=pdu_instance)
-        except (ValueError, PyAsn1Error) as e:
-            raise SubstrateDecodingFailedError(source_document, pdu_instance, parent_node, str(e)) from e
-
-        decoded_pdu_name = get_node_name_for_pdu(decoded)
-    else:
-        try:
-            decoded, rest = decode(substrate, asn1Spec=pdu_instance)
-        except (ValueError, PyAsn1Error) as e:
-            raise SubstrateDecodingFailedError(source_document, pdu_instance, parent_node, str(e)) from e
-
-        decoded_pdu_name = get_node_name_for_pdu(decoded)
-        type_name = decoded.__class__.__name__
+    try:
+        decoded, _ = decode_der(substrate, asn1Spec=pdu_instance)
+    except (ValueError, PyAsn1Error) as e:
+        raise SubstrateDecodingFailedError(source_document, pdu_instance, parent_node, str(e)) from e
 
-        if len(rest) > 0:
-            rest_hex = bytes(rest).hex()
-
-            raise SubstrateDecodingFailedError(
-                source_document, pdu_instance, parent_node,
-                f'{len(rest)} unexpected octet(s) following "{type_name}" TLV: "{rest_hex}"'
-            )
-
-        try:
-            encoded = encode(decoded)
-
-            substrate_is_der = encoded == substrate
-        except (ValueError, PyAsn1Error):
-            substrate_is_der = False
-
-        if not substrate_is_der:
-            raise SubstrateDecodingFailedError(
-                source_document, pdu_instance, parent_node,
-                f'Substrate of type "{type_name}" is not DER-encoded'
-            )
+    decoded_pdu_name = get_node_name_for_pdu(decoded)
 
     node = PDUNode(source_document, decoded_pdu_name, decoded, parent_node)
 

diff --git a/pkilint/itu/bitstring.py b/pkilint/itu/bitstring.py
@@ -1,41 +1,3 @@
-from pyasn1.codec.der.encoder import encode
-from pyasn1.type.univ import BitString
-
-from pkilint import validation
-
-
-class NamedBitStringMinimalEncodingValidator(validation.Validator):
-    VALIDATION_BIT_STRING_NOT_MINIMALLY_ENCODED = validation.ValidationFinding(
-        validation.ValidationFindingSeverity.ERROR,
-        'itu.bitstring_not_der_encoded'
-    )
-
-    def __init__(self):
-        super().__init__(
-            validations=[self.VALIDATION_BIT_STRING_NOT_MINIMALLY_ENCODED],
-            pdu_class=BitString,
-            predicate=lambda n: any(n.pdu.namedValues)
-        )
-
-    def validate(self, node):
-        # extract values then re-encode
-
-        asserted_values = ','.join((k for k in node.pdu.namedValues.keys() if has_named_bit(node, k)))
-
-        encoded = encode(node.pdu)
-
-        new_encoded = encode(type(node.pdu)(asserted_values), asn1Spec=node.pdu)
-
-        if encoded != new_encoded:
-            encoded_hex = encoded.hex()
-            new_encoded_hex = new_encoded.hex()
-
-            raise validation.ValidationFindingEncountered(
-                self.VALIDATION_BIT_STRING_NOT_MINIMALLY_ENCODED,
-                f'Expected: "{new_encoded_hex}", actual: "{encoded_hex}"'
-            )
-
-
 def has_named_bit(node, bit_name):
     bit = node.pdu.namedValues[bit_name]
     return len(node.pdu) > bit and node.pdu[bit] != 0
diff --git a/pkilint/itu/string.py b/pkilint/itu/string.py
diff --git a/pkilint/pkix/certificate/__init__.py b/pkilint/pkix/certificate/__init__.py
@@ -13,8 +13,6 @@
 
 from pkilint import validation, pkix, document
 from pkilint.document import Document, ValueDecoder
-from pkilint.itu.bitstring import NamedBitStringMinimalEncodingValidator
-from pkilint.itu.string import PrintableStringConstraintValidator
 from pkilint.pkix import (extension, time, name,
                           create_name_validator_container, general_name, algorithm
                           )
@@ -339,7 +337,6 @@ def create_pkix_certificate_validator_container(
     ]
 
     validators += [
-        PrintableStringConstraintValidator(),
         certificate_validator.CorrectVersionValidator(),
         pkix.CertificateSerialNumberValidator(),
         certificate_validator.SignatureAlgorithmMatchValidator(),
@@ -348,7 +345,6 @@ def create_pkix_certificate_validator_container(
         certificate_extension.KeyUsagePresenceValidator(),
         time.UtcTimeCorrectSyntaxValidator(),
         time.GeneralizedTimeCorrectSyntaxValidator(),
-        NamedBitStringMinimalEncodingValidator(),
         certificate_validator.IssuerUniqueIdAbsenceValidator(),
         certificate_validator.SubjectUniqueIdAbsenceValidator(),
     ]

diff --git a/pkilint/pkix/crl/__init__.py b/pkilint/pkix/crl/__init__.py
@@ -4,8 +4,6 @@
 
 from pkilint import validation, pkix, document
 from pkilint.document import Document
-from pkilint.itu.bitstring import NamedBitStringMinimalEncodingValidator
-from pkilint.itu.string import PrintableStringConstraintValidator
 from pkilint.pkix import name, extension, time, general_name
 from pkilint.pkix.crl import crl_validator, crl_extension, crl_validity
 
@@ -109,18 +107,17 @@ def create_pkix_crl_validator_container(
     ]
 
     validators += [
-        PrintableStringConstraintValidator(),
         crl_validator.VersionPresenceValidator(),
         crl_validator.CorrectVersionValidator(),
         crl_extension.CrlNumberPresenceValidator(),
         crl_extension.AuthorityKeyIdentifierPresenceValidator(),
         crl_validator.SignatureAlgorithmMatchValidator(),
+        crl_validator.RevokedCertificatesEmptyValidator(),
         crl_extension.CrlReasonCodeCriticalityValidator(),
         time.UtcTimeCorrectSyntaxValidator(),
         time.GeneralizedTimeCorrectSyntaxValidator(),
         pkix.CertificateSerialNumberValidator(),
         crl_extension.CrlNumberValueValidator(),
-        NamedBitStringMinimalEncodingValidator(),
         general_name.GeneralNameIpAddressSyntaxValidator(),
         general_name.GeneralNameMailboxAddressSyntaxValidator(),
         general_name.GeneralNameIpAddressSyntaxValidator(),

diff --git a/pkilint/pkix/crl/crl_validator.py b/pkilint/pkix/crl/crl_validator.py
@@ -42,3 +42,22 @@ def __init__(self):
                 'pkix.crl_signature_algorithm_match'
             )
         )
+
+
+class RevokedCertificatesEmptyValidator(validation.Validator):
+    VALIDATION_REVOKED_CERTIFICATES_EMPTY = validation.ValidationFinding(
+        validation.ValidationFindingSeverity.ERROR,
+        'pkix.crl_revoked_certificates_empty'
+    )
+
+    def __init__(self):
+        super().__init__(
+            validations=[self.VALIDATION_REVOKED_CERTIFICATES_EMPTY],
+            pdu_class=rfc5280.TBSCertList
+        )
+
+    def validate(self, node):
+        revoked_certificates = node.children.get('revokedCertificates')
+
+        if revoked_certificates is not None and not any(revoked_certificates.pdu):
+            raise validation.ValidationFindingEncountered(self.VALIDATION_REVOKED_CERTIFICATES_EMPTY)
diff --git a/pkilint/pkix/ocsp/__init__.py b/pkilint/pkix/ocsp/__init__.py
@@ -1,8 +1,6 @@
 from pyasn1_alt_modules import rfc6960
 
 from pkilint import document, validation
-from pkilint.itu.bitstring import NamedBitStringMinimalEncodingValidator
-from pkilint.itu.string import PrintableStringConstraintValidator
 from pkilint.pkix import time
 from pkilint.pkix.ocsp import ocsp_response, ocsp_basic_response, ocsp_validity
 
@@ -40,10 +38,8 @@ def create_pkix_ocsp_response_validator_container(
         ocsp_basic_response.OCSPBasicResponseCertsNotPresentValidator(),
         ocsp_basic_response.ResponderKeyHashIsSHA1HashValidator(),
         ocsp_validity.OCSPSaneValidityPeriodValidator(),
-        PrintableStringConstraintValidator(),
         time.UtcTimeCorrectSyntaxValidator(),
         time.GeneralizedTimeCorrectSyntaxValidator(),
-        NamedBitStringMinimalEncodingValidator(),
     ]
 
     return validation.ValidatorContainer(

diff --git a/setup.cfg b/setup.cfg
@@ -28,6 +28,7 @@ python_requires = >=3.9
 install_requires =
     pyasn1
     pyasn1-alt-modules >=0.4.3
+    pyasn1-fasder
     cryptography >=39
     iso3166
     # version is pinned due to https://github.com/python-validators/validators/issues/346

diff --git a/...ration_certificate/etsi/qncp_w_ov_eidas_pre_certificate/bad_bc_extension_encoding.crttest b/...ration_certificate/etsi/qncp_w_ov_eidas_pre_certificate/bad_bc_extension_encoding.crttest
@@ -47,7 +47,7 @@ PMeG9uZkYykeSL8vGkuW7QwDUXmnQ5U6hHjM8Wen
 -----END CERTIFICATE-----
 
 node_path,validator,severity,code,message
-certificate.tbsCertificate.extensions.5,ExtensionsDecodingValidator,FATAL,itu.invalid_asn1_syntax,"ASN.1 decoding failure occurred at ""certificate.tbsCertificate.extensions.5.extnValue"" with schema ""BasicConstraints"" corresponding to type OID 2.5.29.19: Substrate of type ""BasicConstraints"" is not DER-encoded"
+certificate.tbsCertificate.extensions.5,ExtensionsDecodingValidator,FATAL,itu.invalid_asn1_syntax,"ASN.1 decoding failure occurred at ""certificate.tbsCertificate.extensions.5.extnValue"" with schema ""BasicConstraints"" corresponding to type OID 2.5.29.19: Error decoding ""BasicConstraints"" TLV near substrate offset 0: Explicitly encoded default value"
 certificate.tbsCertificate.subject.rdnSequence,OvSubscriberAttributeAllowanceValidator,WARNING,cabf.serverauth.ov.common_name_attribute_present,
 certificate.tbsCertificate.subject.rdnSequence,OvSubscriberAttributeAllowanceValidator,WARNING,cabf.serverauth.ov.unknown_attribute_present,Unknown attribute present: 2.5.4.5
 certificate.tbsCertificate.extensions.1.extnValue.subjectKeyIdentifier,SubjectKeyIdentifierValidator,INFO,pkix.subject_key_identifier_method_1_identified,

diff --git a/tests/integration_certificate/pkix/crldp_dp_reasons_not_der.crttest b/tests/integration_certificate/pkix/crldp_dp_reasons_not_der.crttest
@@ -13,7 +13,7 @@ zOQ9r8SRI+9NirupPTkF5AKNe6kUhKJ1luB7S27ZkvB3tSTT3P593VVJvnzOjaA1
 z6Cz+4+eRvcysqhrRgFlwI9TEwIDAQABo4IBrzCCAaswDAYDVR0TAQH/BAIwADAO
 BgNVHQ8BAf8EBAMCB4AwHwYDVR0jBBgwFoAU1kQAMnyoDf+sT2tm7rWumyzFOFQw
 HQYDVR0OBBYEFIkZWV4O8Wn1y71H4TT84pjMaTCRMBQGA1UdIAQNMAswCQYHZ4EM
-AQUEAjAQBgNVHR8ECTAHMAWBAwUAIDBLBggrBgEFBQcBAQQ/MD0wOwYIKwYBBQUH
+AQUEAjAQBgNVHR8ECTAHMAWBAwUAQDBLBggrBgEFBQcBAQQ/MD0wOwYIKwYBBQUH
 MAKGL2h0dHA6Ly9yZXBvc2l0b3J5LmNhLmV4YW1wbGUuY29tL2lzc3VpbmdfY2Eu
 ZGVyMB0GA1UdJQQWMBQGCCsGAQUFBwMEBggrBgEFBQcDAjCBtgYDVR0RBIGuMIGr
 gRloYW5ha28ueWFtYWRhQGV4YW1wbGUuY29toCkGCisGAQQBgjcUAgOgGwwZaGFu
@@ -32,8 +32,7 @@ wGlaWz8x7WKMUf9+POXbTpOz1qlott9ODUkZNwWA6gFRxMWn2leMv/eYQwCNhAbT
 n+QDBx22AIECDkySEND7mAM1EpfaYajbVqZ6oM5nbtv4JsKPSbKEOQh7Rbs/7jqE
 V2zAlZQn7G0hl+EZvO+48fFzaSRN8jATyCcMsxLldw==
 -----END CERTIFICATE-----
+
 node_path,validator,severity,code,message
 certificate.tbsCertificate.extensions.3.extnValue.subjectKeyIdentifier,SubjectKeyIdentifierValidator,INFO,pkix.subject_key_identifier_method_1_identified,
-certificate.tbsCertificate.extensions.5.extnValue.cRLDistributionPoints.0,DistributionPointValidator,ERROR,pkix.distribution_point_does_not_contain_name_or_issuer,
-certificate.tbsCertificate.extensions.5.extnValue.cRLDistributionPoints.0.reasons,NamedBitStringMinimalEncodingValidator,ERROR,itu.bitstring_not_der_encoded,"Expected: ""810100"", actual: ""8103050020"""
-
+certificate.tbsCertificate.extensions.5,ExtensionsDecodingValidator,FATAL,itu.invalid_asn1_syntax,"ASN.1 decoding failure occurred at ""certificate.tbsCertificate.extensions.5.extnValue"" with schema ""CRLDistributionPoints"" corresponding to type OID 2.5.29.31: Error decoding ""ReasonFlags"" TLV near substrate offset 4: Trailing zero bit in named BIT STRING"
diff --git a/tests/integration_certificate/pkix/old_lamps_smime_example.crttest b/tests/integration_certificate/pkix/old_lamps_smime_example.crttest
@@ -19,8 +19,8 @@ ZGil0pxx9jdMS5qaTdjb66GvPpkQI1uH4E9xiYbJu5bD+SX0Sgzih79GEhaP8vjc
 w6+P//nJ3ExJkVT7OvIJmwGvV0ULtmsghoigcd2BBc/fOKdbyIBmJBe152dd02EW
 6FwMfHKDtHO8k+/XBeZcxF0=
 -----END CERTIFICATE-----
+
 node_path,validator,severity,code,message
 certificate.tbsCertificate.serialNumber,CertificateSerialNumberValidator,ERROR,pkix.certificate_serial_number_out_of_range,"ASN.1 constraint failed: Invalid value outside range 1 - 730750818665451459101842416358141509827966271487 on content ""-741604493682452113825656873529250578000121114"""
 certificate.tbsCertificate.extensions.4.extnValue.subjectKeyIdentifier,SubjectKeyIdentifierValidator,NOTICE,pkix.unknown_subject_key_identifier_calculation_method,
-certificate.tbsCertificate.extensions.3.extnValue.keyUsage,NamedBitStringMinimalEncodingValidator,ERROR,itu.bitstring_not_der_encoded,"Expected: ""03020520"", actual: ""0303072000"""
-
+certificate.tbsCertificate.extensions.3,ExtensionsDecodingValidator,FATAL,itu.invalid_asn1_syntax,"ASN.1 decoding failure occurred at ""certificate.tbsCertificate.extensions.3.extnValue"" with schema ""KeyUsage"" corresponding to type OID 2.5.29.15: Error decoding ""KeyUsage"" TLV near substrate offset 0: Trailing zero bit in named BIT STRING"
diff --git a/tests/integration_certificate/pkix/root_bad_ku_encoding.crttest b/tests/integration_certificate/pkix/root_bad_ku_encoding.crttest
@@ -11,7 +11,7 @@ P62jQzBBMA8GA1UdEwEB/wQFMAMBAf8wDwYDVR0PAQH/BAUDAwcGADAdBgNVHQ4EFgQUo0EGrJBt
 0UrrdaVKEJmzsaGLSvcwCgYIKoZIzj0EAwIDRwAwRAIgB+ZU2g6gWrKuEZ+Hxbb/ad4lvvigtwjz
 RM4q3wghDDcCIC0mA6AFvWvR9lz4ZcyGbbOcNEhjhAnFjXca4syc4XR7
 -----END CERTIFICATE-----
+
 node_path,validator,severity,code,message
 certificate.tbsCertificate.extensions.2.extnValue.subjectKeyIdentifier,SubjectKeyIdentifierValidator,INFO,pkix.subject_key_identifier_method_1_identified,
-certificate.tbsCertificate.extensions.1.extnValue.keyUsage,NamedBitStringMinimalEncodingValidator,ERROR,itu.bitstring_not_der_encoded,"Expected: ""03020106"", actual: ""0303070600"""
-
+certificate.tbsCertificate.extensions.1,ExtensionsDecodingValidator,FATAL,itu.invalid_asn1_syntax,"ASN.1 decoding failure occurred at ""certificate.tbsCertificate.extensions.1.extnValue"" with schema ""KeyUsage"" corresponding to type OID 2.5.29.15: Error decoding ""KeyUsage"" TLV near substrate offset 0: Trailing zero bit in named BIT STRING"