From cccb6968530da1f2a251b5a2285f74e2f20f538f Mon Sep 17 00:00:00 2001 From: Ziga Seilnacht Date: Tue, 22 Dec 2009 14:06:31 +0100 Subject: Whitespace cleanup. --- OpenSSL/crypto/crypto.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/OpenSSL/crypto/crypto.c b/OpenSSL/crypto/crypto.c index 1442b5a..8196be2 100644 --- a/OpenSSL/crypto/crypto.c +++ b/OpenSSL/crypto/crypto.c @@ -216,7 +216,7 @@ crypto_dump_privatekey(PyObject *spam, PyObject *args) case X509_FILETYPE_TEXT: rsa = EVP_PKEY_get1_RSA(pkey->pkey); ret = RSA_print(bio, rsa, 0); - RSA_free(rsa); + RSA_free(rsa); break; default: @@ -509,8 +509,8 @@ crypto_load_pkcs7_data(PyObject *spam, PyObject *args) if (!PyArg_ParseTuple(args, "is#:load_pkcs7_data", &type, &buffer, &len)) return NULL; - /* - * Try to read the pkcs7 data from the bio + /* + * Try to read the pkcs7 data from the bio */ bio = BIO_new_mem_buf(buffer, len); switch (type) -- cgit v1.2.1 From 6b90a40aaaae00de1ba4eef736255fc0eeb7f72a Mon Sep 17 00:00:00 2001 From: Ziga Seilnacht Date: Tue, 22 Dec 2009 14:33:47 +0100 Subject: Don't overwrite the error raised by the callback. --- OpenSSL/crypto/crypto.c | 26 +++++++++++++++++--------- OpenSSL/test/test_crypto.py | 41 +++++++++++++++++++++++++---------------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/OpenSSL/crypto/crypto.c b/OpenSSL/crypto/crypto.c index 8196be2..d9ce52e 100644 --- a/OpenSSL/crypto/crypto.c +++ b/OpenSSL/crypto/crypto.c @@ -61,6 +61,21 @@ global_passphrase_callback(char *buf, int len, int rwflag, void *cb_arg) return nchars; } +static PyObject * +raise_current_error(void) +{ + if (PyErr_Occurred()) { + /* + * The python exception from callback is more informative than + * OpenSSL's error. + */ + flush_error_queue(); + return NULL; + } + exception_from_error_queue(crypto_Error); + return NULL; +} + static char crypto_load_privatekey_doc[] = "\n\ Load a private key from a buffer\n\ \n\ @@ -127,8 +142,7 @@ crypto_load_privatekey(PyObject *spam, PyObject *args) if (pkey == NULL) { - exception_from_error_queue(crypto_Error); - return NULL; + return raise_current_error(); } return (PyObject *)crypto_PKey_New(pkey, 1); @@ -202,11 +216,6 @@ crypto_dump_privatekey(PyObject *spam, PyObject *args) { case X509_FILETYPE_PEM: ret = PEM_write_bio_PrivateKey(bio, pkey->pkey, cipher, NULL, 0, cb, cb_arg); - if (PyErr_Occurred()) - { - BIO_free(bio); - return NULL; - } break; case X509_FILETYPE_ASN1: @@ -228,8 +237,7 @@ crypto_dump_privatekey(PyObject *spam, PyObject *args) if (ret == 0) { BIO_free(bio); - exception_from_error_queue(crypto_Error); - return NULL; + return raise_current_error(); } buf_len = BIO_get_mem_data(bio, &temp); diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py index 56638e8..4438f8c 100644 --- a/OpenSSL/test/test_crypto.py +++ b/OpenSSL/test/test_crypto.py @@ -1990,6 +1990,18 @@ class FunctionTests(TestCase): self.assertTrue(isinstance(key, PKeyType)) + def test_load_privatekey_passphrase_exception(self): + """ + If the passphrase callback raises an exception, that exception is raised + by :py:obj:`load_privatekey`. + """ + def cb(ignored): + raise ArithmeticError + + self.assertRaises(ArithmeticError, + load_privatekey, FILETYPE_PEM, encryptedPrivateKeyPEM, cb) + + def test_load_privatekey_wrongPassphraseCallback(self): """ :py:obj:`load_privatekey` raises :py:obj:`OpenSSL.crypto.Error` when it is passed an @@ -2021,22 +2033,6 @@ class FunctionTests(TestCase): self.assertEqual(called, [False]) - def test_load_privatekey_passphrase_exception(self): - """ - An exception raised by the passphrase callback passed to - :py:obj:`load_privatekey` causes :py:obj:`OpenSSL.crypto.Error` to be raised. - - This isn't as nice as just letting the exception pass through. The - behavior might be changed to that eventually. - """ - def broken(ignored): - raise RuntimeError("This is not working.") - self.assertRaises( - Error, - load_privatekey, - FILETYPE_PEM, encryptedPrivateKeyPEM, broken) - - def test_dump_privatekey_wrong_args(self): """ :py:obj:`dump_privatekey` raises :py:obj:`TypeError` if called with the wrong number @@ -2171,6 +2167,19 @@ class FunctionTests(TestCase): self.assertEqual(loadedKey.bits(), key.bits()) + def test_dump_privatekey_passphrase_exception(self): + """ + L{dump_privatekey} should not overwrite the exception raised + by the passphrase callback. + """ + def cb(ignored): + raise ArithmeticError + + key = load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM) + self.assertRaises(ArithmeticError, + dump_privatekey, FILETYPE_PEM, key, "blowfish", cb) + + def test_load_pkcs7_data(self): """ :py:obj:`load_pkcs7_data` accepts a PKCS#7 string and returns an instance of -- cgit v1.2.1 From 781295ae151fb709e613d1d30f47f31426f8008e Mon Sep 17 00:00:00 2001 From: Ziga Seilnacht Date: Tue, 22 Dec 2009 14:58:01 +0100 Subject: Additional error checks and a refcount fix for global_passphrase_callback. There were two really big problems in this function: the first one was the silent truncation of passphrases, the second was the refcounting bug, which kept the passphrase in memory until the process exited. See tests for details. --- OpenSSL/crypto/crypto.c | 13 ++++++++-- OpenSSL/test/test_crypto.py | 58 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/OpenSSL/crypto/crypto.c b/OpenSSL/crypto/crypto.c index d9ce52e..ec4d0b3 100644 --- a/OpenSSL/crypto/crypto.c +++ b/OpenSSL/crypto/crypto.c @@ -45,19 +45,28 @@ global_passphrase_callback(char *buf, int len, int rwflag, void *cb_arg) func = (PyObject *)cb_arg; argv = Py_BuildValue("(i)", rwflag); + if (argv == NULL) { + return 0; + } ret = PyEval_CallObject(func, argv); Py_DECREF(argv); if (ret == NULL) return 0; if (!PyBytes_Check(ret)) { + Py_DECREF(ret); PyErr_SetString(PyExc_ValueError, "String expected"); return 0; } nchars = PyBytes_Size(ret); - if (nchars > len) - nchars = len; + if (nchars > len) { + Py_DECREF(ret); + PyErr_SetString(PyExc_ValueError, + "passphrase returned by callback is too long"); + return 0; + } strncpy(buf, PyBytes_AsString(ret), nchars); + Py_DECREF(ret); return nchars; } diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py index 4438f8c..e108528 100644 --- a/OpenSSL/test/test_crypto.py +++ b/OpenSSL/test/test_crypto.py @@ -7,7 +7,7 @@ Unit tests for :py:mod:`OpenSSL.crypto`. from unittest import main -import os, re +import os, re, sys from subprocess import PIPE, Popen from datetime import datetime, timedelta @@ -2075,6 +2075,33 @@ class FunctionTests(TestCase): self.assertRaises(ValueError, dump_privatekey, 100, key) + def test_load_privatekey_passphraseCallbackLeak(self): + """ + L{crypto.load_privatekey} should not leak a reference to the + passphrase when the passphrase is provided by a callback. + """ + def cb(ignored): + return encryptedPrivateKeyPEMPassphrase + + startCount = sys.getrefcount(encryptedPrivateKeyPEMPassphrase) + for i in range(100): + load_privatekey(FILETYPE_PEM, encryptedPrivateKeyPEM, cb) + endCount = sys.getrefcount(encryptedPrivateKeyPEMPassphrase) + self.assert_(endCount - startCount < 5, endCount - startCount) + + + def test_load_privatekey_passphraseCallbackLength(self): + """ + L{crypto.load_privatekey} should raise an error when the passphrase + provided by the callback is too long, not silently truncate it. + """ + def cb(ignored): + return "a" * 1025 + + self.assertRaises(ValueError, + load_privatekey, FILETYPE_PEM, encryptedPrivateKeyPEM, cb) + + def test_dump_privatekey_passphrase(self): """ :py:obj:`dump_privatekey` writes an encrypted PEM when given a passphrase. @@ -2180,6 +2207,35 @@ class FunctionTests(TestCase): dump_privatekey, FILETYPE_PEM, key, "blowfish", cb) + def test_dump_privatekey_passphraseCallbackLeak(self): + """ + L{crypto.dump_privatekey} should not leak a reference to the + passphrase when the passphrase is provided by a callback. + """ + def cb(ignored): + return encryptedPrivateKeyPEMPassphrase + + startCount = sys.getrefcount(encryptedPrivateKeyPEMPassphrase) + key = load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM) + for i in range(100): + dump_privatekey(FILETYPE_PEM, key, "blowfish", cb) + endCount = sys.getrefcount(encryptedPrivateKeyPEMPassphrase) + self.assert_(endCount - startCount < 5, endCount - startCount) + + + def test_dump_privatekey_passphraseCallbackLength(self): + """ + L{crypto.dump_privatekey} should raise an error when the passphrase + provided by the callback is too long, not silently truncate it. + """ + def cb(ignored): + return "a" * 1025 + + key = load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM) + self.assertRaises(ValueError, + dump_privatekey, FILETYPE_PEM, key, "blowfish", cb) + + def test_load_pkcs7_data(self): """ :py:obj:`load_pkcs7_data` accepts a PKCS#7 string and returns an instance of -- cgit v1.2.1 From 376cf9796ec555bc0baa599ed9aba0c4959d4127 Mon Sep 17 00:00:00 2001 From: Ziga Seilnacht Date: Tue, 22 Dec 2009 16:04:10 +0100 Subject: Raise an error if a passphrase is used with a private key format that does not support encryption. Otherwise users might get an unpleasant surprise once they learn that their private key, which they thought was secure, is in fact readable by everyone. --- OpenSSL/crypto/crypto.c | 59 ++++++++++++++++++++++----------------------- OpenSSL/test/test_crypto.py | 23 ++++++++++++++++++ 2 files changed, 52 insertions(+), 30 deletions(-) diff --git a/OpenSSL/crypto/crypto.c b/OpenSSL/crypto/crypto.c index ec4d0b3..b0e0d2f 100644 --- a/OpenSSL/crypto/crypto.c +++ b/OpenSSL/crypto/crypto.c @@ -85,6 +85,32 @@ raise_current_error(void) return NULL; } +static int +setup_callback(int type, PyObject *pw, pem_password_cb **cb, void **cb_arg) { + if (pw == NULL) { + *cb = NULL; + *cb_arg = NULL; + return 1; + } + if (type != X509_FILETYPE_PEM) { + PyErr_SetString(PyExc_ValueError, + "only FILETYPE_PEM key format supports encryption"); + return 0; + } + if (PyBytes_Check(pw)) { + *cb = NULL; + *cb_arg = PyBytes_AsString(pw); + } else if (PyCallable_Check(pw)) { + *cb = global_passphrase_callback; + *cb_arg = pw; + } else { + PyErr_SetString(PyExc_TypeError, + "Last argument must be string or callable"); + return 0; + } + return 1; +} + static char crypto_load_privatekey_doc[] = "\n\ Load a private key from a buffer\n\ \n\ @@ -112,23 +138,8 @@ crypto_load_privatekey(PyObject *spam, PyObject *args) if (!PyArg_ParseTuple(args, "is#|O:load_privatekey", &type, &buffer, &len, &pw)) return NULL; - if (pw != NULL) - { - if (PyBytes_Check(pw)) - { - cb = NULL; - cb_arg = PyBytes_AsString(pw); - } - else if (PyCallable_Check(pw)) - { - cb = global_passphrase_callback; - cb_arg = pw; - } - else - { - PyErr_SetString(PyExc_TypeError, "Last argument must be string or callable"); - return NULL; - } + if (!setup_callback(type, pw, &cb, &cb_arg)) { + return NULL; } bio = BIO_new_mem_buf(buffer, len); @@ -203,19 +214,7 @@ crypto_dump_privatekey(PyObject *spam, PyObject *args) PyErr_SetString(PyExc_ValueError, "Invalid cipher name"); return NULL; } - if (PyBytes_Check(pw)) - { - cb = NULL; - cb_arg = PyBytes_AsString(pw); - } - else if (PyCallable_Check(pw)) - { - cb = global_passphrase_callback; - cb_arg = pw; - } - else - { - PyErr_SetString(PyExc_TypeError, "Last argument must be string or callable"); + if (!setup_callback(type, pw, &cb, &cb_arg)) { return NULL; } } diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py index e108528..dba9ce8 100644 --- a/OpenSSL/test/test_crypto.py +++ b/OpenSSL/test/test_crypto.py @@ -1979,6 +1979,18 @@ class FunctionTests(TestCase): load_privatekey, FILETYPE_PEM, encryptedPrivateKeyPEM, b("quack")) + def test_load_privatekey_passphraseWrongType(self): + """ + L{load_privatekey} raises C{ValueError} when it is passed a passphrase + with a private key encoded in a format, that doesn't support + encryption. + """ + key = load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM) + blob = dump_privatekey(FILETYPE_ASN1, key) + self.assertRaises(ValueError, + load_privatekey, FILETYPE_ASN1, blob, "secret") + + def test_load_privatekey_passphrase(self): """ :py:obj:`load_privatekey` can create a :py:obj:`PKey` object from an encrypted PEM @@ -2116,6 +2128,17 @@ class FunctionTests(TestCase): self.assertEqual(loadedKey.bits(), key.bits()) + def test_dump_privatekey_passphraseWrongType(self): + """ + L{dump_privatekey} raises C{ValueError} when it is passed a passphrase + with a private key encoded in a format, that doesn't support + encryption. + """ + key = load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM) + self.assertRaises(ValueError, + dump_privatekey, FILETYPE_ASN1, key, "blowfish", "secret") + + def test_dump_certificate(self): """ :py:obj:`dump_certificate` writes PEM, DER, and text. -- cgit v1.2.1 From cd48be9bac3a3e66a53b1942477db03dc5b49ad3 Mon Sep 17 00:00:00 2001 From: Ziga Seilnacht Date: Tue, 22 Dec 2009 16:54:20 +0100 Subject: Add a few more error checks around OpenSSL API calls. These errors can only occur in low memory conditions, so there is no reasonable way to test them. --- OpenSSL/crypto/crypto.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/OpenSSL/crypto/crypto.c b/OpenSSL/crypto/crypto.c index b0e0d2f..d642dc6 100644 --- a/OpenSSL/crypto/crypto.c +++ b/OpenSSL/crypto/crypto.c @@ -143,6 +143,10 @@ crypto_load_privatekey(PyObject *spam, PyObject *args) } bio = BIO_new_mem_buf(buffer, len); + if (bio == NULL) { + exception_from_error_queue(crypto_Error); + return NULL; + } switch (type) { case X509_FILETYPE_PEM: @@ -220,6 +224,10 @@ crypto_dump_privatekey(PyObject *spam, PyObject *args) } bio = BIO_new(BIO_s_mem()); + if (bio == NULL) { + exception_from_error_queue(crypto_Error); + return NULL; + } switch (type) { case X509_FILETYPE_PEM: @@ -232,6 +240,10 @@ crypto_dump_privatekey(PyObject *spam, PyObject *args) case X509_FILETYPE_TEXT: rsa = EVP_PKEY_get1_RSA(pkey->pkey); + if (rsa == NULL) { + ret = 0; + break; + } ret = RSA_print(bio, rsa, 0); RSA_free(rsa); break; -- cgit v1.2.1 From 1a71a4ab1efe0635817383708046a16850666a60 Mon Sep 17 00:00:00 2001 From: Ziga Seilnacht Date: Tue, 22 Dec 2009 17:02:04 +0100 Subject: Unify code formatting in recently changed functions to what seems to be the currently preferred style. --- OpenSSL/crypto/crypto.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/OpenSSL/crypto/crypto.c b/OpenSSL/crypto/crypto.c index d642dc6..ad35ce9 100644 --- a/OpenSSL/crypto/crypto.c +++ b/OpenSSL/crypto/crypto.c @@ -50,10 +50,10 @@ global_passphrase_callback(char *buf, int len, int rwflag, void *cb_arg) } ret = PyEval_CallObject(func, argv); Py_DECREF(argv); - if (ret == NULL) + if (ret == NULL) { return 0; - if (!PyBytes_Check(ret)) - { + } + if (!PyBytes_Check(ret)) { Py_DECREF(ret); PyErr_SetString(PyExc_ValueError, "String expected"); return 0; @@ -135,9 +135,10 @@ crypto_load_privatekey(PyObject *spam, PyObject *args) BIO *bio; EVP_PKEY *pkey; - if (!PyArg_ParseTuple(args, "is#|O:load_privatekey", &type, &buffer, &len, &pw)) + if (!PyArg_ParseTuple(args, "is#|O:load_privatekey", + &type, &buffer, &len, &pw)) { return NULL; - + } if (!setup_callback(type, pw, &cb, &cb_arg)) { return NULL; } @@ -147,8 +148,7 @@ crypto_load_privatekey(PyObject *spam, PyObject *args) exception_from_error_queue(crypto_Error); return NULL; } - switch (type) - { + switch (type) { case X509_FILETYPE_PEM: pkey = PEM_read_bio_PrivateKey(bio, NULL, cb, cb_arg); break; @@ -164,8 +164,7 @@ crypto_load_privatekey(PyObject *spam, PyObject *args) } BIO_free(bio); - if (pkey == NULL) - { + if (pkey == NULL) { return raise_current_error(); } @@ -202,19 +201,16 @@ crypto_dump_privatekey(PyObject *spam, PyObject *args) crypto_PKeyObj *pkey; if (!PyArg_ParseTuple(args, "iO!|sO:dump_privatekey", &type, - &crypto_PKey_Type, &pkey, &cipher_name, &pw)) + &crypto_PKey_Type, &pkey, &cipher_name, &pw)) { return NULL; - - if (cipher_name != NULL && pw == NULL) - { + } + if (cipher_name != NULL && pw == NULL) { PyErr_SetString(PyExc_ValueError, "Illegal number of arguments"); return NULL; } - if (cipher_name != NULL) - { + if (cipher_name != NULL) { cipher = EVP_get_cipherbyname(cipher_name); - if (cipher == NULL) - { + if (cipher == NULL) { PyErr_SetString(PyExc_ValueError, "Invalid cipher name"); return NULL; } @@ -228,8 +224,7 @@ crypto_dump_privatekey(PyObject *spam, PyObject *args) exception_from_error_queue(crypto_Error); return NULL; } - switch (type) - { + switch (type) { case X509_FILETYPE_PEM: ret = PEM_write_bio_PrivateKey(bio, pkey->pkey, cipher, NULL, 0, cb, cb_arg); break; @@ -254,8 +249,7 @@ crypto_dump_privatekey(PyObject *spam, PyObject *args) return NULL; } - if (ret == 0) - { + if (ret == 0) { BIO_free(bio); return raise_current_error(); } -- cgit v1.2.1 From 5d9d7f1797d3ad8987dc760cf6f5c4a0f5747f19 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Sep 2011 09:48:58 -0400 Subject: epytext to rst --- OpenSSL/test/test_crypto.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py index dba9ce8..47b3df3 100644 --- a/OpenSSL/test/test_crypto.py +++ b/OpenSSL/test/test_crypto.py @@ -1981,7 +1981,7 @@ class FunctionTests(TestCase): def test_load_privatekey_passphraseWrongType(self): """ - L{load_privatekey} raises C{ValueError} when it is passed a passphrase + :py:obj:`load_privatekey` raises :py:obj:`ValueError` when it is passed a passphrase with a private key encoded in a format, that doesn't support encryption. """ @@ -2089,7 +2089,7 @@ class FunctionTests(TestCase): def test_load_privatekey_passphraseCallbackLeak(self): """ - L{crypto.load_privatekey} should not leak a reference to the + :py:obj:`crypto.load_privatekey` should not leak a reference to the passphrase when the passphrase is provided by a callback. """ def cb(ignored): @@ -2104,7 +2104,7 @@ class FunctionTests(TestCase): def test_load_privatekey_passphraseCallbackLength(self): """ - L{crypto.load_privatekey} should raise an error when the passphrase + :py:obj:`crypto.load_privatekey` should raise an error when the passphrase provided by the callback is too long, not silently truncate it. """ def cb(ignored): @@ -2130,7 +2130,7 @@ class FunctionTests(TestCase): def test_dump_privatekey_passphraseWrongType(self): """ - L{dump_privatekey} raises C{ValueError} when it is passed a passphrase + :py:obj:`dump_privatekey` raises :py:obj:`ValueError` when it is passed a passphrase with a private key encoded in a format, that doesn't support encryption. """ @@ -2219,7 +2219,7 @@ class FunctionTests(TestCase): def test_dump_privatekey_passphrase_exception(self): """ - L{dump_privatekey} should not overwrite the exception raised + :py:obj:`dump_privatekey` should not overwrite the exception raised by the passphrase callback. """ def cb(ignored): @@ -2232,7 +2232,7 @@ class FunctionTests(TestCase): def test_dump_privatekey_passphraseCallbackLeak(self): """ - L{crypto.dump_privatekey} should not leak a reference to the + :py:obj:`crypto.dump_privatekey` should not leak a reference to the passphrase when the passphrase is provided by a callback. """ def cb(ignored): @@ -2248,7 +2248,7 @@ class FunctionTests(TestCase): def test_dump_privatekey_passphraseCallbackLength(self): """ - L{crypto.dump_privatekey} should raise an error when the passphrase + :py:obj:`crypto.dump_privatekey` should raise an error when the passphrase provided by the callback is too long, not silently truncate it. """ def cb(ignored): -- cgit v1.2.1 From 8e37f7628d1974176ab57bdad52429395c46a5fc Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Sep 2011 10:00:50 -0400 Subject: Add leak checkers for some cases of load_privatekey, notice one leaks against trunk --- leakcheck/crypto.py | 71 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 5 deletions(-) diff --git a/leakcheck/crypto.py b/leakcheck/crypto.py index 07b77e5..c12d8ff 100644 --- a/leakcheck/crypto.py +++ b/leakcheck/crypto.py @@ -3,16 +3,21 @@ import sys -from OpenSSL.crypto import TYPE_DSA, Error, PKey, X509 +from OpenSSL.crypto import ( + FILETYPE_PEM, TYPE_DSA, Error, PKey, X509, load_privatekey) -class Checker_X509_get_pubkey(object): - """ - Leak checks for L{X509.get_pubkey}. - """ + + +class BaseChecker(object): def __init__(self, iterations): self.iterations = iterations + +class Checker_X509_get_pubkey(BaseChecker): + """ + Leak checks for L{X509.get_pubkey}. + """ def check_exception(self): """ Call the method repeatedly such that it will raise an exception. @@ -40,6 +45,62 @@ class Checker_X509_get_pubkey(object): cert.get_pubkey() + +class Checker_load_privatekey(BaseChecker): + """ + Leak checks for :py:obj:`load_privatekey`. + """ + ENCRYPTED_PEM = """\ +-----BEGIN RSA PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: BF-CBC,3763C340F9B5A1D0 + +a/DO10mLjHLCAOG8/Hc5Lbuh3pfjvcTZiCexShP+tupkp0VxW2YbZjML8uoXrpA6 +fSPUo7cEC+r96GjV03ZIVhjmsxxesdWMpfkzXRpG8rUbWEW2KcCJWdSX8bEkuNW3 +uvAXdXZwiOrm56ANDo/48gj27GcLwnlA8ld39+ylAzkUJ1tcMVzzTjfcyd6BMFpR +Yjg23ikseug6iWEsZQormdl0ITdYzmFpM+YYsG7kmmmi4UjCEYfb9zFaqJn+WZT2 +qXxmo2ZPFzmEVkuB46mf5GCqMwLRN2QTbIZX2+Dljj1Hfo5erf5jROewE/yzcTwO +FCB5K3c2kkTv2KjcCAimjxkE+SBKfHg35W0wB0AWkXpVFO5W/TbHg4tqtkpt/KMn +/MPnSxvYr/vEqYMfW4Y83c45iqK0Cyr2pwY60lcn8Kk= +-----END RSA PRIVATE KEY----- +""" + def check_load_privatekey_callback(self): + """ + Call the function with an encrypted PEM and a passphrase callback. + """ + for i in xrange(self.iterations * 10): + load_privatekey( + FILETYPE_PEM, self.ENCRYPTED_PEM, lambda *args: "hello, secret") + + + def check_load_privatekey_callback_incorrect(self): + """ + Call the function with an encrypted PEM and a passphrase callback which + returns the wrong passphrase. + """ + for i in xrange(self.iterations * 10): + try: + load_privatekey( + FILETYPE_PEM, self.ENCRYPTED_PEM, + lambda *args: "hello, public") + except Error: + pass + + + def check_load_privatekey_callback_wrong_type(self): + """ + Call the function with an encrypted PEM and a passphrase callback which + returns a non-string. + """ + for i in xrange(self.iterations * 10): + try: + load_privatekey( + FILETYPE_PEM, self.ENCRYPTED_PEM, + lambda *args: {}) + except Error: + pass + + def vmsize(): return [x for x in file('/proc/self/status').readlines() if 'VmSize' in x] -- cgit v1.2.1 From 105cb95768adb64babd682a8038f1edea337ff27 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Sep 2011 10:16:46 -0400 Subject: Add a test for returning the wrong type from a passphrase callback --- OpenSSL/test/test_crypto.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py index 47b3df3..4c66d6e 100644 --- a/OpenSSL/test/test_crypto.py +++ b/OpenSSL/test/test_crypto.py @@ -2045,6 +2045,17 @@ class FunctionTests(TestCase): self.assertEqual(called, [False]) + def test_load_privatekey_passphrase_wrong_return_type(self): + """ + :py:obj:`load_privatekey` raises :py:obj:`ValueError` if the passphrase + callback returns something other than a byte string. + """ + self.assertRaises( + ValueError, + load_privatekey, + FILETYPE_PEM, encryptedPrivateKeyPEM, lambda *args: 3) + + def test_dump_privatekey_wrong_args(self): """ :py:obj:`dump_privatekey` raises :py:obj:`TypeError` if called with the wrong number -- cgit v1.2.1 From 1eeccd892c4a01d4b1d496e2faee8c8118d26da7 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Sep 2011 10:18:52 -0400 Subject: Add a test for a special argument handling check of dump_privatekey --- OpenSSL/test/test_crypto.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py index 4c66d6e..17a26b0 100644 --- a/OpenSSL/test/test_crypto.py +++ b/OpenSSL/test/test_crypto.py @@ -2062,6 +2062,9 @@ class FunctionTests(TestCase): of arguments. """ self.assertRaises(TypeError, dump_privatekey) + # If cipher name is given, password is required. + self.assertRaises( + ValueError, dump_privatekey, FILETYPE_PEM, PKey(), "foo") def test_dump_privatekey_unknown_cipher(self): -- cgit v1.2.1 From bfac21719aeab888d99eb0d77317aff1f7a81613 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Sep 2011 10:54:25 -0400 Subject: Avoid being CPython specific - the leakcheck script will catch these issues. --- OpenSSL/test/test_crypto.py | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py index 17a26b0..cc0ede2 100644 --- a/OpenSSL/test/test_crypto.py +++ b/OpenSSL/test/test_crypto.py @@ -2101,21 +2101,6 @@ class FunctionTests(TestCase): self.assertRaises(ValueError, dump_privatekey, 100, key) - def test_load_privatekey_passphraseCallbackLeak(self): - """ - :py:obj:`crypto.load_privatekey` should not leak a reference to the - passphrase when the passphrase is provided by a callback. - """ - def cb(ignored): - return encryptedPrivateKeyPEMPassphrase - - startCount = sys.getrefcount(encryptedPrivateKeyPEMPassphrase) - for i in range(100): - load_privatekey(FILETYPE_PEM, encryptedPrivateKeyPEM, cb) - endCount = sys.getrefcount(encryptedPrivateKeyPEMPassphrase) - self.assert_(endCount - startCount < 5, endCount - startCount) - - def test_load_privatekey_passphraseCallbackLength(self): """ :py:obj:`crypto.load_privatekey` should raise an error when the passphrase @@ -2244,22 +2229,6 @@ class FunctionTests(TestCase): dump_privatekey, FILETYPE_PEM, key, "blowfish", cb) - def test_dump_privatekey_passphraseCallbackLeak(self): - """ - :py:obj:`crypto.dump_privatekey` should not leak a reference to the - passphrase when the passphrase is provided by a callback. - """ - def cb(ignored): - return encryptedPrivateKeyPEMPassphrase - - startCount = sys.getrefcount(encryptedPrivateKeyPEMPassphrase) - key = load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM) - for i in range(100): - dump_privatekey(FILETYPE_PEM, key, "blowfish", cb) - endCount = sys.getrefcount(encryptedPrivateKeyPEMPassphrase) - self.assert_(endCount - startCount < 5, endCount - startCount) - - def test_dump_privatekey_passphraseCallbackLength(self): """ :py:obj:`crypto.dump_privatekey` should raise an error when the passphrase -- cgit v1.2.1 From d440a083e0077c5e3cbeafa6092bccdc4b2b29da Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Sep 2011 11:02:05 -0400 Subject: Python 3.x compatibility --- OpenSSL/test/test_crypto.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py index cc0ede2..e0d7b27 100644 --- a/OpenSSL/test/test_crypto.py +++ b/OpenSSL/test/test_crypto.py @@ -2016,14 +2016,14 @@ class FunctionTests(TestCase): def test_load_privatekey_wrongPassphraseCallback(self): """ - :py:obj:`load_privatekey` raises :py:obj:`OpenSSL.crypto.Error` when it is passed an - encrypted PEM and a passphrase callback which returns an incorrect - passphrase. + :py:obj:`load_privatekey` raises :py:obj:`OpenSSL.crypto.Error` when it + is passed an encrypted PEM and a passphrase callback which returns an + incorrect passphrase. """ called = [] def cb(*a): called.append(None) - return "quack" + return b("quack") self.assertRaises( Error, load_privatekey, FILETYPE_PEM, encryptedPrivateKeyPEM, cb) -- cgit v1.2.1 From 2a864f111fb0df5f53a173ad8adac47e81ce5d93 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Sep 2011 11:10:29 -0400 Subject: The callback exception now propagates, yay --- leakcheck/crypto.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/leakcheck/crypto.py b/leakcheck/crypto.py index c12d8ff..6a9af92 100644 --- a/leakcheck/crypto.py +++ b/leakcheck/crypto.py @@ -97,7 +97,7 @@ FCB5K3c2kkTv2KjcCAimjxkE+SBKfHg35W0wB0AWkXpVFO5W/TbHg4tqtkpt/KMn load_privatekey( FILETYPE_PEM, self.ENCRYPTED_PEM, lambda *args: {}) - except Error: + except ValueError: pass -- cgit v1.2.1