diff options
| -rw-r--r-- | src/_cffi_src/openssl/x509.py | 3 | ||||
| -rw-r--r-- | src/cryptography/hazmat/backends/openssl/backend.py | 11 | ||||
| -rw-r--r-- | src/cryptography/hazmat/backends/openssl/x509.py | 8 | ||||
| -rw-r--r-- | tests/hazmat/backends/test_openssl_memleak.py | 59 |
4 files changed, 75 insertions, 6 deletions
diff --git a/src/_cffi_src/openssl/x509.py b/src/_cffi_src/openssl/x509.py index 59fdbf7e..3f2ac90d 100644 --- a/src/_cffi_src/openssl/x509.py +++ b/src/_cffi_src/openssl/x509.py @@ -76,6 +76,8 @@ static const int XN_FLAG_FN_ALIGN; 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_push(X509_EXTENSIONS *, X509_EXTENSION *); 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 bdf8f370..cfd7c89f 100644 --- a/src/cryptography/hazmat/backends/openssl/backend.py +++ b/src/cryptography/hazmat/backends/openssl/backend.py @@ -707,10 +707,15 @@ class Backend(object): 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 b870eeb7..a7a2c70d 100644 --- a/src/cryptography/hazmat/backends/openssl/x509.py +++ b/src/cryptography/hazmat/backends/openssl/x509.py @@ -429,6 +429,14 @@ class _CertificateSigningRequest(object): @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 5cb7cbc7..34ad11ba 100644 --- a/tests/hazmat/backends/test_openssl_memleak.py +++ b/tests/hazmat/backends/test_openssl_memleak.py @@ -23,14 +23,46 @@ def main(argv): 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 @@ def main(argv): 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 @@ def main(argv): "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 @@ class TestOpenSSLMemoryLeaks(object): @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 @@ class TestOpenSSLMemoryLeaks(object): 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(): |