Skip to content

Commit a78e46b

Browse files
committed
fix aia encoding memory leak
1 parent 857d401 commit a78e46b

File tree

2 files changed

+79
-14
lines changed

2 files changed

+79
-14
lines changed

src/cryptography/hazmat/backends/openssl/encode_asn1.py

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -345,16 +345,22 @@ def _encode_authority_information_access(backend, authority_info_access):
345345
aia = backend._lib.sk_ACCESS_DESCRIPTION_new_null()
346346
backend.openssl_assert(aia != backend._ffi.NULL)
347347
aia = backend._ffi.gc(
348-
aia, backend._lib.sk_ACCESS_DESCRIPTION_free
348+
aia,
349+
lambda x: backend._lib.sk_ACCESS_DESCRIPTION_pop_free(
350+
x, backend._ffi.addressof(
351+
backend._lib._original_lib, "ACCESS_DESCRIPTION_free"
352+
)
353+
)
349354
)
350355
for access_description in authority_info_access:
351356
ad = backend._lib.ACCESS_DESCRIPTION_new()
352357
method = _txt2obj(
353358
backend, access_description.access_method.dotted_string
354359
)
355-
gn = _encode_general_name(backend, access_description.access_location)
360+
_encode_general_name_preallocated(
361+
backend, access_description.access_location, ad.location
362+
)
356363
ad.method = method
357-
ad.location = gn
358364
res = backend._lib.sk_ACCESS_DESCRIPTION_push(aia, ad)
359365
backend.openssl_assert(res >= 1)
360366

@@ -385,8 +391,12 @@ def _encode_subject_key_identifier(backend, ski):
385391

386392

387393
def _encode_general_name(backend, name):
394+
gn = backend._lib.GENERAL_NAME_new()
395+
return _encode_general_name_preallocated(backend, name, gn)
396+
397+
398+
def _encode_general_name_preallocated(backend, name, gn):
388399
if isinstance(name, x509.DNSName):
389-
gn = backend._lib.GENERAL_NAME_new()
390400
backend.openssl_assert(gn != backend._ffi.NULL)
391401
gn.type = backend._lib.GEN_DNS
392402

@@ -400,7 +410,6 @@ def _encode_general_name(backend, name):
400410
backend.openssl_assert(res == 1)
401411
gn.d.dNSName = ia5
402412
elif isinstance(name, x509.RegisteredID):
403-
gn = backend._lib.GENERAL_NAME_new()
404413
backend.openssl_assert(gn != backend._ffi.NULL)
405414
gn.type = backend._lib.GEN_RID
406415
obj = backend._lib.OBJ_txt2obj(
@@ -409,13 +418,11 @@ def _encode_general_name(backend, name):
409418
backend.openssl_assert(obj != backend._ffi.NULL)
410419
gn.d.registeredID = obj
411420
elif isinstance(name, x509.DirectoryName):
412-
gn = backend._lib.GENERAL_NAME_new()
413421
backend.openssl_assert(gn != backend._ffi.NULL)
414422
dir_name = _encode_name(backend, name.value)
415423
gn.type = backend._lib.GEN_DIRNAME
416424
gn.d.directoryName = dir_name
417425
elif isinstance(name, x509.IPAddress):
418-
gn = backend._lib.GENERAL_NAME_new()
419426
backend.openssl_assert(gn != backend._ffi.NULL)
420427
if isinstance(name.value, ipaddress.IPv4Network):
421428
packed = (
@@ -433,7 +440,6 @@ def _encode_general_name(backend, name):
433440
gn.type = backend._lib.GEN_IPADD
434441
gn.d.iPAddress = ipaddr
435442
elif isinstance(name, x509.OtherName):
436-
gn = backend._lib.GENERAL_NAME_new()
437443
backend.openssl_assert(gn != backend._ffi.NULL)
438444
other_name = backend._lib.OTHERNAME_new()
439445
backend.openssl_assert(other_name != backend._ffi.NULL)
@@ -456,7 +462,6 @@ def _encode_general_name(backend, name):
456462
gn.type = backend._lib.GEN_OTHERNAME
457463
gn.d.otherName = other_name
458464
elif isinstance(name, x509.RFC822Name):
459-
gn = backend._lib.GENERAL_NAME_new()
460465
backend.openssl_assert(gn != backend._ffi.NULL)
461466
# ia5strings are supposed to be ITU T.50 but to allow round-tripping
462467
# of broken certs that encode utf8 we'll encode utf8 here too.
@@ -465,7 +470,6 @@ def _encode_general_name(backend, name):
465470
gn.type = backend._lib.GEN_EMAIL
466471
gn.d.rfc822Name = asn1_str
467472
elif isinstance(name, x509.UniformResourceIdentifier):
468-
gn = backend._lib.GENERAL_NAME_new()
469473
backend.openssl_assert(gn != backend._ffi.NULL)
470474
# ia5strings are supposed to be ITU T.50 but to allow round-tripping
471475
# of broken certs that encode utf8 we'll encode utf8 here too.

tests/hazmat/backends/test_openssl_memleak.py

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,16 @@
2222
def main(argv):
2323
import gc
2424
import json
25+
import traceback
2526
2627
import cffi
2728
2829
from cryptography.hazmat.bindings._openssl import ffi, lib
2930
3031
heap = {}
3132
32-
BACKTRACE_ENABLED = False
33-
if BACKTRACE_ENABLED:
33+
C_BACKTRACE_ENABLED = False
34+
if C_BACKTRACE_ENABLED:
3435
backtrace_ffi = cffi.FFI()
3536
backtrace_ffi.cdef('''
3637
int backtrace(void **, int);
@@ -54,10 +55,10 @@ def symbolize_backtrace(trace):
5455
return stack
5556
else:
5657
def backtrace():
57-
return None
58+
return traceback.format_stack()
5859
5960
def symbolize_backtrace(trace):
60-
return None
61+
return trace
6162
6263
@ffi.callback("void *(size_t, const char *, int)")
6364
def malloc(size, path, line):
@@ -389,3 +390,63 @@ def func():
389390
x509.IssuingDistributionPoint
390391
)
391392
"""))
393+
394+
def test_create_certificate_with_extensions(self):
395+
assert_no_memory_leaks(textwrap.dedent("""
396+
def func():
397+
import datetime
398+
399+
from cryptography import x509
400+
from cryptography.hazmat.backends.openssl import backend
401+
from cryptography.hazmat.primitives import hashes
402+
from cryptography.hazmat.primitives.asymmetric import ec
403+
from cryptography.x509.oid import (
404+
AuthorityInformationAccessOID, ExtendedKeyUsageOID, NameOID
405+
)
406+
407+
private_key = ec.generate_private_key(ec.SECP256R1(), backend)
408+
409+
not_valid_before = datetime.datetime.now()
410+
not_valid_after = not_valid_before + datetime.timedelta(days=365)
411+
412+
aia = x509.AuthorityInformationAccess([
413+
x509.AccessDescription(
414+
AuthorityInformationAccessOID.OCSP,
415+
x509.UniformResourceIdentifier(u"http://ocsp.domain.com")
416+
),
417+
x509.AccessDescription(
418+
AuthorityInformationAccessOID.CA_ISSUERS,
419+
x509.UniformResourceIdentifier(u"http://domain.com/ca.crt")
420+
)
421+
])
422+
sans = [u'*.example.org', u'foobar.example.net']
423+
san = x509.SubjectAlternativeName(list(map(x509.DNSName, sans)))
424+
425+
ski = x509.SubjectKeyIdentifier.from_public_key(
426+
private_key.public_key()
427+
)
428+
eku = x509.ExtendedKeyUsage([
429+
ExtendedKeyUsageOID.CLIENT_AUTH,
430+
ExtendedKeyUsageOID.SERVER_AUTH,
431+
ExtendedKeyUsageOID.CODE_SIGNING,
432+
])
433+
434+
builder = x509.CertificateBuilder().serial_number(
435+
777
436+
).issuer_name(x509.Name([
437+
x509.NameAttribute(NameOID.COUNTRY_NAME, u'US'),
438+
])).subject_name(x509.Name([
439+
x509.NameAttribute(NameOID.COUNTRY_NAME, u'US'),
440+
])).public_key(
441+
private_key.public_key()
442+
).add_extension(
443+
aia, critical=False
444+
).not_valid_before(
445+
not_valid_before
446+
).not_valid_after(
447+
not_valid_after
448+
)
449+
450+
cert = builder.sign(private_key, hashes.SHA256(), backend)
451+
cert.extensions
452+
"""))

0 commit comments

Comments
 (0)