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.
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 @@
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 @@
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 @@
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 @@
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