From 9a22851fab924fd58482fdad3f8dd23dc3987f91 Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Sat, 18 May 2019 16:37:54 -0400 Subject: [PATCH] fix aia encoding memory leak (#4889) * fix aia encoding memory leak * don't return anything from the prealloc func --- .../hazmat/backends/openssl/encode_asn1.py | 27 +++++---- tests/hazmat/backends/test_openssl_memleak.py | 60 +++++++++++++++++++ 2 files changed, 75 insertions(+), 12 deletions(-) diff --git a/src/cryptography/hazmat/backends/openssl/encode_asn1.py b/src/cryptography/hazmat/backends/openssl/encode_asn1.py index 61cfd14de0..a774daa788 100644 --- a/src/cryptography/hazmat/backends/openssl/encode_asn1.py +++ b/src/cryptography/hazmat/backends/openssl/encode_asn1.py @@ -345,16 +345,22 @@ def _encode_authority_information_access(backend, authority_info_access): aia = backend._lib.sk_ACCESS_DESCRIPTION_new_null() backend.openssl_assert(aia != backend._ffi.NULL) aia = backend._ffi.gc( - aia, backend._lib.sk_ACCESS_DESCRIPTION_free + aia, + lambda x: backend._lib.sk_ACCESS_DESCRIPTION_pop_free( + x, backend._ffi.addressof( + backend._lib._original_lib, "ACCESS_DESCRIPTION_free" + ) + ) ) for access_description in authority_info_access: ad = backend._lib.ACCESS_DESCRIPTION_new() method = _txt2obj( backend, access_description.access_method.dotted_string ) - gn = _encode_general_name(backend, access_description.access_location) + _encode_general_name_preallocated( + backend, access_description.access_location, ad.location + ) ad.method = method - ad.location = gn res = backend._lib.sk_ACCESS_DESCRIPTION_push(aia, ad) backend.openssl_assert(res >= 1) @@ -385,8 +391,13 @@ def _encode_subject_key_identifier(backend, ski): def _encode_general_name(backend, name): + gn = backend._lib.GENERAL_NAME_new() + _encode_general_name_preallocated(backend, name, gn) + return gn + + +def _encode_general_name_preallocated(backend, name, gn): if isinstance(name, x509.DNSName): - gn = backend._lib.GENERAL_NAME_new() backend.openssl_assert(gn != backend._ffi.NULL) gn.type = backend._lib.GEN_DNS @@ -400,7 +411,6 @@ def _encode_general_name(backend, name): backend.openssl_assert(res == 1) gn.d.dNSName = ia5 elif isinstance(name, x509.RegisteredID): - gn = backend._lib.GENERAL_NAME_new() backend.openssl_assert(gn != backend._ffi.NULL) gn.type = backend._lib.GEN_RID obj = backend._lib.OBJ_txt2obj( @@ -409,13 +419,11 @@ def _encode_general_name(backend, name): backend.openssl_assert(obj != backend._ffi.NULL) gn.d.registeredID = obj elif isinstance(name, x509.DirectoryName): - gn = backend._lib.GENERAL_NAME_new() backend.openssl_assert(gn != backend._ffi.NULL) dir_name = _encode_name(backend, name.value) gn.type = backend._lib.GEN_DIRNAME gn.d.directoryName = dir_name elif isinstance(name, x509.IPAddress): - gn = backend._lib.GENERAL_NAME_new() backend.openssl_assert(gn != backend._ffi.NULL) if isinstance(name.value, ipaddress.IPv4Network): packed = ( @@ -433,7 +441,6 @@ def _encode_general_name(backend, name): gn.type = backend._lib.GEN_IPADD gn.d.iPAddress = ipaddr elif isinstance(name, x509.OtherName): - gn = backend._lib.GENERAL_NAME_new() backend.openssl_assert(gn != backend._ffi.NULL) other_name = backend._lib.OTHERNAME_new() backend.openssl_assert(other_name != backend._ffi.NULL) @@ -456,7 +463,6 @@ def _encode_general_name(backend, name): gn.type = backend._lib.GEN_OTHERNAME gn.d.otherName = other_name elif isinstance(name, x509.RFC822Name): - gn = backend._lib.GENERAL_NAME_new() backend.openssl_assert(gn != backend._ffi.NULL) # ia5strings are supposed to be ITU T.50 but to allow round-tripping # of broken certs that encode utf8 we'll encode utf8 here too. @@ -465,7 +471,6 @@ def _encode_general_name(backend, name): gn.type = backend._lib.GEN_EMAIL gn.d.rfc822Name = asn1_str elif isinstance(name, x509.UniformResourceIdentifier): - gn = backend._lib.GENERAL_NAME_new() backend.openssl_assert(gn != backend._ffi.NULL) # ia5strings are supposed to be ITU T.50 but to allow round-tripping # of broken certs that encode utf8 we'll encode utf8 here too. @@ -478,8 +483,6 @@ def _encode_general_name(backend, name): "{} is an unknown GeneralName type".format(name) ) - return gn - def _encode_extended_key_usage(backend, extended_key_usage): eku = backend._lib.sk_ASN1_OBJECT_new_null() diff --git a/tests/hazmat/backends/test_openssl_memleak.py b/tests/hazmat/backends/test_openssl_memleak.py index f9ae1c46b9..935ea3dfe3 100644 --- a/tests/hazmat/backends/test_openssl_memleak.py +++ b/tests/hazmat/backends/test_openssl_memleak.py @@ -389,3 +389,63 @@ def func(): x509.IssuingDistributionPoint ) """)) + + def test_create_certificate_with_extensions(self): + assert_no_memory_leaks(textwrap.dedent(""" + def func(): + import datetime + + from cryptography import x509 + from cryptography.hazmat.backends.openssl import backend + from cryptography.hazmat.primitives import hashes + from cryptography.hazmat.primitives.asymmetric import ec + from cryptography.x509.oid import ( + AuthorityInformationAccessOID, ExtendedKeyUsageOID, NameOID + ) + + private_key = ec.generate_private_key(ec.SECP256R1(), backend) + + not_valid_before = datetime.datetime.now() + not_valid_after = not_valid_before + datetime.timedelta(days=365) + + aia = x509.AuthorityInformationAccess([ + x509.AccessDescription( + AuthorityInformationAccessOID.OCSP, + x509.UniformResourceIdentifier(u"http://ocsp.domain.com") + ), + x509.AccessDescription( + AuthorityInformationAccessOID.CA_ISSUERS, + x509.UniformResourceIdentifier(u"http://domain.com/ca.crt") + ) + ]) + sans = [u'*.example.org', u'foobar.example.net'] + san = x509.SubjectAlternativeName(list(map(x509.DNSName, sans))) + + ski = x509.SubjectKeyIdentifier.from_public_key( + private_key.public_key() + ) + eku = x509.ExtendedKeyUsage([ + ExtendedKeyUsageOID.CLIENT_AUTH, + ExtendedKeyUsageOID.SERVER_AUTH, + ExtendedKeyUsageOID.CODE_SIGNING, + ]) + + builder = x509.CertificateBuilder().serial_number( + 777 + ).issuer_name(x509.Name([ + x509.NameAttribute(NameOID.COUNTRY_NAME, u'US'), + ])).subject_name(x509.Name([ + x509.NameAttribute(NameOID.COUNTRY_NAME, u'US'), + ])).public_key( + private_key.public_key() + ).add_extension( + aia, critical=False + ).not_valid_before( + not_valid_before + ).not_valid_after( + not_valid_after + ) + + cert = builder.sign(private_key, hashes.SHA256(), backend) + cert.extensions + """))