From f8e551cf088bff08de95132ed40d5850f8547fef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Thu, 1 Feb 2018 16:25:49 +0100 Subject: Fix loading pkcs#8 encrypted DER-encoded keys in openssl When we load DER-encoded keys in the openssl-backend we always turn it into PEM-encoded keys (essentially we prepend and append a header and footer and use 'toBase64' on the DER data). The problem comes from the header and footer which is simply chosen based on which key algorithm was chosen by the user. Which would be wrong when the key is a PKCS#8 key. This caused OpenSSL to fail when trying to read it. Surprisingly it still loads correctly for unencrypted keys with the wrong header, but not for encrypted keys. This patch adds a small function which checks if a key is an encrypted PKCS#8 key and then uses this function to figure out if a PKCS#8 header and footer should be used (note that I only do this for encrypted PKCS#8 keys since, as previously mentioned, unencrypted keys are read correctly by openssl). The passphrase is now also passed to the QSslKeyPrivate::decodeDer function so DER-encoded files can actually be decrypted. [ChangeLog][QtNetwork][QSslKey] The openssl backend can now load encrypted PKCS#8 DER-encoded keys. Task-number: QTBUG-17718 Change-Id: I52eedf19bde297c9aa7fb050e835b3fc0db724e2 Reviewed-by: Edward Welbourne --- tests/auto/network/ssl/qsslkey/tst_qsslkey.cpp | 43 +++++++++++++++++++++----- 1 file changed, 35 insertions(+), 8 deletions(-) (limited to 'tests/auto/network/ssl/qsslkey/tst_qsslkey.cpp') diff --git a/tests/auto/network/ssl/qsslkey/tst_qsslkey.cpp b/tests/auto/network/ssl/qsslkey/tst_qsslkey.cpp index 27d92db3bf..e39bcd30e5 100644 --- a/tests/auto/network/ssl/qsslkey/tst_qsslkey.cpp +++ b/tests/auto/network/ssl/qsslkey/tst_qsslkey.cpp @@ -110,10 +110,10 @@ void tst_QSslKey::initTestCase() testDataDir += QLatin1String("/"); QDir dir(testDataDir + "keys"); - QFileInfoList fileInfoList = dir.entryInfoList(QDir::Files | QDir::Readable); - QRegExp rx(QLatin1String("^(rsa|dsa|ec)-(pub|pri)-(\\d+)-?\\w*\\.(pem|der)$")); - foreach (QFileInfo fileInfo, fileInfoList) { - if (rx.indexIn(fileInfo.fileName()) >= 0) + const QFileInfoList fileInfoList = dir.entryInfoList(QDir::Files | QDir::Readable); + QRegExp rx(QLatin1String("^(rsa|dsa|ec)-(pub|pri)-(\\d+)-?[\\w-]*\\.(pem|der)$")); + for (const QFileInfo &fileInfo : fileInfoList) { + if (rx.indexIn(fileInfo.fileName()) >= 0) { keyInfoList << KeyInfo( fileInfo, rx.cap(1) == QLatin1String("rsa") ? QSsl::Rsa : @@ -121,6 +121,7 @@ void tst_QSslKey::initTestCase() rx.cap(2) == QLatin1String("pub") ? QSsl::PublicKey : QSsl::PrivateKey, rx.cap(3).toInt(), rx.cap(4) == QLatin1String("pem") ? QSsl::Pem : QSsl::Der); + } } } @@ -164,6 +165,11 @@ void tst_QSslKey::createPlainTestRows(bool filter, QSsl::EncodingFormat format) if (filter && keyInfo.format != format) continue; +#if !defined(QT_NO_SSL) && defined(QT_NO_OPENSSL) // generic backend + if (keyInfo.fileInfo.fileName().contains("pkcs8")) + continue; // The generic backend does not support pkcs8 (yet) +#endif + QTest::newRow(keyInfo.fileInfo.fileName().toLatin1()) << keyInfo.fileInfo.absoluteFilePath() << keyInfo.algorithm << keyInfo.type << keyInfo.length << keyInfo.format; @@ -186,7 +192,10 @@ void tst_QSslKey::constructor() QFETCH(QSsl::EncodingFormat, format); QByteArray encoded = readFile(absFilePath); - QSslKey key(encoded, algorithm, format, type); + QByteArray passphrase; + if (QByteArray(QTest::currentDataTag()).contains("-pkcs8-")) + passphrase = QByteArray("1234"); + QSslKey key(encoded, algorithm, format, type, passphrase); QVERIFY(!key.isNull()); } @@ -215,9 +224,12 @@ void tst_QSslKey::constructorHandle() ? q_PEM_read_bio_PUBKEY : q_PEM_read_bio_PrivateKey); + QByteArray passphrase; + if (QByteArray(QTest::currentDataTag()).contains("-pkcs8-")) + passphrase = "1234"; BIO* bio = q_BIO_new(q_BIO_s_mem()); q_BIO_write(bio, pem.constData(), pem.length()); - QSslKey key(func(bio, nullptr, nullptr, nullptr), type); + QSslKey key(func(bio, nullptr, nullptr, static_cast(passphrase.data())), type); q_BIO_free(bio); QVERIFY(!key.isNull()); @@ -245,7 +257,10 @@ void tst_QSslKey::copyAndAssign() QFETCH(QSsl::EncodingFormat, format); QByteArray encoded = readFile(absFilePath); - QSslKey key(encoded, algorithm, format, type); + QByteArray passphrase; + if (QByteArray(QTest::currentDataTag()).contains("-pkcs8-")) + passphrase = QByteArray("1234"); + QSslKey key(encoded, algorithm, format, type, passphrase); QSslKey copied(key); QCOMPARE(key, copied); @@ -286,7 +301,10 @@ void tst_QSslKey::length() QFETCH(QSsl::EncodingFormat, format); QByteArray encoded = readFile(absFilePath); - QSslKey key(encoded, algorithm, format, type); + QByteArray passphrase; + if (QByteArray(QTest::currentDataTag()).contains("-pkcs8-")) + passphrase = QByteArray("1234"); + QSslKey key(encoded, algorithm, format, type, passphrase); QVERIFY(!key.isNull()); QCOMPARE(key.length(), length); } @@ -306,6 +324,13 @@ void tst_QSslKey::toPemOrDer() QFETCH(QSsl::KeyType, type); QFETCH(QSsl::EncodingFormat, format); + if (QByteArray(QTest::currentDataTag()).contains("-pkcs8-")) // these are encrypted + QSKIP("Encrypted PKCS#8 keys gets decrypted when loaded. So we can't compare it to the encrypted version."); +#ifndef QT_NO_OPENSSL + if (QByteArray(QTest::currentDataTag()).contains("pkcs8")) + QSKIP("OpenSSL converts PKCS#8 keys to other formats, invalidating comparisons."); +#endif // openssl + QByteArray encoded = readFile(absFilePath); QSslKey key(encoded, algorithm, format, type); QVERIFY(!key.isNull()); @@ -326,6 +351,8 @@ void tst_QSslKey::toEncryptedPemOrDer_data() passwords << " " << "foobar" << "foo bar" << "aAzZ`1234567890-=~!@#$%^&*()_+[]{}\\|;:'\",.<>/?"; // ### add more (?) foreach (KeyInfo keyInfo, keyInfoList) { + if (keyInfo.fileInfo.fileName().contains("pkcs8")) + continue; // pkcs8 keys are encrypted in a different way than the other keys foreach (QString password, passwords) { const QByteArray testName = keyInfo.fileInfo.fileName().toLatin1() + '-' + (keyInfo.algorithm == QSsl::Rsa ? "RSA" : -- cgit v1.2.3