ssl: disable (broken) i/o on DER encoded keys

QSslKey currently has methods which supposedly allow decoding and
encoding private keys as DER protected by a passphrase. This is
broken by design as explained in QTBUG-41038, as storing the encrypted
DER data alone makes no sense: such a file lacks the necessary
information about the encryption algorithm and initialization vector.

This change:

- explicitly stops using the passphrase when decoding DER in the
  constructor. The behavior is unchanged, it is not possible to
  read the encrypted DER alone.

- refuses to honor the passphrase to DER encode a private key. The toDer
  method now outputs an empty QByteArray instead of garbage.

Task-number: QTBUG-41038
Change-Id: I4281050cf1104f12d154db201a173633bfe22bd9
Reviewed-by: Richard J. Moore <rich@kde.org>

src/network/ssl/qsslcertificate_qt.cpp

@@ -143,7 +143,7 @@ QSslKey QSslCertificate::publicKey() const
     key.d->type = QSsl::PublicKey;
     if (d->publicKeyAlgorithm != QSsl::Opaque) {
     key.d->algorithm = d->publicKeyAlgorithm;
-    key.d->decodeDer(d->publicKeyDerData, QByteArray());
+    key.d->decodeDer(d->publicKeyDerData);
     }
     return key;
 }

src/network/ssl/qsslkey_openssl.cpp

@@ -109,10 +109,9 @@ bool QSslKeyPrivate::fromEVP_PKEY(EVP_PKEY *pkey)
     return false;
 }
 
-void QSslKeyPrivate::decodeDer(const QByteArray &der, const QByteArray &passPhrase,
-                               bool deepClear)
+void QSslKeyPrivate::decodeDer(const QByteArray &der, bool deepClear)
 {
-    decodePem(pemFromDer(der), passPhrase, deepClear);
+    decodePem(pemFromDer(der), QByteArray(), deepClear);
 }
 
 void QSslKeyPrivate::decodePem(const QByteArray &pem, const QByteArray &passPhrase,

src/network/ssl/qsslkey_p.cpp

@@ -175,8 +175,10 @@ QByteArray QSslKeyPrivate::derFromPem(const QByteArray &pem) const
 /*!
     Constructs a QSslKey by decoding the string in the byte array
     \a encoded using a specified \a algorithm and \a encoding format.
-    If the encoded key is encrypted, \a passPhrase is used to decrypt
-    it. \a type specifies whether the key is public or private.
+    \a type specifies whether the key is public or private.
+
+    If the key is encoded as PEM and encrypted, \a passPhrase is used
+    to decrypt it.
 
     After construction, use isNull() to check if \a encoded contained
     a valid key.
@@ -188,7 +190,7 @@ QSslKey::QSslKey(const QByteArray &encoded, QSsl::KeyAlgorithm algorithm,
     d->type = type;
     d->algorithm = algorithm;
     if (encoding == QSsl::Der)
-        d->decodeDer(encoded, passPhrase);
+        d->decodeDer(encoded);
     else
         d->decodePem(encoded, passPhrase);
 }
@@ -196,8 +198,10 @@ QSslKey::QSslKey(const QByteArray &encoded, QSsl::KeyAlgorithm algorithm,
 /*!
     Constructs a QSslKey by reading and decoding data from a
     \a device using a specified \a algorithm and \a encoding format.
-    If the encoded key is encrypted, \a passPhrase is used to decrypt
-    it. \a type specifies whether the key is public or private.
+    \a type specifies whether the key is public or private.
+
+    If the key is encoded as PEM and encrypted, \a passPhrase is used
+    to decrypt it.
 
     After construction, use isNull() to check if \a device provided
     a valid key.
@@ -211,9 +215,10 @@ QSslKey::QSslKey(QIODevice *device, QSsl::KeyAlgorithm algorithm, QSsl::Encoding
         encoded = device->readAll();
     d->type = type;
     d->algorithm = algorithm;
-    d->decodePem((encoding == QSsl::Der) ?
-                 d->pemFromDer(encoded) : encoded,
-                 passPhrase);
+    if (encoding == QSsl::Der)
+        d->decodeDer(encoded);
+    else
+        d->decodePem(encoded, passPhrase);
 }
 
 /*!
@@ -317,23 +322,23 @@ QSsl::KeyAlgorithm QSslKey::algorithm() const
 }
 
 /*!
-  Returns the key in DER encoding. The result is encrypted with
-  \a passPhrase if the key is a private key and \a passPhrase is
-  non-empty.
+  Returns the key in DER encoding.
+
+  The \a passPhrase argument should be omitted as DER cannot be
+  encrypted. It will be removed in a future version of Qt.
 */
-// ### autotest failure for non-empty passPhrase and private key
 QByteArray QSslKey::toDer(const QByteArray &passPhrase) const
 {
     if (d->isNull || d->algorithm == QSsl::Opaque)
         return QByteArray();
 
-#ifndef QT_NO_OPENSSL
-    return d->derFromPem(toPem(passPhrase));
-#else
     // Encrypted DER is nonsense, see QTBUG-41038.
     if (d->type == QSsl::PrivateKey && !passPhrase.isEmpty())
         return QByteArray();
 
+#ifndef QT_NO_OPENSSL
+    return d->derFromPem(toPem(passPhrase));
+#else
     return d->derData;
 #endif
 }

src/network/ssl/qsslkey_p.h

@@ -86,8 +86,7 @@ public:
 #ifndef QT_NO_OPENSSL
     bool fromEVP_PKEY(EVP_PKEY *pkey);
 #endif
-    void decodeDer(const QByteArray &der, const QByteArray &passPhrase,
-                   bool deepClear = true);
+    void decodeDer(const QByteArray &der, bool deepClear = true);
     void decodePem(const QByteArray &pem, const QByteArray &passPhrase,
                    bool deepClear = true);
     QByteArray pemHeader() const;

src/network/ssl/qsslkey_qt.cpp

@@ -86,19 +86,13 @@ void QSslKeyPrivate::clear(bool deep)
     keyLength = -1;
 }
 
-void QSslKeyPrivate::decodeDer(const QByteArray &der, const QByteArray &passPhrase,
-                               bool deepClear)
+void QSslKeyPrivate::decodeDer(const QByteArray &der, bool deepClear)
 {
     clear(deepClear);
 
     if (der.isEmpty())
         return;
 
-    if (type == QSsl::PrivateKey && !passPhrase.isEmpty()) {
-        Q_UNIMPLEMENTED();
-        return;
-    }
-
     QAsn1Element elem;
     if (!elem.read(der) || elem.type() != QAsn1Element::SequenceType)
         return;
@@ -161,7 +155,12 @@ void QSslKeyPrivate::decodeDer(const QByteArray &der, const QByteArray &passPhra
 void QSslKeyPrivate::decodePem(const QByteArray &pem, const QByteArray &passPhrase,
                                bool deepClear)
 {
-    decodeDer(derFromPem(pem), passPhrase, deepClear);
+    if (type == QSsl::PrivateKey && !passPhrase.isEmpty()) {
+        Q_UNIMPLEMENTED();
+        return;
+    }
+
+    decodeDer(derFromPem(pem), deepClear);
 }
 
 int QSslKeyPrivate::length() const

tests/auto/network/ssl/qsslkey/tst_qsslkey.cpp

@@ -325,18 +325,10 @@ void tst_QSslKey::toEncryptedPemOrDer()
     }
 
     if (type == QSsl::PrivateKey) {
+        // verify that private keys are never "encrypted" by toDer() and
+        // instead an empty string is returned, see QTBUG-41038.
         QByteArray encryptedDer = key.toDer(pwBytes);
-        // ### at this point, encryptedDer is invalid, hence the below QEXPECT_FAILs
-        QVERIFY(!encryptedDer.isEmpty());
-        QSslKey keyDer(encryptedDer, algorithm, QSsl::Der, type, pwBytes);
-        if (type == QSsl::PrivateKey)
-            QEXPECT_FAIL(
-                QTest::currentDataTag(), "We're not able to decrypt these yet...", Continue);
-        QVERIFY(!keyDer.isNull());
-        if (type == QSsl::PrivateKey)
-            QEXPECT_FAIL(
-                QTest::currentDataTag(), "We're not able to decrypt these yet...", Continue);
-        QCOMPARE(keyDer.toPem(), key.toPem());
+        QVERIFY(encryptedDer.isEmpty());
     } else {
         // verify that public keys are never encrypted by toDer()
         QByteArray encryptedDer = key.toDer(pwBytes);