Skip to content

Commit

Permalink
[#560] Implement workaround for memory leak in CertificateChainVerifier
Browse files Browse the repository at this point in the history
  • Loading branch information
nabla-c0d3 committed Apr 24, 2022
1 parent 8bf376c commit 1f33eca
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 2 deletions.
33 changes: 31 additions & 2 deletions 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
Expand Down Expand Up @@ -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:
Expand Down
21 changes: 21 additions & 0 deletions 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

0 comments on commit 1f33eca

Please sign in to comment.