From d1f5f472cba05ef003225cf21e73866bd4440466 Mon Sep 17 00:00:00 2001 From: "Carlos D. Garza" Date: Thu, 10 Jul 2014 08:59:21 -0500 Subject: [PATCH 1/5] Demonstration that _subjectAltNameString on the X509Extensions method has a memory leak. for issue https://github.com/pyca/pyopenssl/issues/139 --- leakcheck/crypto.py | 62 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/leakcheck/crypto.py b/leakcheck/crypto.py index ca79b7ccc..254df24c9 100644 --- a/leakcheck/crypto.py +++ b/leakcheck/crypto.py @@ -5,7 +5,7 @@ from OpenSSL.crypto import ( FILETYPE_PEM, TYPE_DSA, Error, PKey, X509, load_privatekey, CRL, Revoked, - get_elliptic_curves, _X509_REVOKED_dup) + get_elliptic_curves, _X509_REVOKED_dup, load_certificate) from OpenSSL._util import lib as _lib @@ -15,6 +15,66 @@ class BaseChecker(object): def __init__(self, iterations): self.iterations = iterations +class Checker_AltSubjectNameExt(BaseChecker): + """ + Leak check for X509Extention. member function _subjectAltNameString() + """ + ALTSUBJ_X509 = \ +"""-----BEGIN CERTIFICATE----- +MIIISzCCBzOgAwIBAgIQCiuGDMoB9F/X7mNgGxw+gzANBgkqhkiG9w0BAQsFADB1 +MQswCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMRkwFwYDVQQLExB3 +d3cuZGlnaWNlcnQuY29tMTQwMgYDVQQDEytEaWdpQ2VydCBTSEEyIEV4dGVuZGVk +IFZhbGlkYXRpb24gU2VydmVyIENBMB4XDTE0MDMyMDAwMDAwMFoXDTE2MDYxMjEy +MDAwMFowggELMR0wGwYDVQQPDBRQcml2YXRlIE9yZ2FuaXphdGlvbjETMBEGCysG +AQQBgjc8AgEDEwJVUzEVMBMGCysGAQQBgjc8AgECEwRVdGFoMRUwEwYDVQQFEww1 +Mjk5NTM3LTAxNDIxEjAQBgNVBAkTCVN1aXRlIDUwMDEkMCIGA1UECRMbMjYwMCBX +ZXN0IEV4ZWN1dGl2ZSBQYXJrd2F5MQ4wDAYDVQQREwU4NDA0MzELMAkGA1UEBhMC +VVMxDTALBgNVBAgTBFV0YWgxDTALBgNVBAcTBExlaGkxFzAVBgNVBAoTDkRpZ2lD +ZXJ0LCBJbmMuMRkwFwYDVQQDExB3d3cuZGlnaWNlcnQuY29tMIICIjANBgkqhkiG +9w0BAQEFAAOCAg8AMIICCgKCAgEAqImzO5GUV4dyCVtfyyxCKp7twv0geyxjf90H +v/tJXO0conB5dcI0zOsS8ECIOrnqKaIRj1PhAuGHBPZYuYa2f4VeClhHw73nayEH +ndvvV4sWzjjx4+LkWhC4ObsKrcrFEIU6oW9nyRjDW7JMpgG2w1C+fsh5yjxTXgJ4 +rpZfViGzpDw//knFF3Olbqlgqr0WBFb6VNLLJcDpn4nJ7hCHAfLHky3DL57QnEIk +nQkk9oDE6DSZWi4mw3MoUiasCTSOxXDh9fuTuDQtRPRQH4YKm2RFJgXURcpyA90e +gBqcUwZ7yDYxA9pfVcQNKcBSnCOVjalVlcQRAlujG+55sm5Kak1KRD45nosN7DiT +XlyzT1OPTip4sVJUS/tqlDVhAwZ56AacjoFbazbfwP5DztUWGfaClOiAAOGEFB0o +c4vpurZV56YXjK5wFb4E78gIJ9nfOn5njAYNUZQFlS8n5MHUpF7KlhOJ0gWLQ2j8 +MYeptvLDR+Pf2RkTT7kFqYqYA8rFkinjc+dL6AraG5zbaFBmlSvc6DkbFPpB0/za +5o0ELIHREkfGJ53XVL1P7kIgllKmg59ZBWsrGEF6WruJG0WCim57lHjgTgnrHKja +2bRW1KB9CNXylIEuobQKFFYhJsPEJ0g8UNVxRTVLNyJ7aSZs27hO8vGi+Gv7Gq7m +61seFdUCAwEAAaOCAz0wggM5MB8GA1UdIwQYMBaAFD3TUKXWoK3u80pgCmXTIdT4 ++NYPMB0GA1UdDgQWBBT4o6dhq9l3SxlmkMef45/msEQhBjBsBgNVHREEZTBjghB3 +d3cuZGlnaWNlcnQuY29tghRjb250ZW50LmRpZ2ljZXJ0LmNvbYIMZGlnaWNlcnQu +Y29tghd3d3cub3JpZ2luLmRpZ2ljZXJ0LmNvbYISbG9naW4uZGlnaWNlcnQuY29t +MA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIw +dQYDVR0fBG4wbDA0oDKgMIYuaHR0cDovL2NybDMuZGlnaWNlcnQuY29tL3NoYTIt +ZXYtc2VydmVyLWcxLmNybDA0oDKgMIYuaHR0cDovL2NybDQuZGlnaWNlcnQuY29t +L3NoYTItZXYtc2VydmVyLWcxLmNybDBCBgNVHSAEOzA5MDcGCWCGSAGG/WwCATAq +MCgGCCsGAQUFBwIBFhxodHRwczovL3d3dy5kaWdpY2VydC5jb20vQ1BTMIGIBggr +BgEFBQcBAQR8MHowJAYIKwYBBQUHMAGGGGh0dHA6Ly9vY3NwLmRpZ2ljZXJ0LmNv +bTBSBggrBgEFBQcwAoZGaHR0cDovL2NhY2VydHMuZGlnaWNlcnQuY29tL0RpZ2lD +ZXJ0U0hBMkV4dGVuZGVkVmFsaWRhdGlvblNlcnZlckNBLmNydDAMBgNVHRMBAf8E +AjAAMIIBBAYKKwYBBAHWeQIEAgSB9QSB8gDwAHYApLkJkLQYWBSHuxOizGdwCjw1 +mAT5G9+443fNDsgN3BAAAAFGAfj4EQAABAMARzBFAiBCsenxdlQ3dZBcfRMYphsr +6HrdAPxcfKp5U9BB8pU55gIhAI/rMx1PyNPn55YZxVp4zKgn9LAd2fAzCHvrfAeE +k4NrAHYAaPaY+B9kgr46jO65KB1M/HFRXWeT1ETRCmesu09P+8QAAAFGAfj4DQAA +BAMARzBFAiEAg8qcGjwzNf44Xz/TX4sj8HvgVHi66rY+p7tuGLXECS8CIAdmUF8C +CHveoNoZf5Gb1peHsT8Egl7sN2AL2WvbF+OYMA0GCSqGSIb3DQEBCwUAA4IBAQAt +nIIupEenVPHngDTSHo+3jvC0jtCatrc2HxciDQ6Rf7+d6m96qRjNjGCKTcnqswuN +vXcwlz716XIAMzPNO9YTFKOnTfzdwZcs5fYaJJc9eRIBm8icbialjb2dqLG9EFYR +BdY7VtwMQs2M3IEwWsl5hAsDEZkGDjL3uTONWfzl5CWj9olBfzI4RFY+4rHa/kML +WlwZqlMPruOGLN7HThOJ6KeTUkVxBjUusO1Nl3Ye7FCE9hXOhgSrq+CT/o7P9VPT +Q9FXgnA36oSFOPyD64yfMF8xT1fC5ogluE7smQcjkPFRLcoPq5pYMxIsYr3Z18rw +DcxdKIGW/9KPNNapvbom +-----END CERTIFICATE-----""" + + x509 = load_certificate(FILETYPE_PEM, ALTSUBJ_X509) + ext = x509.get_extension(2) + + + def check_alt_name_ext(self): + for i in xrange(0, int(self.iterations) * 100): + x = str(Checker_AltSubjectNameExt.ext) class Checker_X509_get_pubkey(BaseChecker): From 368018c6cc41c0de8cbd023378ff1c71b80a72e6 Mon Sep 17 00:00:00 2001 From: "Carlos D. Garza" Date: Thu, 10 Jul 2014 05:58:53 -0500 Subject: [PATCH 2/5] Calling GENERAL_NAMES_free in _subjectAltNameString member method of X509Extenion method to avoid memory leaks.fixes https://github.com/pyca/pyopenssl/issues/139 --- OpenSSL/crypto.py | 1 + 1 file changed, 1 insertion(+) diff --git a/OpenSSL/crypto.py b/OpenSSL/crypto.py index 54569eaab..6f8fd57f5 100644 --- a/OpenSSL/crypto.py +++ b/OpenSSL/crypto.py @@ -690,6 +690,7 @@ def _subjectAltNameString(self): value = _native( _ffi.buffer(name.d.ia5.data, name.d.ia5.length)[:]) parts.append(label + ":" + value) + _lib.GENERAL_NAMES_free(names) return ", ".join(parts) From 09a475387cd8d4afb3a59498a75956ea8b885c9e Mon Sep 17 00:00:00 2001 From: "Carlos D. Garza" Date: Tue, 15 Jul 2014 16:40:21 -0500 Subject: [PATCH 3/5] Coding the fix to check if GENERAL_NAMES_free method exists before attempting to call it. --- OpenSSL/crypto.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/OpenSSL/crypto.py b/OpenSSL/crypto.py index 6f8fd57f5..1390f9a81 100644 --- a/OpenSSL/crypto.py +++ b/OpenSSL/crypto.py @@ -690,7 +690,10 @@ def _subjectAltNameString(self): value = _native( _ffi.buffer(name.d.ia5.data, name.d.ia5.length)[:]) parts.append(label + ":" + value) - _lib.GENERAL_NAMES_free(names) + if hasattr(_lib.GENERAL_NAMES_free): + #hopefully cryptography provides this so we can avoid memory + #leaks. + _lib.GENERAL_NAMES_free(names) return ", ".join(parts) From 7d2bc80cca4b19df2225ba3a0bb361b8f6a2c437 Mon Sep 17 00:00:00 2001 From: "Carlos D. Garza" Date: Tue, 15 Jul 2014 19:07:54 -0500 Subject: [PATCH 4/5] Fixing blatent missuse of parameter to hasattr. --- OpenSSL/crypto.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OpenSSL/crypto.py b/OpenSSL/crypto.py index 1390f9a81..f440a4d2c 100644 --- a/OpenSSL/crypto.py +++ b/OpenSSL/crypto.py @@ -690,7 +690,7 @@ def _subjectAltNameString(self): value = _native( _ffi.buffer(name.d.ia5.data, name.d.ia5.length)[:]) parts.append(label + ":" + value) - if hasattr(_lib.GENERAL_NAMES_free): + if hasattr(_lib,"GENERAL_NAMES_free"): #hopefully cryptography provides this so we can avoid memory #leaks. _lib.GENERAL_NAMES_free(names) From 55fd3a95a30936d8ea02710839c537e2fa4ec508 Mon Sep 17 00:00:00 2001 From: "Carlos D. Garza" Date: Tue, 22 Jul 2014 16:06:24 -0500 Subject: [PATCH 5/5] Removing hasattr on cryptography X509_GENERALNAMES_free method. --- OpenSSL/crypto.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/OpenSSL/crypto.py b/OpenSSL/crypto.py index f440a4d2c..6f8fd57f5 100644 --- a/OpenSSL/crypto.py +++ b/OpenSSL/crypto.py @@ -690,10 +690,7 @@ def _subjectAltNameString(self): value = _native( _ffi.buffer(name.d.ia5.data, name.d.ia5.length)[:]) parts.append(label + ":" + value) - if hasattr(_lib,"GENERAL_NAMES_free"): - #hopefully cryptography provides this so we can avoid memory - #leaks. - _lib.GENERAL_NAMES_free(names) + _lib.GENERAL_NAMES_free(names) return ", ".join(parts)