diff --git a/sslyze/plugins/certificate_info/_cert_chain_analyzer.py b/sslyze/plugins/certificate_info/_cert_chain_analyzer.py index 3a79f96f..7a95416a 100644 --- a/sslyze/plugins/certificate_info/_cert_chain_analyzer.py +++ b/sslyze/plugins/certificate_info/_cert_chain_analyzer.py @@ -11,7 +11,10 @@ from cryptography.x509.ocsp import load_der_ocsp_response, OCSPResponseStatus, OCSPResponse import nassl.ocsp_response -from sslyze.plugins.certificate_info._certificate_utils import extract_dns_subject_alternative_names, get_common_names +from sslyze.plugins.certificate_info._certificate_utils import ( + parse_subject_alternative_name_extension, + get_common_names, +) from sslyze.plugins.certificate_info._symantec import SymantecDistructTester from sslyze.plugins.certificate_info.trust_stores.trust_store import TrustStore, PathValidationResult @@ -280,9 +283,13 @@ def _certificate_matches_hostname(certificate: Certificate, server_hostname: str # Cryptography could not parse the certificate https://github.com/nabla-c0d3/sslyze/issues/495 return False + subj_alt_name_ext = parse_subject_alternative_name_extension(certificate) + subj_alt_name_as_list = [("DNS", name) for name in subj_alt_name_ext.dns_names] + subj_alt_name_as_list.extend([("IP Address", ip) for ip in subj_alt_name_ext.ip_addresses]) + certificate_names = { "subject": (tuple([("commonName", name) for name in get_common_names(cert_subject)]),), - "subjectAltName": tuple([("DNS", name) for name in extract_dns_subject_alternative_names(certificate)]), + "subjectAltName": tuple(subj_alt_name_as_list), } # CertificateError is raised on failure try: diff --git a/sslyze/plugins/certificate_info/_certificate_utils.py b/sslyze/plugins/certificate_info/_certificate_utils.py index 84a906a9..a29cb697 100644 --- a/sslyze/plugins/certificate_info/_certificate_utils.py +++ b/sslyze/plugins/certificate_info/_certificate_utils.py @@ -1,34 +1,57 @@ +from dataclasses import dataclass from hashlib import sha256 from typing import List, cast -from cryptography import x509 from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat -from cryptography.x509 import ExtensionOID, DNSName, ExtensionNotFound, NameOID, DuplicateExtension - - -def extract_dns_subject_alternative_names(certificate: x509.Certificate) -> List[str]: - """Retrieve all the DNS entries of the Subject Alternative Name extension.""" - subj_alt_names: List[str] = [] +from cryptography.x509 import ( + ExtensionOID, + DNSName, + ExtensionNotFound, + NameOID, + DuplicateExtension, + IPAddress, + Certificate, + SubjectAlternativeName, + Name, +) + + +@dataclass(frozen=True) +class SubjectAlternativeNameExtension: + dns_names: List[str] + ip_addresses: List[str] + + +def parse_subject_alternative_name_extension(certificate: Certificate) -> SubjectAlternativeNameExtension: try: san_ext = certificate.extensions.get_extension_for_oid(ExtensionOID.SUBJECT_ALTERNATIVE_NAME) - san_ext_value = cast(x509.SubjectAlternativeName, san_ext.value) - subj_alt_names = san_ext_value.get_values_for_type(DNSName) + san_ext_value = cast(SubjectAlternativeName, san_ext.value) except ExtensionNotFound: - pass + return SubjectAlternativeNameExtension(dns_names=[], ip_addresses=[]) except DuplicateExtension: # Fix for https://github.com/nabla-c0d3/sslyze/issues/420 # Not sure how browsers behave in this case but having a duplicate extension makes the certificate invalid # so we just return no SANs (likely to make hostname validation fail, which is fine) - pass + return SubjectAlternativeNameExtension(dns_names=[], ip_addresses=[]) + + dns_names = [] + ip_addresses = [] + for san_value in san_ext_value: + if isinstance(san_value, IPAddress): + ip_addresses.append(str(san_value.value)) + elif isinstance(san_value, DNSName): + dns_names.append(san_value.value) + else: + pass - return subj_alt_names + return SubjectAlternativeNameExtension(dns_names=dns_names, ip_addresses=ip_addresses) -def get_common_names(name_field: x509.Name) -> List[str]: +def get_common_names(name_field: Name) -> List[str]: return [cn.value for cn in name_field.get_attributes_for_oid(NameOID.COMMON_NAME)] # type: ignore -def get_public_key_sha256(certificate: x509.Certificate) -> bytes: +def get_public_key_sha256(certificate: Certificate) -> bytes: pub_bytes = certificate.public_key().public_bytes(encoding=Encoding.DER, format=PublicFormat.SubjectPublicKeyInfo) digest = sha256(pub_bytes).digest() return digest diff --git a/sslyze/plugins/certificate_info/_cli_connector.py b/sslyze/plugins/certificate_info/_cli_connector.py index 4f909aa0..95a3a610 100644 --- a/sslyze/plugins/certificate_info/_cli_connector.py +++ b/sslyze/plugins/certificate_info/_cli_connector.py @@ -10,7 +10,10 @@ from cryptography.x509.ocsp import OCSPResponseStatus from sslyze.plugins.certificate_info._cert_chain_analyzer import CertificateDeploymentAnalysisResult -from sslyze.plugins.certificate_info._certificate_utils import get_common_names, extract_dns_subject_alternative_names +from sslyze.plugins.certificate_info._certificate_utils import ( + get_common_names, + parse_subject_alternative_name_extension, +) from sslyze.plugins.plugin_base import ScanCommandCliConnector, OptParseCliOption @@ -295,15 +298,12 @@ def _get_basic_certificate_text(cls, certificate: Certificate) -> List[str]: # DSA Key? https://github.com/nabla-c0d3/sslyze/issues/314 pass - try: - # Print the SAN extension if there's one - text_output.append( - cls._format_field( - "DNS Subject Alternative Names:", str(extract_dns_subject_alternative_names(certificate)) - ) - ) - except KeyError: - pass + # Print the SAN extension if there's one + subj_alt_name_ext = parse_subject_alternative_name_extension(certificate) + if subj_alt_name_ext.dns_names: + text_output.append(cls._format_field("SubjAltName - DNS Names:", str(subj_alt_name_ext.dns_names))) + if subj_alt_name_ext.ip_addresses: + text_output.append(cls._format_field("SubjAltName - IP Addresses", str(subj_alt_name_ext.ip_addresses))) return text_output diff --git a/sslyze/plugins/certificate_info/json_output.py b/sslyze/plugins/certificate_info/json_output.py index 93768ac0..e1c6f687 100644 --- a/sslyze/plugins/certificate_info/json_output.py +++ b/sslyze/plugins/certificate_info/json_output.py @@ -20,7 +20,7 @@ from sslyze.json.scan_attempt_json import ScanCommandAttemptAsJson from sslyze.plugins.certificate_info._certificate_utils import ( get_public_key_sha256, - extract_dns_subject_alternative_names, + parse_subject_alternative_name_extension, ) @@ -107,7 +107,13 @@ def from_orm(cls, name: Name) -> "_X509NameAsJson": class _SubjAltNameAsJson(pydantic.BaseModel): - dns: List[str] + + # TODO(6.0.0): Remove the Config, alias and default value as the name "dns" is deprecated + class Config: + allow_population_by_field_name = True + + dns_names: List[str] = pydantic.Field(alias="dns") + ip_addresses: List[pydantic.IPvAnyAddress] = [] class _HashAlgorithmAsJson(_BaseModelWithOrmMode): @@ -164,6 +170,8 @@ def from_orm(cls, certificate: Certificate) -> "_CertificateAsJson": except ValueError: issuer_field = None + subj_alt_name_ext = parse_subject_alternative_name_extension(certificate) + return cls( as_pem=certificate.public_bytes(Encoding.PEM).decode("ascii"), hpkp_pin=b64encode(get_public_key_sha256(certificate)).decode("ascii"), @@ -172,7 +180,10 @@ def from_orm(cls, certificate: Certificate) -> "_CertificateAsJson": serial_number=certificate.serial_number, not_valid_before=certificate.not_valid_before, not_valid_after=certificate.not_valid_after, - subject_alternative_name=_SubjAltNameAsJson(dns=extract_dns_subject_alternative_names(certificate)), + subject_alternative_name=_SubjAltNameAsJson( + dns_names=subj_alt_name_ext.dns_names, + ip_addresses=subj_alt_name_ext.ip_addresses, + ), signature_hash_algorithm=signature_hash_algorithm, signature_algorithm_oid=certificate.signature_algorithm_oid, subject=subject_field, diff --git a/sslyze/plugins/http_headers_plugin.py b/sslyze/plugins/http_headers_plugin.py index 7dfb94f5..9dd0bc14 100755 --- a/sslyze/plugins/http_headers_plugin.py +++ b/sslyze/plugins/http_headers_plugin.py @@ -79,7 +79,7 @@ class HttpHeadersScanResult(ScanCommandResult): http_path_redirected_to: Optional[str] strict_transport_security_header: Optional[StrictTransportSecurityHeader] - expect_ct_header: None = None # Deprecated; to be removed in the next major version + expect_ct_header: None = None # TODO(6.0.0): Remove as this is a deprecated field class _StrictTransportSecurityHeaderAsJson(pydantic.BaseModel): @@ -97,7 +97,7 @@ class HttpHeadersScanResultAsJson(pydantic.BaseModel): http_path_redirected_to: Optional[str] strict_transport_security_header: Optional[_StrictTransportSecurityHeaderAsJson] - expect_ct_header: None = None # Deprecated; to be removed in the next major version + expect_ct_header: None = None # TODO(6.0.0): Remove as this is a deprecated field class Config: orm_mode = True diff --git a/tests/plugins_tests/certificate_info/test_certificate_utils.py b/tests/plugins_tests/certificate_info/test_certificate_utils.py index c8946d40..ec4600b7 100644 --- a/tests/plugins_tests/certificate_info/test_certificate_utils.py +++ b/tests/plugins_tests/certificate_info/test_certificate_utils.py @@ -6,7 +6,7 @@ from sslyze.plugins.certificate_info._cert_chain_analyzer import _certificate_matches_hostname from sslyze.plugins.certificate_info._certificate_utils import ( get_common_names, - extract_dns_subject_alternative_names, + parse_subject_alternative_name_extension, get_public_key_sha256, ) from sslyze.plugins.certificate_info._cli_connector import _get_name_as_short_text @@ -26,8 +26,10 @@ def test_certificate_matches_hostname_bad_hostname(self): def test_get_common_names(self): assert get_common_names(certificate.subject) == ["github.com"] - def test_get_dns_subject_alternative_names(self): - assert extract_dns_subject_alternative_names(certificate) == ["github.com", "www.github.com"] + def test_parse_subject_alternative_name_extension(self): + subj_alt_name_ext = parse_subject_alternative_name_extension(certificate) + assert subj_alt_name_ext.dns_names == ["github.com", "www.github.com"] + assert subj_alt_name_ext.ip_addresses == [] def test_get_name_as_short_text(self): assert _get_name_as_short_text(certificate.issuer) == "DigiCert SHA2 Extended Validation Server CA"