fix aia encoding memory leak (#4889)

* fix aia encoding memory leak

* don't return anything from the prealloc func

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 61cfd14d..a774daa7 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 f9ae1c46..935ea3df 100644
--- a/tests/hazmat/backends/test_openssl_memleak.py
+++ b/tests/hazmat/backends/test_openssl_memleak.py
@@ -389,3 +389,63 @@ class TestOpenSSLMemoryLeaks(object):
                 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
+        """))