Skip to content

Commit

Permalink
Gracefully handle unsupported signature algorithms
Browse files Browse the repository at this point in the history
  • Loading branch information
CBonnell committed Nov 22, 2024
1 parent 92379f4 commit e9166d4
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 32 deletions.
22 changes: 15 additions & 7 deletions pkilint/pkix/certificate/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import logging
from typing import Set, Optional

from cryptography import x509
from cryptography import x509, exceptions
from pyasn1.codec.der.encoder import encode
from pyasn1.type import univ
from pyasn1.type.base import Asn1Type
Expand Down Expand Up @@ -128,6 +128,15 @@ def public_key_object(self):
self.root.navigate("tbsCertificate.subjectPublicKeyInfo")
)

def is_signed_with_key(self, public_key):
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
)

@functools.cached_property
def is_self_signed(self):
if not self.is_self_issued:
Expand All @@ -139,13 +148,12 @@ def is_self_signed(self):
if public_key is None:
return False

tbs_octets = self.cryptography_object.tbs_certificate_bytes
signature_hash_alg = self.cryptography_object.signature_hash_algorithm
signature_octets = self.cryptography_object.signature
try:
self.is_signed_with_key(public_key)

return key.verify_signature(
public_key, tbs_octets, signature_octets, signature_hash_alg
)
# gracefully handle unsupported signature algorithms
except exceptions.UnsupportedAlgorithm:
return False

def get_extension_by_oid(self, oid):
tbs_cert = self.root.children["tbsCertificate"]
Expand Down
40 changes: 28 additions & 12 deletions pkilint/pkix/certificate/certificate_extension.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import NamedTuple, Set

import unicodedata
from cryptography import exceptions
from pyasn1.type.univ import ObjectIdentifier
from pyasn1_alt_modules import rfc5280, rfc4262, rfc6962, rfc3739

Expand Down Expand Up @@ -628,16 +629,16 @@ class AuthorityKeyIdentifierPresenceValidator(validation.Validator):
"pkix.authority_key_identifier_extension_absent",
)

VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM = validation.ValidationFinding(
VALIDATION_UNSUPPORTED_ALGORITHM = validation.ValidationFinding(
validation.ValidationFindingSeverity.NOTICE,
"pkix.aki_absent_self_issued_and_unsupported_public_key_algorithm",
"pkix.aki_absent_self_issued_and_unsupported_algorithm",
)

def __init__(self):
super().__init__(
validations=[
self.VALIDATION_AKI_EXTENSION_NOT_PRESENT,
self.VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM,
self.VALIDATION_UNSUPPORTED_ALGORITHM,
],
pdu_class=rfc5280.Certificate,
)
Expand All @@ -650,25 +651,40 @@ def validate(self, node):
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:
if cert_doc.is_self_issued:
public_key = cert_doc.public_key_object

if public_key is None:
key_oid = str(
node.navigate(
"tbsCertificate.subjectPublicKeyInfo.algorithm.algorithm"
":certificate.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}",
self.VALIDATION_UNSUPPORTED_ALGORITHM,
f"Self-issued certificate uses unsupported public key algorithm: {key_oid}",
)
# if the cert is not self-issued and/or certifies a supported key type, then report the PKIX error
else:

try:
is_self_signed = cert_doc.is_signed_with_key(public_key)

if is_self_signed:
return
except exceptions.UnsupportedAlgorithm:
sig_oid = str(
node.navigate(":certificate.signatureAlgorithm.algorithm").pdu
)

raise validation.ValidationFindingEncountered(
self.VALIDATION_AKI_EXTENSION_NOT_PRESENT
self.VALIDATION_UNSUPPORTED_ALGORITHM,
f"Self-issued certificate uses unsupported signature algorithm: {sig_oid}",
)

raise validation.ValidationFindingEncountered(
self.VALIDATION_AKI_EXTENSION_NOT_PRESENT
)


class AuthorityInformationAccessCriticalityValidator(ExtensionCriticalityValidator):
VALIDATION_AIA_EXTENSION_CRITICAL = validation.ValidationFinding(
Expand Down
28 changes: 17 additions & 11 deletions pkilint/pkix/certificate/certificate_key.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import binascii

from cryptography import exceptions
from cryptography.hazmat.primitives import hashes
from pyasn1.codec.der.encoder import encode
from pyasn1.error import PyAsn1Error
Expand Down Expand Up @@ -169,16 +170,16 @@ class SubjectSignatureVerificationValidator(validation.Validator):
validation.ValidationFindingSeverity.ERROR, "pkix.signature_verification_failed"
)

VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM = validation.ValidationFinding(
VALIDATION_UNSUPPORTED_ALGORITHM = validation.ValidationFinding(
validation.ValidationFindingSeverity.NOTICE,
"pkix.unsupported_public_key_algorithm",
"pkix.unsupported_algorithm",
)

def __init__(self, *, tbs_node_retriever, **kwargs):
super().__init__(
validations=[
self.VALIDATION_SIGNATURE_MISMATCH,
self.VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM,
self.VALIDATION_UNSUPPORTED_ALGORITHM,
],
**kwargs,
)
Expand All @@ -191,21 +192,26 @@ def validate(self, node):
public_key = issuer_cert_doc.public_key_object
if public_key is None:
raise validation.ValidationFindingEncountered(
self.VALIDATION_UNSUPPORTED_PUBLIC_KEY_ALGORITHM
self.VALIDATION_UNSUPPORTED_ALGORITHM
)

subject_crypto_doc = node.document.cryptography_object

tbs_octets = encode(self._tbs_node_retriever(node).pdu)

if not verify_signature(
public_key,
tbs_octets,
node.pdu.asOctets(),
subject_crypto_doc.signature_hash_algorithm,
):
try:
if not verify_signature(
public_key,
tbs_octets,
node.pdu.asOctets(),
subject_crypto_doc.signature_hash_algorithm,
):
raise validation.ValidationFindingEncountered(
self.VALIDATION_SIGNATURE_MISMATCH
)
except exceptions.UnsupportedAlgorithm:
raise validation.ValidationFindingEncountered(
self.VALIDATION_SIGNATURE_MISMATCH
self.VALIDATION_UNSUPPORTED_ALGORITHM
)


Expand Down
6 changes: 6 additions & 0 deletions pkilint/pkix/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ def verify_signature(public_key, message, signature, signature_hash_algorithm=No
public_key.verify(signature, message)
elif isinstance(public_key, ed448.Ed448PublicKey):
public_key.verify(signature, message)
else:
type_name = type(public_key).__name__

raise exceptions.UnsupportedAlgorithm(
f'Unsupported public key type "{type_name}"'
)
except exceptions.InvalidSignature:
return False

Expand Down
2 changes: 1 addition & 1 deletion tests/integration_certificate/pkix/mldsa44_root.crttest
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,6 @@ YNuixctKaeFUW6J1qIEi7SQ0n3wR0KM44FcCQ3LaZEYG7U/zVgI552D9+kJJENEB
-----END CERTIFICATE-----

node_path,validator,severity,code,message
certificate,AuthorityKeyIdentifierPresenceValidator,NOTICE,pkix.aki_absent_self_issued_and_unsupported_public_key_algorithm,Self-issued certificate certifies a public key of unsupported algorithm: 2.16.840.1.101.3.4.3.17
certificate,AuthorityKeyIdentifierPresenceValidator,NOTICE,pkix.aki_absent_self_issued_and_unsupported_algorithm,Self-issued certificate uses unsupported public key algorithm: 2.16.840.1.101.3.4.3.17
certificate.tbsCertificate.extensions.0.extnValue.subjectKeyIdentifier,SubjectKeyIdentifierValidator,INFO,pkix.subject_key_identifier_method_1_identified,
certificate.tbsCertificate.extensions.2.extnValue.keyUsage,SpkiKeyUsageConsistencyValidator,NOTICE,pkix.public_key_algorithm_unsupported,Unsupported public key algorithm: 2.16.840.1.101.3.4.3.17
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ AiAMA1aqNACzlZdAvhHk1o2fjCYYodSWbLbAd5cNAzPftw==
-----END CERTIFICATE-----

node_path,validator,severity,code,message
certificate,AuthorityKeyIdentifierPresenceValidator,NOTICE,pkix.aki_absent_self_issued_and_unsupported_public_key_algorithm,Self-issued certificate certifies a public key of unsupported algorithm: 1.2.840.113549.1.1.1.999
certificate.tbsCertificate.extensions.1.extnValue.subjectKeyIdentifier,SubjectKeyIdentifierValidator,INFO,pkix.subject_key_identifier_method_1_identified,
certificate,AuthorityKeyIdentifierPresenceValidator,NOTICE,pkix.aki_absent_self_issued_and_unsupported_algorithm,Self-issued certificate uses unsupported public key algorithm: 1.2.840.113549.1.1.1.999
certificate.tbsCertificate.extensions,KeyUsagePresenceValidator,ERROR,pkix.ca_certificate_no_ku_extension,
19 changes: 19 additions & 0 deletions tests/integration_certificate/pkix/v1_root_signed_with_md2.crttest
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-----BEGIN CERTIFICATE-----
MIICPDCCAaUCEHC65B0Q2Sk0tjjKewPMur8wDQYJKoZIhvcNAQECBQAwXzELMAkG
A1UEBhMCVVMxFzAVBgNVBAoTDlZlcmlTaWduLCBJbmMuMTcwNQYDVQQLEy5DbGFz
cyAzIFB1YmxpYyBQcmltYXJ5IENlcnRpZmljYXRpb24gQXV0aG9yaXR5MB4XDTk2
MDEyOTAwMDAwMFoXDTI4MDgwMTIzNTk1OVowXzELMAkGA1UEBhMCVVMxFzAVBgNV
BAoTDlZlcmlTaWduLCBJbmMuMTcwNQYDVQQLEy5DbGFzcyAzIFB1YmxpYyBQcmlt
YXJ5IENlcnRpZmljYXRpb24gQXV0aG9yaXR5MIGfMA0GCSqGSIb3DQEBAQUAA4GN
ADCBiQKBgQDJXFme8huKARS0EN8EQNvjV69qRUCPhAwL0TPZ2RHP7gJYHyX3KqhE
BarsAx94f56TuZoAqiN91qyFomNFx3InzPRMxnVx0jnvT0Lwdd8KkMaOIG+YD/is
I19wKTakyYbnsZogy1Olhec9vn2a/iRFM9x2Fe0PonFkTGUugWhFpwIDAQABMA0G
CSqGSIb3DQEBAgUAA4GBALtMEivPLCYATxQT3ab7/AoRhIzzKBxnki98tsX63/Do
lbwdj2wsqFHMc9ikwFPwTtYmwHYBV4GSXiHx0bH/59AhWM1pF+NEHJwZRDmJXNyc
AA9WjQKZ7aKQRUzkuxCkPfAyAw7xzvjoyVGM5mKf5p/AfbdynMk2OmufTqj/ZA1k
-----END CERTIFICATE-----

node_path,validator,severity,code,message
certificate,SubjectKeyIdentifierPresenceValidator,WARNING,pkix.certificate_skid_end_entity_missing,
certificate.tbsCertificate.version,CorrectVersionValidator,ERROR,pkix.certificate_version_is_not_v3,"Expected=""2"", actual=""v1"""
certificate,AuthorityKeyIdentifierPresenceValidator,NOTICE,pkix.aki_absent_self_issued_and_unsupported_algorithm,Self-issued certificate uses unsupported signature algorithm: 1.2.840.113549.1.1.2

0 comments on commit e9166d4

Please sign in to comment.