Merge pull request #2558 from reaperhulk/managing-memory-is-fun
fix a potential memory issue when retaining revoked certs from a CRL
diff --git a/src/cryptography/hazmat/backends/openssl/x509.py b/src/cryptography/hazmat/backends/openssl/x509.py
index 7e89ac6..12f5d46 100644
--- a/src/cryptography/hazmat/backends/openssl/x509.py
+++ b/src/cryptography/hazmat/backends/openssl/x509.py
@@ -747,8 +747,16 @@
@utils.register_interface(x509.RevokedCertificate)
class _RevokedCertificate(object):
- def __init__(self, backend, x509_revoked):
+ def __init__(self, backend, crl, x509_revoked):
self._backend = backend
+ # The X509_REVOKED_value is a X509_REVOKED * that has
+ # no reference counting. This means when X509_CRL_free is
+ # called then the CRL and all X509_REVOKED * are freed. Since
+ # you can retain a reference to a single revoked certificate
+ # and let the CRL fall out of scope we need to retain a
+ # private reference to the CRL inside the RevokedCertificate
+ # object to prevent the gc from being called inappropriately.
+ self._crl = crl
self._x509_revoked = x509_revoked
@property
@@ -861,7 +869,10 @@
for i in range(num):
r = self._backend._lib.sk_X509_REVOKED_value(revoked, i)
self._backend.openssl_assert(r != self._backend._ffi.NULL)
- revoked_list.append(_RevokedCertificate(self._backend, r))
+ revoked_certificate = _RevokedCertificate(
+ self._backend, self, r
+ )
+ revoked_list.append(revoked_certificate)
return revoked_list
diff --git a/tests/test_x509.py b/tests/test_x509.py
index ae2746e..ab4d666 100644
--- a/tests/test_x509.py
+++ b/tests/test_x509.py
@@ -173,6 +173,20 @@
# Check that len() works for CRLs.
assert len(crl) == 12
+ def test_revoked_cert_retrieval_retain_only_revoked(self, backend):
+ """
+ This test attempts to trigger the crash condition described in
+ https://github.com/pyca/cryptography/issues/2557
+ PyPy does gc at its own pace, so it will only be reliable on CPython.
+ """
+ revoked = _load_cert(
+ os.path.join("x509", "custom", "crl_all_reasons.pem"),
+ x509.load_pem_x509_crl,
+ backend
+ )[11]
+ assert revoked.revocation_date == datetime.datetime(2015, 1, 1, 0, 0)
+ assert revoked.serial_number == 11
+
def test_extensions(self, backend):
crl = _load_cert(
os.path.join("x509", "custom", "crl_ian_aia_aki.pem"),