Gracefully handle unsupported key types for AKI presence validator. V…

…arious signature verification fixes and cleanups (#134)

pkilint/bin/lint_pkix_signer_signee_cert_chain.py

@@ -6,7 +6,7 @@
 from pyasn1_alt_modules import rfc5280
 
 from pkilint import loader, document, cli_util, validation, pkix, report
-from pkilint.pkix import certificate, name, extension, algorithm
+from pkilint.pkix import certificate, name, extension, algorithm, key
 from pkilint.pkix.certificate import certificate_extension, certificate_key
 
 
@@ -19,8 +19,8 @@ def create_decoder_validation_container():
             path="certificate.tbsCertificate.signature",
         ),
         certificate.create_spki_decoder(
-            certificate_key.SUBJECT_PUBLIC_KEY_ALGORITHM_IDENTIFIER_MAPPINGS,
-            certificate_key.SUBJECT_KEY_PARAMETER_ALGORITHM_IDENTIFIER_MAPPINGS,
+            key.SUBJECT_PUBLIC_KEY_ALGORITHM_IDENTIFIER_MAPPINGS,
+            key.SUBJECT_KEY_PARAMETER_ALGORITHM_IDENTIFIER_MAPPINGS,
         ),
     ]
 

pkilint/cabf/cabf_key.py

@@ -1,7 +1,7 @@
 from pyasn1_alt_modules import rfc3279, rfc5480, rfc5280
 
+from pkilint.pkix import key
 from pkilint import validation
-from pkilint.pkix.certificate import certificate_key
 
 
 class RsaKeyValidator(validation.Validator):
@@ -238,7 +238,7 @@ def __init__(self):
 
     def validate(self, node):
         try:
-            key_obj = certificate_key.convert_spki_to_object(node)
+            key_obj = key.convert_spki_to_object(node)
 
             if key_obj is None:
                 raise validation.ValidationFindingEncountered(

pkilint/cabf/serverauth/finding_metadata.csv

@@ -303,6 +303,7 @@ WARNING,pkix.san_extension_is_critical_non_empty_subject,"RFC 5280 4.2.1.6: "" W
 NOTICE,cabf.ca_certificate_no_digital_signature_bit,CA certificates with no digitalSignature bit asserted imply that the CA Private Key cannot sign OCSP responses
 NOTICE,cabf.serverauth.unparsed_common_name_encountered,Validates that the content of the commonName attribute conforms to BR 7.1.4.3.
 NOTICE,cabf.serverauth.unparsed_san_extension_encountered,Validates that the content of the commonName attribute conforms to BR 7.1.4.3.
+NOTICE,pkix.aki_absent_self_issued_and_unsupported_public_key_algorithm,Authority Key Identifier extension is absent is a self-issued certificate and the certificate certifies a public key of an unsupported algorithm.
 NOTICE,pkix.certificate_policies_policy_has_qualifier,"RFC 5280 4.2.1.4: ""To promote interoperability, this profile RECOMMENDS that policy information terms consist of only an OID. Where an OID alone is insufficient, this profile strongly recommends that the use of qualifiers be limited to those identified in this section"""
 NOTICE,pkix.ldap_uri_not_validated,": Notice that the linter encountered a LDAP URI but did not validate the correctness of the URI, as support for LDAP validation has not (yet) been implemented. This NOTICE should probably be of a lower severity or supressed entirely."
 NOTICE,pkix.unknown_subject_key_identifier_calculation_method,RFC 5280 4.2.1.2: The Subject key identifier was not calculated using one of the algorithms defined in RFC 5280

pkilint/cabf/smime/finding_metadata.csv

@@ -144,6 +144,7 @@ NOTICE,cabf.smime.unparsed_attribute_value_encountered,,"A validator encountered
 NOTICE,cabf.smime.unparsed_common_name_value,,"A validator encountered a common name attribute value that was not decoded, so its validations could not performed"
 NOTICE,googl.gmail.authority_info_access_ca_issuers_missing,https://support.google.com/a/answer/7300887?hl=en&ref_topic=9061730&sjid=12609481378327192584-NA,"""caIssuers and, if present, ocsp, must contain at least one publicly accessible HTTP uniformResourceIdentifier."""
 NOTICE,googl.prohibited_rsa_modulus_length,https://support.google.com/a/answer/7300887?hl=en&ref_topic=9061730&sjid=12609481378327192584-NA,"""rsaEncryption with an RSA modulus of 2048, 3072, or 4096"""
+NOTICE,pkix.aki_absent_self_issued_and_unsupported_public_key_algorithm,,Authority Key Identifier extension is absent is a self-issued certificate and the certificate certifies a public key of an unsupported algorithm.
 NOTICE,pkix.certificate_policies_policy_has_qualifier,RFC 5280 4.2.1.4,"""To promote interoperability, this profile RECOMMENDS that policy information terms consist of only an OID. Where an OID alone is insufficient, this profile strongly recommends that the use of qualifiers be limited to those identified in this section"""
 NOTICE,pkix.ldap_uri_not_validated,,"Notice that the linter encountered a LDAP URI but did not validate the correctness of the URI, as support for LDAP validation has not (yet) been implemented. This NOTICE should probably be of a lower severity or supressed entirely."
 NOTICE,pkix.unknown_subject_key_identifier_calculation_method,RFC 5280 4.2.1.2,The Subject key identifier was not calculated using one of the algorithms defined in RFC 5280

pkilint/pkix/certificate/__init__.py

@@ -3,15 +3,7 @@
 import logging
 from typing import Set, Optional
 
-from cryptography import x509, exceptions
-from cryptography.hazmat.primitives.asymmetric import (
-    padding,
-    rsa,
-    dsa,
-    ec,
-    ed25519,
-    ed448,
-)
+from cryptography import x509
 from pyasn1.codec.der.encoder import encode
 from pyasn1.type import univ
 from pyasn1.type.base import Asn1Type
@@ -26,6 +18,7 @@
     create_name_validator_container,
     general_name,
     algorithm,
+    key,
 )
 from pkilint.pkix.certificate import (
     certificate_validity,
@@ -129,33 +122,30 @@ def is_self_issued(self):
 
         return encode(issuer_node.pdu) == encode(subject_node.pdu)
 
+    @functools.cached_property
+    def public_key_object(self):
+        return key.convert_spki_to_object(
+            self.root.navigate("tbsCertificate.subjectPublicKeyInfo")
+        )
+
     @functools.cached_property
     def is_self_signed(self):
         if not self.is_self_issued:
             return False
 
-        public_key = self.cryptography_object.public_key()
-        tbs_octets = self.cryptography_object.tbs_certificate_bytes
-        hash_alg = self.cryptography_object.signature_hash_algorithm
-        signature_octets = self.cryptography_object.signature
+        public_key = self.public_key_object
 
-        try:
-            if isinstance(public_key, rsa.RSAPublicKey):
-                public_key.verify(
-                    signature_octets, tbs_octets, padding.PKCS1v15(), hash_alg
-                )
-            elif isinstance(public_key, dsa.DSAPublicKey):
-                public_key.verify(signature_octets, tbs_octets, hash_alg)
-            elif isinstance(public_key, ec.EllipticCurvePublicKey):
-                public_key.verify(signature_octets, tbs_octets, ec.ECDSA(hash_alg))
-            elif isinstance(public_key, ed25519.Ed25519PublicKey):
-                public_key.verify(signature_octets, tbs_octets)
-            elif isinstance(public_key, ed448.Ed448PublicKey):
-                public_key.verify(signature_octets, tbs_octets)
-        except exceptions.InvalidSignature:
+        # return False if the certificate certifies an unsupported public key type
+        if public_key is None:
             return False
 
-        return True
+        tbs_octets = self.cryptography_object.tbs_certificate_bytes
+        signature_hash_alg = self.cryptography_object.signature_hash_algorithm
+        signature_octets = self.cryptography_object.signature
+
+        return key.verify_signature(
+            public_key, tbs_octets, signature_octets, signature_hash_alg
+        )
 
     def get_extension_by_oid(self, oid):
         tbs_cert = self.root.children["tbsCertificate"]
@@ -383,8 +373,8 @@ def create_decoding_validators(
             path="certificate.tbsCertificate.signature",
         ),
         create_spki_decoder(
-            certificate_key.SUBJECT_PUBLIC_KEY_ALGORITHM_IDENTIFIER_MAPPINGS,
-            certificate_key.SUBJECT_KEY_PARAMETER_ALGORITHM_IDENTIFIER_MAPPINGS,
+            key.SUBJECT_PUBLIC_KEY_ALGORITHM_IDENTIFIER_MAPPINGS,
+            key.SUBJECT_KEY_PARAMETER_ALGORITHM_IDENTIFIER_MAPPINGS,
         ),
         create_policy_qualifier_decoder(
             certificate_extension.CERTIFICATE_POLICY_QUALIFIER_MAPPINGS

pkilint/pkix/certificate/certificate_extension.py

@@ -628,19 +628,46 @@ class AuthorityKeyIdentifierPresenceValidator(validation.Validator):
         "pkix.authority_key_identifier_extension_absent",
     )
 
+    VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM = validation.ValidationFinding(
+        validation.ValidationFindingSeverity.NOTICE,
+        "pkix.aki_absent_self_issued_and_unsupported_public_key_algorithm",
+    )
+
     def __init__(self):
         super().__init__(
-            validations=[self.VALIDATION_AKI_EXTENSION_NOT_PRESENT],
+            validations=[
+                self.VALIDATION_AKI_EXTENSION_NOT_PRESENT,
+                self.VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM,
+            ],
             pdu_class=rfc5280.Certificate,
         )
 
     def validate(self, node):
-        ext = node.document.get_extension_by_oid(rfc5280.id_ce_authorityKeyIdentifier)
+        cert_doc = node.document
 
-        if ext is None and not node.document.is_self_signed:
-            raise validation.ValidationFindingEncountered(
-                self.VALIDATION_AKI_EXTENSION_NOT_PRESENT
-            )
+        ext = cert_doc.get_extension_by_oid(rfc5280.id_ce_authorityKeyIdentifier)
+
+        if ext is not None:
+            return
+
+        if not node.document.is_self_signed:
+            # check if certificate is not considered self-signed merely due to unsupported key algorithm
+            if node.document.is_self_issued and cert_doc.public_key_object is None:
+                key_oid = str(
+                    node.navigate(
+                        "tbsCertificate.subjectPublicKeyInfo.algorithm.algorithm"
+                    ).pdu
+                )
+
+                raise validation.ValidationFindingEncountered(
+                    self.VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM,
+                    f"Self-issued certificate certifies a public key of unsupported algorithm: {key_oid}",
+                )
+            # if the cert is not self-issued and/or certifies a supported key type, then report the PKIX error
+            else:
+                raise validation.ValidationFindingEncountered(
+                    self.VALIDATION_AKI_EXTENSION_NOT_PRESENT
+                )
 
 
 class AuthorityInformationAccessCriticalityValidator(ExtensionCriticalityValidator):

pkilint/pkix/certificate/certificate_key.py

@@ -1,66 +1,12 @@
 import binascii
 
-from cryptography.exceptions import InvalidSignature
 from cryptography.hazmat.primitives import hashes
-from cryptography.hazmat.primitives.asymmetric import padding, rsa, ec
 from pyasn1.codec.der.encoder import encode
 from pyasn1.type import univ
-from pyasn1_alt_modules import rfc5280, rfc3279, rfc5480, rfc8410
+from pyasn1_alt_modules import rfc5280, rfc5480
 
 from pkilint import validation, util, document
-from pkilint.document import PDUNode
-
-SUBJECT_PUBLIC_KEY_ALGORITHM_IDENTIFIER_MAPPINGS = {
-    rfc3279.rsaEncryption: rfc5480.RSAPublicKey(),
-    rfc5480.id_ecPublicKey: rfc5480.ECPoint(),
-    rfc5480.id_ecDH: rfc5480.ECPoint(),
-    rfc5480.id_ecMQV: rfc5480.ECPoint(),
-}
-
-SUBJECT_KEY_PARAMETER_ALGORITHM_IDENTIFIER_MAPPINGS = {
-    rfc3279.rsaEncryption: univ.Null(),
-    rfc5480.id_ecPublicKey: rfc5480.ECParameters(),
-    rfc5480.id_ecDH: rfc5480.ECParameters(),
-    rfc5480.id_ecMQV: rfc5480.ECParameters(),
-    **{
-        o: document.ValueDecoder.VALUE_NODE_ABSENT
-        for o in (
-            rfc8410.id_Ed448,
-            rfc8410.id_Ed25519,
-            rfc8410.id_X448,
-            rfc8410.id_X25519,
-        )
-    },
-}
-
-EC_CURVE_OID_TO_OBJECT_MAPPINGS = {
-    rfc5480.secp256r1: ec.SECP256R1(),
-    rfc5480.secp384r1: ec.SECP384R1(),
-    rfc5480.secp521r1: ec.SECP521R1(),
-}
-
-
-def convert_spki_to_object(spki_node: PDUNode):
-    key_type = spki_node.navigate("algorithm.algorithm").pdu
-
-    if key_type == rfc3279.rsaEncryption:
-        modulus = spki_node.navigate("subjectPublicKey.rSAPublicKey.modulus").pdu
-        exponent = spki_node.navigate("subjectPublicKey.rSAPublicKey.exponent").pdu
-
-        return rsa.RSAPublicNumbers(int(modulus), int(exponent)).public_key()
-    elif key_type in {rfc5480.id_ecPublicKey, rfc5480.id_ecDH, rfc5480.id_ecMQV}:
-        curve_oid = spki_node.navigate(
-            "algorithm.parameters.eCParameters.namedCurve"
-        ).pdu
-
-        curve = EC_CURVE_OID_TO_OBJECT_MAPPINGS.get(curve_oid)
-        if curve is not None:
-            return ec.EllipticCurvePublicKey.from_encoded_point(
-                curve, spki_node.navigate("subjectPublicKey").pdu.asOctets()
-            )
-
-    # TODO: DSA
-    return None
+from pkilint.pkix.key import verify_signature
 
 
 class SubjectPublicKeyDecoder(document.ValueDecoder):
@@ -207,40 +153,41 @@ def validate(self, node):
         raise validation.ValidationFindingEncountered(finding)
 
 
-def _verify_signature(public_key, message, signature, signature_algorithm):
-    try:
-        if isinstance(public_key, rsa.RSAPublicKey):
-            public_key.verify(
-                signature, message, padding.PKCS1v15(), signature_algorithm
-            )
-        else:
-            public_key.verify(signature, message, ec.ECDSA(signature_algorithm))
-
-        return True
-    except InvalidSignature:
-        return False
-
-
 class SubjectSignatureVerificationValidator(validation.Validator):
     VALIDATION_SIGNATURE_MISMATCH = validation.ValidationFinding(
         validation.ValidationFindingSeverity.ERROR, "pkix.signature_verification_failed"
     )
 
+    VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM = validation.ValidationFinding(
+        validation.ValidationFindingSeverity.NOTICE,
+        "pkix.unsupported_public_key_algorithm",
+    )
+
     def __init__(self, *, tbs_node_retriever, **kwargs):
-        super().__init__(validations=[self.VALIDATION_SIGNATURE_MISMATCH], **kwargs)
+        super().__init__(
+            validations=[
+                self.VALIDATION_SIGNATURE_MISMATCH,
+                self.VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM,
+            ],
+            **kwargs,
+        )
 
         self._tbs_node_retriever = tbs_node_retriever
 
     def validate(self, node):
         issuer_cert_doc = document.get_document_by_name(node, "issuer")
 
-        issuer_crypto_cert = issuer_cert_doc.cryptography_object
+        public_key = issuer_cert_doc.public_key_object
+        if public_key is None:
+            raise validation.ValidationFindingEncountered(
+                self.VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM
+            )
+
         subject_crypto_doc = node.document.cryptography_object
-        public_key = issuer_crypto_cert.public_key()
 
         tbs_octets = encode(self._tbs_node_retriever(node).pdu)
 
-        if not _verify_signature(
+        if not verify_signature(
             public_key,
             tbs_octets,
             node.pdu.asOctets(),