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.

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.