CRL linter fixes

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.5 - 2024-10-03
+
+### New features/enhancements
+
+### Fixes
+
 ## 0.11.4 - 2024-08-29
 
 ### New features/enhancements

VERSION.txt

@@ -1 +1 @@
-0.11.4
+0.11.5

pkilint/bin/lint_crl.py

@@ -49,7 +49,7 @@ def main(cli_args=None) -> int:
 
     if args.profile == 'BR':
         doc_additional_validators.append(
-            cabf_crl.create_reason_code_validator(crl_type)
+            cabf_crl.CabfCrlReasonCodeAllowlistValidator(crl_type)
         )
 
         validity_additional_validators.append(

pkilint/cabf/cabf_crl.py

@@ -36,23 +36,25 @@ def create_validity_period_validator(
     )
 
 
-def create_reason_code_validator(
-        crl_type: crl.CertificateRevocationListType
-):
-    allowed_reasons = [
-        rfc5280.CRLReason.namedValues[r]
-        for r in [
-            'keyCompromise',
-            'affiliationChanged',
-            'superseded',
-            'cessationOfOperation',
-            'privilegeWithdrawn',
+class CabfCrlReasonCodeAllowlistValidator(crl_extension.CrlReasonCodeAllowlistValidator):
+    VALIDATION_PROHIBITED_CRL_REASON_CODE = validation.ValidationFinding(
+        validation.ValidationFindingSeverity.ERROR,
+        'cabf.crl_prohibited_reason_code'
+    )
+
+    def __init__(self, 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'])
+        if crl_type == crl.CertificateRevocationListType.ARL:
+            allowed_reasons.append(rfc5280.CRLReason.namedValues['cACompromise'])
 
-    return crl_extension.CrlReasonCodeAllowlistValidator(
-        allowed_reason_codes=allowed_reasons
-    )
+        super().__init__(allowed_reasons, self.VALIDATION_PROHIBITED_CRL_REASON_CODE)

pkilint/pkix/certificate/__init__.py

@@ -293,15 +293,7 @@ def create_extensions_validator_container(additional_validators=None):
             certificate_extension.KeyUsageCriticalityValidator(),
             certificate_extension.KeyUsageValidator(),
             general_name.UriSyntaxValidator(pdu_class=rfc5280.CPSuri),
-            general_name.GeneralNameUriSyntaxValidator(),
-            general_name.GeneralNameDnsNameSyntaxValidator(),
-            general_name.GeneralNameIpAddressSyntaxValidator(),
-            general_name.GeneralNameMailboxAddressSyntaxValidator(),
-            general_name.SmtpUTF8MailboxValidator(),
-            general_name.GeneralNameDnsNameDomainNameLengthValidator(),
-            general_name.GeneralNameUriDomainNameLengthValidator(),
-            general_name.GeneralNameRfc822NameDomainNameLengthValidator(),
-            general_name.SmtpUTF8MailboxDomainNameLengthValidator(),
+            general_name.GeneralNameValidatorContainer(),
             certificate_extension.DuplicatePolicyValidator(),
             certificate_extension.CrlDpCriticalityValidator(),
             certificate_extension.NameConstraintsCriticalityValidator(),

pkilint/pkix/crl/__init__.py

@@ -75,10 +75,9 @@ def create_validity_validator_container(additional_validators=None):
         additional_validators = []
     return validation.ValidatorContainer(
         validators=[
-                       crl_validity.CrlSaneValidityPeriodValidator(),
                        time.TimeCorrectEncodingValidator(),
                    ] + additional_validators,
-        path='certificateList.tbsCertList'
+        pdu_class=rfc5280.Time
     )
 
 
@@ -116,12 +115,10 @@ def create_pkix_crl_validator_container(
         crl_extension.CrlReasonCodeCriticalityValidator(),
         time.UtcTimeCorrectSyntaxValidator(),
         time.GeneralizedTimeCorrectSyntaxValidator(),
+        crl_validity.CrlSaneValidityPeriodValidator(),
         pkix.CertificateSerialNumberValidator(),
         crl_extension.CrlNumberValueValidator(),
-        general_name.GeneralNameIpAddressSyntaxValidator(),
-        general_name.GeneralNameMailboxAddressSyntaxValidator(),
-        general_name.GeneralNameIpAddressSyntaxValidator(),
-        general_name.GeneralNameUriSyntaxValidator(),
+        general_name.GeneralNameValidatorContainer(),
     ]
 
     return validation.ValidatorContainer(

pkilint/pkix/crl/crl_extension.py

@@ -93,22 +93,18 @@ def __init__(self):
 
 
 class CrlReasonCodeAllowlistValidator(validation.Validator):
-    VALIDATION_PROHIBITED_CRL_REASON_CODE = validation.ValidationFinding(
-        validation.ValidationFindingSeverity.ERROR,
-        'pkix.crl_prohibited_reason_code'
-    )
-
-    def __init__(self, *, allowed_reason_codes):
+    def __init__(self, allowed_reason_codes, prohibited_reason_code_validation: validation.ValidationFinding):
         self._allowed_reason_codes = allowed_reason_codes
+        self._prohibited_reason_code_validation = prohibited_reason_code_validation
 
         super().__init__(
             pdu_class=rfc5280.CRLReason,
-            validations=[self.VALIDATION_PROHIBITED_CRL_REASON_CODE]
+            validations=[self._prohibited_reason_code_validation]
         )
 
     def validate(self, node):
         if node.pdu not in self._allowed_reason_codes:
             raise validation.ValidationFindingEncountered(
-                self.VALIDATION_PROHIBITED_CRL_REASON_CODE,
+                self._prohibited_reason_code_validation,
                 f'Prohibited reason code "{node.pdu}"'
             )

pkilint/pkix/crl/crl_validity.py

@@ -11,6 +11,6 @@ class CrlSaneValidityPeriodValidator(time.SaneValidityPeriodValidator):
     def __init__(self):
         super().__init__(
             end_validity_node_retriever=lambda n: n.navigate('^.nextUpdate'),
-            path='certificateList.tbsCertificateList.thisUpdate',
+            path='certificateList.tbsCertList.thisUpdate',
             validation=self.VALIDATION_NEGATIVE_VALIDITY_PERIOD
         )

pkilint/pkix/general_name.py

@@ -48,6 +48,24 @@ def create_generalname_type_predicate(generalname_type):
         raise ValueError(f'Invalid type specified: {generalname_type}')
 
 
+class GeneralNameValidatorContainer(validation.ValidatorContainer):
+    def __init__(self):
+        super().__init__(
+            validators=[
+                GeneralNameUriSyntaxValidator(),
+                GeneralNameDnsNameSyntaxValidator(),
+                GeneralNameIpAddressSyntaxValidator(),
+                GeneralNameMailboxAddressSyntaxValidator(),
+                SmtpUTF8MailboxValidator(),
+                GeneralNameDnsNameDomainNameLengthValidator(),
+                GeneralNameUriDomainNameLengthValidator(),
+                GeneralNameRfc822NameDomainNameLengthValidator(),
+                SmtpUTF8MailboxDomainNameLengthValidator(),
+            ],
+            pdu_class=rfc5280.GeneralName
+        )
+
+
 class UriSyntaxValidator(validation.Validator):
     VALIDATION_INVALID_URI_SYNTAX = validation.ValidationFinding(
         validation.ValidationFindingSeverity.ERROR,

pkilint/pkix/time.py

@@ -85,18 +85,15 @@ class TimeCorrectEncodingValidator(validation.Validator):
 
     def __init__(self):
         super().__init__(pdu_class=rfc5280.Time, validations=[
-            VALIDATION_INVALID_TIME_SYNTAX,
             self.VALIDATION_WRONG_TIME_TYPE,
         ])
 
     def validate(self, node):
         try:
             parsed_datetime = parse_time_node(node)
-        except ValueError as e:
-            raise validation.ValidationFindingEncountered(
-                VALIDATION_INVALID_TIME_SYNTAX,
-                str(e)
-            )
+        except ValueError:
+            # the time syntax validator will report any errors
+            return
 
         if ('generalTime' in node.children) and (
                 parsed_datetime.year >= 1950) and (

tests/integration_certificate/__init__.py

@@ -1,96 +1,23 @@
-import csv
-import io
-import typing
-from os import path
+import functools
 from pathlib import Path
 
-from pkilint import loader, validation, finding_filter as result_filter
+from pkilint import loader
+from tests import integration_test_common
 
 _FIXTURE_DIR = Path(__file__).parent.resolve()
 
 _CERT_END_ASCII_ARMOR = '-----END CERTIFICATE-----'
 
 
-class TestFinding(typing.NamedTuple):
-    node_path: str
-    validator: str
-    severity: validation.ValidationFindingSeverity
-    code: str
-    message: typing.Optional[str]
+def register_test(module, file, test_name, validator, filters=None):
+    if hasattr(module, test_name):
+        raise ValueError(f'Duplicate test name in {module}: {test_name}')
 
-    @staticmethod
-    def from_dict(d):
-        return TestFinding(d['node_path'], d['validator'], validation.ValidationFindingSeverity[d['severity']],
-                           d['code'], None if not d['message'] else d['message'])
-
-    @staticmethod
-    def from_result_and_finding_description(result: validation.ValidationResult,
-                                            finding_description: validation.ValidationFindingDescription):
-        return TestFinding(
-            result.node.path,
-            str(result.validator),
-            finding_description.finding.severity,
-            finding_description.finding.code,
-            finding_description.message
-        )
-
-    def __repr__(self):
-        with io.StringIO() as s:
-            c = csv.writer(s)
-
-            c.writerow([self.node_path, self.validator, str(self.severity), self.code, self.message])
-
-            return s.getvalue()
-
-    def __str__(self):
-        return repr(self)
-
-
-def certificate_test_file(file_path):
-    with open(file_path, 'r', encoding='utf-8') as f:
-        lines = [l.strip() for l in f.readlines() if l.strip() != '']
-
-    assert any((_CERT_END_ASCII_ARMOR in line for line in lines))
-
-    pem_text = ''
-
-    line_num = -1
-    for line_num, line in enumerate(lines):
-        pem_text += line
-
-        if _CERT_END_ASCII_ARMOR in line:
-            break
-
-    with io.StringIO('\n'.join(lines[line_num + 1:])) as s:
-        c = csv.DictReader(s)
-
-        expected_findings = set((TestFinding.from_dict(row) for row in c))
-
-        cert = loader.load_pem_certificate(pem_text, path.basename(file_path))
-        cert.decode()
-
-    return cert, expected_findings
-
-
-def run_test(test_file_path, validator, filters=None):
-    if filters is None:
-        filters = []
-
-    cert, expected_findings = certificate_test_file(test_file_path)
-
-    results = validator.validate(cert.root)
-    results, _ = result_filter.filter_results(filters, results)
-
-    actual_findings = set()
-
-    for result in results:
-        finding_descriptions = [fd for fd in result.finding_descriptions]
-
-        for finding_description in finding_descriptions:
-            actual_findings.add(TestFinding.from_result_and_finding_description(result, finding_description))
-
-    missing_findings = expected_findings - actual_findings
-    unexpected_findings = actual_findings - expected_findings
-
-    assert not any(missing_findings) and not any(unexpected_findings), (
-        f'Missing findings: {missing_findings}\nUnexpected findings: {unexpected_findings}')
+    setattr(module, test_name, functools.partial(
+        integration_test_common.run_test,
+        _CERT_END_ASCII_ARMOR,
+        loader.load_pem_certificate,
+        file,
+        validator,
+        filters)
+    )

tests/integration_certificate/test_cabf_serverauth_cert.py

@@ -1,12 +1,11 @@
-import functools
 import glob
 import sys
 from os import path
 
 from pkilint.cabf import serverauth
 from pkilint.cabf.serverauth import serverauth_constants
 from pkilint.pkix import certificate
-from tests import integration_certificate
+from tests.integration_certificate import register_test
 
 this_module = sys.modules[__name__]
 
@@ -27,6 +26,6 @@
 
         file_no_ext, _ = path.splitext(path.basename(file))
 
-        func_name = f'test_{certificate_type}_{file_no_ext}'
+        test_name = f'test_{certificate_type}_{file_no_ext}'
 
-        setattr(this_module, func_name, functools.partial(integration_certificate.run_test, file, validator, filters))
+        register_test(this_module, file, test_name, validator, filters)

tests/integration_certificate/test_cabf_smime_cert.py

@@ -3,10 +3,11 @@
 import sys
 from os import path
 
+import tests.integration_test_common
 from pkilint.cabf import smime
 from pkilint.cabf.smime import smime_constants
 from pkilint.pkix import certificate
-from tests import integration_certificate
+from tests.integration_certificate import register_test
 
 this_module = sys.modules[__name__]
 
@@ -29,6 +30,6 @@
 
             file_no_ext, _ = path.splitext(path.basename(file))
 
-            func_name = f'test_{validation_level}-{generation}_{file_no_ext}'
+            test_name = f'test_{validation_level}-{generation}_{file_no_ext}'
 
-            setattr(this_module, func_name, functools.partial(integration_certificate.run_test, file, validator))
+            register_test(this_module, file, test_name, validator)

tests/integration_certificate/test_etsi_cert.py

@@ -3,10 +3,11 @@
 import sys
 from os import path
 
+import tests.integration_test_common
 from pkilint import etsi
 from pkilint.etsi import etsi_constants
 from pkilint.pkix import certificate
-from tests import integration_certificate
+from tests.integration_certificate import register_test
 
 this_module = sys.modules[__name__]
 
@@ -27,6 +28,6 @@
 
         file_no_ext, _ = path.splitext(path.basename(file))
 
-        func_name = f'test_{certificate_type}_{file_no_ext}'
+        test_name = f'test_{certificate_type}_{file_no_ext}'
 
-        setattr(this_module, func_name, functools.partial(integration_certificate.run_test, file, validator, filters))
+        register_test(this_module, file, test_name, validator, filters)

tests/integration_certificate/test_pkix_cert.py

@@ -3,8 +3,9 @@
 import sys
 from os import path
 
+import tests.integration_test_common
 from pkilint.pkix import certificate, name, extension
-from tests import integration_certificate
+from tests.integration_certificate import register_test
 
 cur_dir = path.dirname(__file__)
 test_dir = path.join(cur_dir, 'pkix')
@@ -31,6 +32,6 @@
 
     file_no_ext, _ = path.splitext(path.basename(file))
 
-    func_name = f'test_{file_no_ext}'
+    test_name = f'test_{file_no_ext}'
 
-    setattr(this_module, func_name, functools.partial(integration_certificate.run_test, file, validator))
+    register_test(this_module, file, test_name, validator)