[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#914212: [libkio5] please remove insecure TLS version fall-back mechanism



Package: libkio5
Version: 4:4.14.26-2
Severity: important
Tags: patch

Hi,

Until recently KDE kio had a custom TLS version fall-back mechanism which made 
it possible to downgrade a TLS connection to TLSv1.0 even if the server and 
client support a higher TLS version. This has been fixed upstream in [1], [2] 
and is also included in KDE Frameworks 5.52.0 [3].

I backported the patch from [2] for the kio code in kde4libs. Please also 
consider fixing this in stretch.

[1] https://phabricator.kde.org/D16344
[2] https://cgit.kde.org/kio.git/commit/src/core/tcpslavebase.cpp?id=e11d4d18f66ad1c6927b058be84e11d46d9de55a
[3] https://www.kde.org/announcements/kde-frameworks-5.52.0.php

Thanks for your work on Debian!
backport https://cgit.kde.org/kio.git/commit/src/core/tcpslavebase.cpp?id=e11d4d18f66ad1c6927b058be84e11d46d9de55a
to stretch.
--- a/kio/kio/tcpslavebase.cpp
+++ b/kio/kio/tcpslavebase.cpp
@@ -349,106 +349,50 @@
         }
     }
 
-    /*
-      By default the SSL handshake attempt uses these settings in the order shown:
-
-      1.) Protocol: KTcpSocket::SecureProtocols   SSL compression: OFF (DEFAULT)
-      2.) Protocol: KTcpSocket::TlsV1             SSL compression: OFF
-      3.) Protocol: KTcpSocket::SslV3             SSL compression: OFF
-
-      If any combination other than the one marked DEFAULT is used to complete
-      the SSL handshake, then that combination will be cached using KIO's internal
-      meta-data mechanism in order to speed up future connections to the same host.
-    */
-
     QSslConfiguration sslConfig = d->socket.sslConfiguration();
+    const int timeout = (connectTimeout() * 1000); // 20 sec timeout value
 
-#if QT_VERSION >= 0x040800
-    // NOTE: Due to 'CRIME' SSL attacks, compression is always disabled.
-    sslConfig.setSslOption(QSsl::SslOptionDisableCompression, true);
-#endif
-    
-    const int lastSslVerson = config()->readEntry("LastUsedSslVersion", static_cast<int>(KTcpSocket::SecureProtocols));
-    KTcpSocket::SslVersion trySslVersion = static_cast<KTcpSocket::SslVersion>(lastSslVerson);
-    KTcpSocket::SslVersions alreadyTriedSslVersions = trySslVersion;
+    disconnectFromHost();  //Reset some state, even if we are already disconnected
+    d->host = host;
 
-    const int timeout = (connectTimeout() * 1000); // 20 sec timeout value
-    while (true) {
-        disconnectFromHost();  //Reset some state, even if we are already disconnected
-        d->host = host;
-
-        d->socket.connectToHost(host, port);
-        const bool connectOk = d->socket.waitForConnected(timeout > -1 ? timeout : -1);
-
-        kDebug(7027) << "Socket: state=" << d->socket.state()
-                     << ", error=" << d->socket.error()
-                     << ", connected?" << connectOk;
+    d->socket.connectToHost(host, port);
+    const bool connectOk = d->socket.waitForConnected(timeout > -1 ? timeout : -1);
 
-        if (d->socket.state() != KTcpSocket::ConnectedState) {
-            if (errorString)
-                *errorString = host + QLatin1String(": ") + d->socket.errorString();
-            switch (d->socket.error()) {
-            case KTcpSocket::UnsupportedSocketOperationError:
-                return ERR_UNSUPPORTED_ACTION;
-            case KTcpSocket::RemoteHostClosedError:
-                return ERR_CONNECTION_BROKEN;
-            case KTcpSocket::SocketTimeoutError:
-                return ERR_SERVER_TIMEOUT;
-            case KTcpSocket::HostNotFoundError:
-                return ERR_UNKNOWN_HOST;
-            default:
-                return ERR_COULD_NOT_CONNECT;
-            }
+    kDebug(7027) << "Socket: state=" << d->socket.state()
+                 << ", error=" << d->socket.error()
+                 << ", connected?" << connectOk;
+
+    if (d->socket.state() != KTcpSocket::ConnectedState) {
+        if (errorString)
+            *errorString = host + QLatin1String(": ") + d->socket.errorString();
+        switch (d->socket.error()) {
+        case KTcpSocket::UnsupportedSocketOperationError:
+            return ERR_UNSUPPORTED_ACTION;
+        case KTcpSocket::RemoteHostClosedError:
+            return ERR_CONNECTION_BROKEN;
+        case KTcpSocket::SocketTimeoutError:
+            return ERR_SERVER_TIMEOUT;
+        case KTcpSocket::HostNotFoundError:
+            return ERR_UNKNOWN_HOST;
+        default:
+            return ERR_COULD_NOT_CONNECT;
         }
+    }
 
-        //### check for proxyAuthenticationRequiredError
+    //### check for proxyAuthenticationRequiredError
 
-        d->ip = d->socket.peerAddress().toString();
-        d->port = d->socket.peerPort();
+    d->ip = d->socket.peerAddress().toString();
+    d->port = d->socket.peerPort();
 
-        if (d->autoSSL) {
-            SslResult res = d->startTLSInternal(trySslVersion, sslConfig, timeout);
-            if ((res & ResultFailed) && (res & ResultFailedEarly)) {
-                if (!(alreadyTriedSslVersions & KTcpSocket::SecureProtocols)) {
-                    trySslVersion = KTcpSocket::SecureProtocols;
-                    alreadyTriedSslVersions |= trySslVersion;
-                    continue;
-                }
-
-                if (!(alreadyTriedSslVersions & KTcpSocket::TlsV1)) {
-                    trySslVersion = KTcpSocket::TlsV1;
-                    alreadyTriedSslVersions |= trySslVersion;
-                    continue;
-                }
-
-                if (!(alreadyTriedSslVersions & KTcpSocket::SslV3)) {
-                    trySslVersion = KTcpSocket::SslV3;
-                    alreadyTriedSslVersions |= trySslVersion;
-                    continue;
-                }
-            }
-
-            //### SSL 2.0 is (close to) dead and it's a good thing, too.
-            if (res & ResultFailed) {
-                if (errorString)
-                    *errorString = i18nc("%1 is a host name", "%1: SSL negotiation failed", host);
-                return ERR_COULD_NOT_CONNECT;
-            }
-        }
-        // If the SSL handshake was done with anything protocol other than the default,
-        // save that information so that any subsequent requests do not have to do thesame thing.
-        if (trySslVersion != KTcpSocket::SecureProtocols && lastSslVerson == KTcpSocket::SecureProtocols) {
-            setMetaData(QLatin1String("{internal~currenthost}LastUsedSslVersion"),
-                        QString::number(trySslVersion));
+    if (d->autoSSL) {
+        SslResult res = d->startTLSInternal(KTcpSocket::SecureProtocols, sslConfig, timeout);
+
+        if (res & ResultFailed) {
+            if (errorString)
+                *errorString = i18nc("%1 is a host name", "%1: SSL negotiation failed", host);
+            return ERR_COULD_NOT_CONNECT;
         }
-        return 0;
     }
-    Q_ASSERT(false);
-    // Code flow never gets here but let's make the compiler happy.
-    // More: the stack allocation of QSslSettings seems to be confusing the compiler;
-    //       in fact, any non-POD allocation does. 
-    //       even a 'return 0;' directly after the allocation (so before the while(true))
-    //       is ignored. definitely seems to be a compiler bug? - aseigo
     return 0;
 }
 

Attachment: signature.asc
Description: This is a digitally signed message part.


Reply to: