From 5c19fad8c178b055e8864b2576cfa3cbaa44a19e Mon Sep 17 00:00:00 2001 From: "Richard J. Moore" Date: Fri, 17 Jan 2014 21:23:20 +0000 Subject: [PATCH] Ensure weak ciphers are not part of the default SSL configuration. Any cipher that is < 128 bits is excluded from the default SSL configuration. These ciphers are still included in the list of availableCiphers() and can be used by applications if required. Calling QSslSocket::setDefaultCiphers(QSslSocket::availableCiphers()) will restore the old behavior. Note that in doing so I spotted that calling defaultCiphers() before doing other actions with SSL had an existing bug that I've addressed as part of the change. [ChangeLog][Important Behavior Changes] The default set of ciphers used by QSslSocket has been changed to exclude ciphers that are using key lengths smaller than 128 bits. These ciphers are still available and can be enabled by applications if required. Change-Id: If2241dda67b624e5febf788efa1369f38c6b1dba Reviewed-by: Thiago Macieira --- src/network/ssl/qsslconfiguration.cpp | 2 +- src/network/ssl/qsslsocket.cpp | 1 + src/network/ssl/qsslsocket_openssl.cpp | 6 ++++- .../network/ssl/qsslsocket/tst_qsslsocket.cpp | 22 ++++++++++++++----- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/network/ssl/qsslconfiguration.cpp b/src/network/ssl/qsslconfiguration.cpp index 4aad7c04c5..3d7656262b 100644 --- a/src/network/ssl/qsslconfiguration.cpp +++ b/src/network/ssl/qsslconfiguration.cpp @@ -663,7 +663,7 @@ int QSslConfiguration::sessionTicketLifeTimeHint() const \li protocol SecureProtocols (meaning either TLS 1.0 or SSL 3 will be used) \li the system's default CA certificate list \li the cipher list equal to the list of the SSL libraries' - supported SSL ciphers + supported SSL ciphers that are 128 bits or more \endlist \sa QSslSocket::supportedCiphers(), setDefaultConfiguration() diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp index 38b493a769..0f934fa5d6 100644 --- a/src/network/ssl/qsslsocket.cpp +++ b/src/network/ssl/qsslsocket.cpp @@ -1953,6 +1953,7 @@ void QSslSocketPrivate::init() */ QList QSslSocketPrivate::defaultCiphers() { + QSslSocketPrivate::ensureInitialized(); QMutexLocker locker(&globalData()->mutex); return globalData()->config->ciphers; } diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp index 69b9e53884..ce6b6562b9 100644 --- a/src/network/ssl/qsslsocket_openssl.cpp +++ b/src/network/ssl/qsslsocket_openssl.cpp @@ -596,6 +596,7 @@ void QSslSocketPrivate::resetDefaultCiphers() SSL *mySsl = q_SSL_new(myCtx); QList ciphers; + QList defaultCiphers; STACK_OF(SSL_CIPHER) *supportedCiphers = q_SSL_get_ciphers(mySsl); for (int i = 0; i < q_sk_SSL_CIPHER_num(supportedCiphers); ++i) { @@ -603,8 +604,11 @@ void QSslSocketPrivate::resetDefaultCiphers() if (cipher->valid) { QSslCipher ciph = QSslSocketBackendPrivate::QSslCipher_from_SSL_CIPHER(cipher); if (!ciph.isNull()) { + // Unconditionally exclude ADH ciphers since they offer no MITM protection if (!ciph.name().toLower().startsWith(QLatin1String("adh"))) ciphers << ciph; + if (ciph.usedBits() >= 128) + defaultCiphers << ciph; } } } @@ -614,7 +618,7 @@ void QSslSocketPrivate::resetDefaultCiphers() q_SSL_free(mySsl); setDefaultSupportedCiphers(ciphers); - setDefaultCiphers(ciphers); + setDefaultCiphers(defaultCiphers); } QList QSslSocketPrivate::systemCaCertificates() diff --git a/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp b/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp index 82543fbc91..3162165139 100644 --- a/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp +++ b/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp @@ -589,13 +589,13 @@ void tst_QSslSocket::ciphers() return; QSslSocket socket; - QCOMPARE(socket.ciphers(), QSslSocket::supportedCiphers()); + QCOMPARE(socket.ciphers(), QSslSocket::defaultCiphers()); socket.setCiphers(QList()); QVERIFY(socket.ciphers().isEmpty()); socket.setCiphers(socket.defaultCiphers()); - QCOMPARE(socket.ciphers(), QSslSocket::supportedCiphers()); + QCOMPARE(socket.ciphers(), QSslSocket::defaultCiphers()); socket.setCiphers(socket.defaultCiphers()); - QCOMPARE(socket.ciphers(), QSslSocket::supportedCiphers()); + QCOMPARE(socket.ciphers(), QSslSocket::defaultCiphers()); // Task 164356 socket.setCiphers("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH"); @@ -678,6 +678,11 @@ void tst_QSslSocket::sessionCipher() if (!socket->waitForEncrypted(5000)) QSKIP("Skipping flaky test - See QTBUG-29941"); QVERIFY(!socket->sessionCipher().isNull()); + + qDebug() << "Supported Ciphers:" << QSslSocket::supportedCiphers(); + qDebug() << "Default Ciphers:" << QSslSocket::defaultCiphers(); + qDebug() << "Session Cipher:" << socket->sessionCipher(); + QVERIFY(QSslSocket::supportedCiphers().contains(socket->sessionCipher())); socket->disconnectFromHost(); QVERIFY(socket->waitForDisconnected()); @@ -1386,6 +1391,15 @@ void tst_QSslSocket::defaultCaCertificates() void tst_QSslSocket::defaultCiphers() { + if (!QSslSocket::supportsSsl()) + return; + + QList ciphers = QSslSocket::defaultCiphers(); + QVERIFY(ciphers.size() > 1); + + QSslSocket socket; + QCOMPARE(socket.defaultCiphers(), ciphers); + QCOMPARE(socket.ciphers(), ciphers); } void tst_QSslSocket::resetDefaultCiphers() @@ -1410,8 +1424,6 @@ void tst_QSslSocket::supportedCiphers() QSslSocket socket; QCOMPARE(socket.supportedCiphers(), ciphers); - QCOMPARE(socket.defaultCiphers(), ciphers); - QCOMPARE(socket.ciphers(), ciphers); } void tst_QSslSocket::systemCaCertificates()