Skip to content
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

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

Merged
merged 6 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
2 changes: 1 addition & 1 deletion VERSION.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.11.3
0.11.4
2 changes: 0 additions & 2 deletions pkilint/cabf/serverauth/finding_metadata.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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 | 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."""
Expand Down
2 changes: 0 additions & 2 deletions pkilint/cabf/smime/finding_metadata.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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 | 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"""
Expand Down
50 changes: 6 additions & 44 deletions pkilint/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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)

Expand Down
38 changes: 0 additions & 38 deletions pkilint/itu/bitstring.py
Original file line number Diff line number Diff line change
@@ -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
27 changes: 0 additions & 27 deletions pkilint/itu/string.py

This file was deleted.

4 changes: 0 additions & 4 deletions pkilint/pkix/certificate/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -339,7 +337,6 @@ def create_pkix_certificate_validator_container(
]

validators += [
PrintableStringConstraintValidator(),
certificate_validator.CorrectVersionValidator(),
pkix.CertificateSerialNumberValidator(),
certificate_validator.SignatureAlgorithmMatchValidator(),
Expand All @@ -348,7 +345,6 @@ def create_pkix_certificate_validator_container(
certificate_extension.KeyUsagePresenceValidator(),
time.UtcTimeCorrectSyntaxValidator(),
time.GeneralizedTimeCorrectSyntaxValidator(),
NamedBitStringMinimalEncodingValidator(),
certificate_validator.IssuerUniqueIdAbsenceValidator(),
certificate_validator.SubjectUniqueIdAbsenceValidator(),
]
Expand Down
5 changes: 1 addition & 4 deletions pkilint/pkix/crl/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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(),
Expand Down
19 changes: 19 additions & 0 deletions pkilint/pkix/crl/crl_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
4 changes: 0 additions & 4 deletions pkilint/pkix/ocsp/__init__.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ zOQ9r8SRI+9NirupPTkF5AKNe6kUhKJ1luB7S27ZkvB3tSTT3P593VVJvnzOjaA1
z6Cz+4+eRvcysqhrRgFlwI9TEwIDAQABo4IBrzCCAaswDAYDVR0TAQH/BAIwADAO
BgNVHQ8BAf8EBAMCB4AwHwYDVR0jBBgwFoAU1kQAMnyoDf+sT2tm7rWumyzFOFQw
HQYDVR0OBBYEFIkZWV4O8Wn1y71H4TT84pjMaTCRMBQGA1UdIAQNMAswCQYHZ4EM
AQUEAjAQBgNVHR8ECTAHMAWBAwUAIDBLBggrBgEFBQcBAQQ/MD0wOwYIKwYBBQUH
AQUEAjAQBgNVHR8ECTAHMAWBAwUAQDBLBggrBgEFBQcBAQQ/MD0wOwYIKwYBBQUH
MAKGL2h0dHA6Ly9yZXBvc2l0b3J5LmNhLmV4YW1wbGUuY29tL2lzc3VpbmdfY2Eu
ZGVyMB0GA1UdJQQWMBQGCCsGAQUFBwMEBggrBgEFBQcDAjCBtgYDVR0RBIGuMIGr
gRloYW5ha28ueWFtYWRhQGV4YW1wbGUuY29toCkGCisGAQQBgjcUAgOgGwwZaGFu
Expand All @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Loading