diff -up pyOpenSSL-0.13.1/OpenSSL/crypto/crypto.c.exception pyOpenSSL-0.13.1/OpenSSL/crypto/crypto.c --- pyOpenSSL-0.13.1/OpenSSL/crypto/crypto.c.exception 2018-05-04 18:56:35.946486574 +0200 +++ pyOpenSSL-0.13.1/OpenSSL/crypto/crypto.c 2018-05-04 18:56:35.950486667 +0200 @@ -45,26 +45,72 @@ global_passphrase_callback(char *buf, in func = (PyObject *)cb_arg; argv = Py_BuildValue("(i)", rwflag); - if (argv == NULL) + if (argv == NULL) { return 0; + } 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; } 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; } +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 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\ @@ -89,31 +135,20 @@ crypto_load_privatekey(PyObject *spam, P 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; - - 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; - } } bio = BIO_new_mem_buf(buffer, len); - switch (type) - { + if (bio == NULL) { + exception_from_error_queue(crypto_Error); + return NULL; + } + switch (type) { case X509_FILETYPE_PEM: pkey = PEM_read_bio_PrivateKey(bio, NULL, cb, cb_arg); break; @@ -129,10 +164,8 @@ crypto_load_privatekey(PyObject *spam, P } BIO_free(bio); - if (pkey == NULL) - { - exception_from_error_queue(crypto_Error); - return NULL; + if (pkey == NULL) { + return raise_current_error(); } return (PyObject *)crypto_PKey_New(pkey, 1); @@ -168,49 +201,32 @@ crypto_dump_privatekey(PyObject *spam, P 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; } - 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; } } bio = BIO_new(BIO_s_mem()); - switch (type) - { + if (bio == NULL) { + exception_from_error_queue(crypto_Error); + return NULL; + } + switch (type) { 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: @@ -219,8 +235,12 @@ crypto_dump_privatekey(PyObject *spam, P 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); + RSA_free(rsa); break; default: @@ -229,11 +249,9 @@ crypto_dump_privatekey(PyObject *spam, P return NULL; } - if (ret == 0) - { + 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); @@ -513,8 +531,8 @@ crypto_load_pkcs7_data(PyObject *spam, P 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) diff -up pyOpenSSL-0.13.1/OpenSSL/test/test_crypto.py.exception pyOpenSSL-0.13.1/OpenSSL/test/test_crypto.py --- pyOpenSSL-0.13.1/OpenSSL/test/test_crypto.py.exception 2018-05-04 18:56:35.948486620 +0200 +++ pyOpenSSL-0.13.1/OpenSSL/test/test_crypto.py 2018-05-04 18:57:16.363420609 +0200 @@ -7,7 +7,7 @@ Unit tests for L{OpenSSL.crypto}. from unittest import main -import os, re +import os, re, sys from subprocess import PIPE, Popen from datetime import datetime, timedelta @@ -2038,6 +2038,18 @@ class FunctionTests(TestCase): load_privatekey, FILETYPE_PEM, encryptedPrivateKeyPEM, b("quack")) + def test_load_privatekey_passphraseWrongType(self): + """ + :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. + """ + 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): """ L{load_privatekey} can create a L{PKey} object from an encrypted PEM @@ -2058,7 +2070,7 @@ class FunctionTests(TestCase): called = [] def cb(*a): called.append(None) - return "quack" + return b("quack") self.assertRaises( Error, load_privatekey, FILETYPE_PEM, encryptedPrivateKeyPEM, cb) @@ -2083,25 +2095,36 @@ class FunctionTests(TestCase): def test_load_privatekey_passphrase_exception(self): """ An exception raised by the passphrase callback passed to - L{load_privatekey} causes L{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. + L{load_privatekey} is propagated. """ def broken(ignored): raise RuntimeError("This is not working.") self.assertRaises( - Error, + RuntimeError, load_privatekey, FILETYPE_PEM, encryptedPrivateKeyPEM, broken) + 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): """ L{dump_privatekey} raises L{TypeError} if called with the wrong number 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): @@ -2138,6 +2161,18 @@ class FunctionTests(TestCase): self.assertRaises(ValueError, dump_privatekey, 100, key) + def test_load_privatekey_passphraseCallbackLength(self): + """ + :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): + return "a" * 1025 + + self.assertRaises(ValueError, + load_privatekey, FILETYPE_PEM, encryptedPrivateKeyPEM, cb) + + def test_dump_privatekey_passphrase(self): """ L{dump_privatekey} writes an encrypted PEM when given a passphrase. @@ -2152,6 +2187,17 @@ class FunctionTests(TestCase): self.assertEqual(loadedKey.bits(), key.bits()) + def test_dump_privatekey_passphraseWrongType(self): + """ + :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. + """ + key = load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM) + self.assertRaises(ValueError, + dump_privatekey, FILETYPE_ASN1, key, "blowfish", "secret") + + def test_dump_certificate(self): """ L{dump_certificate} writes PEM, DER, and text. @@ -2230,6 +2276,32 @@ class FunctionTests(TestCase): self.assertEqual(loadedKey.bits(), key.bits()) + def test_dump_privatekey_passphrase_exception(self): + """ + :py:obj:`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_dump_privatekey_passphraseCallbackLength(self): + """ + :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): + 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): """ L{load_pkcs7_data} accepts a PKCS#7 string and returns an instance of