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

Bug#914212: marked as done ([libkio5] please remove insecure TLS version fall-back mechanism)

Your message dated Sun, 25 Aug 2019 15:34:27 +0000
with message-id <E1i1uXD-000HxJ-7D@fasolo.debian.org>
and subject line Bug#935666: Removed package(s) from unstable
has caused the Debian Bug report #914212,
regarding [libkio5] please remove insecure TLS version fall-back mechanism
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
914212: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=914212
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
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.


--- End Message ---
--- Begin Message --- 
Version: 4:4.14.38-4+rm

Dear submitter,

as the package kde4libs has just been removed from the Debian archive
unstable we hereby close the associated bug reports.  We are sorry
that we couldn't deal with your issue properly.

For details on the removal, please see https://bugs.debian.org/935666

The version of this package that was in Debian prior to this removal
can still be found using http://snapshot.debian.org/.

This message was generated automatically; if you believe that there is
a problem with it please contact the archive administrators by mailing
ftpmaster@ftp-master.debian.org.

Debian distribution maintenance software
pp.
Scott Kitterman (the ftpmaster behind the curtain)

--- End Message ---
Reply to: