Fixed two memory leaks in x509 csr extensions (#4434)
* Fixed a memory leak in x.509 OCSP no check
* Fix the _actual_ leak
* Speed up symbolizations
* Disable backtrace by default, because it doesn't work on Windows
* line length
diff --git a/src/_cffi_src/openssl/x509.py b/src/_cffi_src/openssl/x509.py
index 59fdbf7..3f2ac90 100644
--- a/src/_cffi_src/openssl/x509.py
+++ b/src/_cffi_src/openssl/x509.py
@@ -76,6 +76,8 @@
static const int XN_FLAG_RFC2253;
static const int XN_FLAG_ONELINE;
static const int XN_FLAG_MULTILINE;
+
+typedef void (*sk_X509_EXTENSION_freefunc)(X509_EXTENSION *);
"""
FUNCTIONS = """
@@ -282,6 +284,7 @@
int sk_X509_EXTENSION_insert(X509_EXTENSIONS *, X509_EXTENSION *, int);
X509_EXTENSION *sk_X509_EXTENSION_delete(X509_EXTENSIONS *, int);
void sk_X509_EXTENSION_free(X509_EXTENSIONS *);
+void sk_X509_EXTENSION_pop_free(X509_EXTENSIONS *, sk_X509_EXTENSION_freefunc);
int sk_X509_REVOKED_num(Cryptography_STACK_OF_X509_REVOKED *);
X509_REVOKED *sk_X509_REVOKED_value(Cryptography_STACK_OF_X509_REVOKED *, int);
diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index bdf8f37..cfd7c89 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -707,10 +707,15 @@
sk_extension = self._lib.sk_X509_EXTENSION_new_null()
self.openssl_assert(sk_extension != self._ffi.NULL)
sk_extension = self._ffi.gc(
- sk_extension, self._lib.sk_X509_EXTENSION_free
+ sk_extension,
+ lambda x: self._lib.sk_X509_EXTENSION_pop_free(
+ x, self._ffi.addressof(
+ self._lib._original_lib, "X509_EXTENSION_free"
+ )
+ )
)
- # gc is not necessary for CSRs, as sk_X509_EXTENSION_free
- # will release all the X509_EXTENSIONs.
+ # Don't GC individual extensions because the memory is owned by
+ # sk_extensions and will be freed along with it.
self._create_x509_extensions(
extensions=builder._extensions,
handlers=_EXTENSION_ENCODE_HANDLERS,
diff --git a/src/cryptography/hazmat/backends/openssl/x509.py b/src/cryptography/hazmat/backends/openssl/x509.py
index b870eeb..a7a2c70 100644
--- a/src/cryptography/hazmat/backends/openssl/x509.py
+++ b/src/cryptography/hazmat/backends/openssl/x509.py
@@ -429,6 +429,14 @@
@utils.cached_property
def extensions(self):
x509_exts = self._backend._lib.X509_REQ_get_extensions(self._x509_req)
+ x509_exts = self._backend._ffi.gc(
+ x509_exts,
+ lambda x: self._backend._lib.sk_X509_EXTENSION_pop_free(
+ x, self._backend._ffi.addressof(
+ self._backend._lib._original_lib, "X509_EXTENSION_free"
+ )
+ )
+ )
return _CSR_EXTENSION_PARSER.parse(self._backend, x509_exts)
def public_bytes(self, encoding):
diff --git a/tests/hazmat/backends/test_openssl_memleak.py b/tests/hazmat/backends/test_openssl_memleak.py
index 5cb7cbc..34ad11b 100644
--- a/tests/hazmat/backends/test_openssl_memleak.py
+++ b/tests/hazmat/backends/test_openssl_memleak.py
@@ -23,14 +23,46 @@
import gc
import json
+ import cffi
+
from cryptography.hazmat.bindings._openssl import ffi, lib
heap = {}
+ BACKTRACE_ENABLED = False
+ if BACKTRACE_ENABLED:
+ backtrace_ffi = cffi.FFI()
+ backtrace_ffi.cdef('''
+ int backtrace(void **, int);
+ char **backtrace_symbols(void *const *, int);
+ ''')
+ backtrace_lib = backtrace_ffi.dlopen(None)
+
+ def backtrace():
+ buf = backtrace_ffi.new("void*[]", 24)
+ length = backtrace_lib.backtrace(buf, len(buf))
+ return (buf, length)
+
+ def symbolize_backtrace(trace):
+ (buf, length) = trace
+ symbols = backtrace_lib.backtrace_symbols(buf, length)
+ stack = [
+ backtrace_ffi.string(symbols[i]).decode()
+ for i in range(length)
+ ]
+ lib.Cryptography_free_wrapper(symbols, backtrace_ffi.NULL, 0)
+ return stack
+ else:
+ def backtrace():
+ return None
+
+ def symbolize_backtrace(trace):
+ return None
+
@ffi.callback("void *(size_t, const char *, int)")
def malloc(size, path, line):
ptr = lib.Cryptography_malloc_wrapper(size, path, line)
- heap[ptr] = (size, path, line)
+ heap[ptr] = (size, path, line, backtrace())
return ptr
@ffi.callback("void *(void *, size_t, const char *, int)")
@@ -38,7 +70,7 @@
if ptr != ffi.NULL:
del heap[ptr]
new_ptr = lib.Cryptography_realloc_wrapper(ptr, size, path, line)
- heap[new_ptr] = (size, path, line)
+ heap[new_ptr] = (size, path, line, backtrace())
return new_ptr
@ffi.callback("void(void *, const char *, int)")
@@ -81,6 +113,7 @@
"size": heap[ptr][0],
"path": ffi.string(heap[ptr][1]).decode(),
"line": heap[ptr][2],
+ "backtrace": symbolize_backtrace(heap[ptr][3]),
})
for ptr in remaining
)))
@@ -177,7 +210,7 @@
@pytest.mark.parametrize("path", [
"x509/PKITS_data/certs/ValidcRLIssuerTest28EE.crt",
])
- def test_x509_extensions(self, path):
+ def test_x509_certificate_extensions(self, path):
assert_no_memory_leaks(textwrap.dedent("""
def func(path):
from cryptography import x509
@@ -193,6 +226,26 @@
cert.extensions
"""), [path])
+ def test_x509_csr_extensions(self):
+ assert_no_memory_leaks(textwrap.dedent("""
+ def func():
+ from cryptography import x509
+ from cryptography.hazmat.backends.openssl import backend
+ from cryptography.hazmat.primitives import hashes
+ from cryptography.hazmat.primitives.asymmetric import rsa
+
+ private_key = rsa.generate_private_key(
+ key_size=2048, public_exponent=65537, backend=backend
+ )
+ cert = x509.CertificateSigningRequestBuilder().subject_name(
+ x509.Name([])
+ ).add_extension(
+ x509.OCSPNoCheck(), critical=False
+ ).sign(private_key, hashes.SHA256(), backend)
+
+ cert.extensions
+ """))
+
def test_ec_private_numbers_private_key(self):
assert_no_memory_leaks(textwrap.dedent("""
def func():