diff options
author | Ziga Seilnacht <ziga.seilnacht@gmail.com> | 2009-12-22 14:58:01 +0100 |
---|---|---|
committer | Jean-Paul Calderone <exarkun@divmod.com> | 2009-12-22 14:58:01 +0100 |
commit | 781295ae151fb709e613d1d30f47f31426f8008e (patch) | |
tree | 5e049389c765d7960b55ad8ea9c5cc7a6822eedb | |
parent | 6b90a40aaaae00de1ba4eef736255fc0eeb7f72a (diff) | |
download | pyopenssl-781295ae151fb709e613d1d30f47f31426f8008e.tar.gz |
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.
-rw-r--r-- | OpenSSL/crypto/crypto.c | 13 | ||||
-rw-r--r-- | 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 |