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.

1 files changed, 57 insertions, 1 deletions

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