diff --git a/dev-requirements.txt b/dev-requirements.txt index df8ce4d6..5f0d4ce4 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -7,6 +7,7 @@ sphinx-rtd-theme twine sphinx-autodoc-typehints black==19.10b0 +click==8.0.4 # TODO(AD): Fix for black; to remove after updating black pytest-cov faker diff --git a/sslyze/plugins/certificate_info/_cert_chain_analyzer.py b/sslyze/plugins/certificate_info/_cert_chain_analyzer.py index da0e2f33..e404a585 100644 --- a/sslyze/plugins/certificate_info/_cert_chain_analyzer.py +++ b/sslyze/plugins/certificate_info/_cert_chain_analyzer.py @@ -1,7 +1,8 @@ from dataclasses import dataclass +from pathlib import Path from ssl import CertificateError, match_hostname -from typing import Optional, List, cast +from typing import Optional, List, cast, Dict import cryptography from cryptography.hazmat.backends import default_backend @@ -321,9 +322,37 @@ def _certificate_matches_hostname(certificate: Certificate, server_hostname: str return False +# TODO(AD): There is probably a memory leak in nassl.X509 or nassl.X509_STORE_CTX +# https://github.com/nabla-c0d3/sslyze/issues/560 +# It might be due to bad reference counting in nassl_X509_STORE_CTX_set0_trusted_stack() +# More specifically the call to X509_chain_up_ref() - is there corresponding call to decrease ref count? +# As a workaround, we cache the (huge) list of trusted certificates, for each trust store +_cache_for_trusted_certificates_per_file: Dict[Path, List[X509]] = {} + + +def _convert_and_cache_pem_certs_to_x509s(trusted_certificates_path: Path) -> List[X509]: + certs_as_509s = _cache_for_trusted_certificates_per_file.get(trusted_certificates_path) + if certs_as_509s: + return certs_as_509s + + # Parse the PEM certificate in the file + all_certs_as_pem: List[str] = [] + with trusted_certificates_path.open() as file_content: + for pem_segment in file_content.read().split("-----BEGIN CERTIFICATE-----")[1::]: + pem_content = pem_segment.split("-----END CERTIFICATE-----")[0] + pem_cert = f"-----BEGIN CERTIFICATE-----{pem_content}-----END CERTIFICATE-----" + all_certs_as_pem.append(pem_cert) + + # Convert them to X509 objects and save that in the cache + all_certs_as_509s = [X509(cert_pem) for cert_pem in all_certs_as_pem] + _cache_for_trusted_certificates_per_file[trusted_certificates_path] = all_certs_as_509s + return all_certs_as_509s + + def _verify_certificate_chain(server_certificate_chain: List[str], trust_store: TrustStore) -> PathValidationResult: server_chain_as_x509s = [X509(pem_cert) for pem_cert in server_certificate_chain] - chain_verifier = CertificateChainVerifier.from_file(trust_store.path) + trust_store_as_x509s = _convert_and_cache_pem_certs_to_x509s(trust_store.path) + chain_verifier = CertificateChainVerifier(trust_store_as_x509s) verified_chain: Optional[List[Certificate]] try: diff --git a/tests/plugins_tests/certificate_info/test_cert_chain_analyzer.py b/tests/plugins_tests/certificate_info/test_cert_chain_analyzer.py new file mode 100644 index 00000000..6064336b --- /dev/null +++ b/tests/plugins_tests/certificate_info/test_cert_chain_analyzer.py @@ -0,0 +1,21 @@ +from sslyze.plugins.certificate_info.trust_stores.trust_store_repository import TrustStoresRepository +from sslyze.plugins.certificate_info._cert_chain_analyzer import ( + _cache_for_trusted_certificates_per_file, + _convert_and_cache_pem_certs_to_x509s, +) + + +class TestMemoryLeakWorkaroundWithX509Cache: + def test(self): + # Given a path to a file with a list of PEM certificates + trusted_certificates_path = TrustStoresRepository.get_default().get_main_store().path + + # And the file's content has not been cached yet + assert trusted_certificates_path not in _cache_for_trusted_certificates_per_file + + # When converting the content of the file to X509 objects + certs_as_x509s = _convert_and_cache_pem_certs_to_x509s(trusted_certificates_path) + + # It succeeds, and the x509 objects were cached + assert certs_as_x509s + assert trusted_certificates_path in _cache_for_trusted_certificates_per_file