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

Bug#914211: marked as done ([src:kio] please remove insecure TLS version fall-back mechanism)

Your message dated Sat, 19 Jan 2019 08:15:20 +0100
with message-id <13703784.39V7omvgxf@pendragon>
and subject line Re: Processed: fixed in 5.54.1-1
has caused the Debian Bug report #914211,
regarding [src:kio] 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.)


-- 
914211: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=914211
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: src:kio
Version: 5.49.0-1
Severity: important
Tags: patch
Control: found -1 5.28.0-2

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].

Please consider backporting the patch from [2] or shipping a newer KDE 
Frameworks version, so this fix can be included in buster.

Attached you also find a backported version of [2] for the version in stretch. 
Please consider also 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/src/core/tcpslavebase.cpp
+++ b/src/core/tcpslavebase.cpp
@@ -335,111 +335,51 @@
         }
     }
 
-    /*
-       SSL handshake is attempted in the following order:
+    const int timeout = (connectTimeout() * 1000); // 20 sec timeout value
 
-       1.) KTcpSocket::SecureProtocols
-       2.) KTcpSocket::TlsV1_2
-       3.) KTcpSocket::TlsV1_1
-       4.) KTcpSocket::TlsV1_0
-
-       Note that we indivially attempt connection with each TLS version
-       because some sites don't support SSL negotiation. #275524
-
-       The version used to successfully make encrypted connection with the
-       remote server is cached within the process to make subsequent
-       connection requests to the same server faster.
-     */
-
-    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);
-
-        /*qDebug() << "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_CANNOT_CONNECT;
-            }
+    /*qDebug() << "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_CANNOT_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, timeout);
-            if ((res & ResultFailed) && (res & ResultFailedEarly)) {
-                if (!(alreadyTriedSslVersions & KTcpSocket::SecureProtocols)) {
-                    trySslVersion = KTcpSocket::SecureProtocols;
-                    alreadyTriedSslVersions |= trySslVersion;
-                    continue;
-                }
-
-                if (!(alreadyTriedSslVersions & KTcpSocket::TlsV1_2)) {
-                    trySslVersion = KTcpSocket::TlsV1;
-                    alreadyTriedSslVersions |= trySslVersion;
-                    continue;
-                }
-
-                if (!(alreadyTriedSslVersions & KTcpSocket::TlsV1_1)) {
-                    trySslVersion = KTcpSocket::TlsV1;
-                    alreadyTriedSslVersions |= trySslVersion;
-                    continue;
-                }
-
-                if (!(alreadyTriedSslVersions & KTcpSocket::TlsV1_0)) {
-                    trySslVersion = KTcpSocket::TlsV1_0;
-                    alreadyTriedSslVersions |= trySslVersion;
-                    continue;
-                }
-            }
+    if (d->autoSSL) {
+        const SslResult res = d->startTLSInternal(KTcpSocket::SecureProtocols, timeout);
 
-            //### 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_CANNOT_CONNECT;
+        if (res & ResultFailed) {
+            if (errorString) {
+                *errorString = i18nc("%1 is a host name", "%1: SSL negotiation failed", host);
             }
+            return ERR_CANNOT_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(QStringLiteral("{internal~currenthost}LastUsedSslVersion"),
-                        QString::number(trySslVersion));
-        }
-        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 --- 
In data sabato 19 gennaio 2019 01:12:38 CET, Debian Bug Tracking System ha scritto:
> Processing commands for control@bugs.debian.org:
> 
> > fixed 914211 5.54.1-1
> Bug #914211 [src:kio] [src:kio] please remove insecure TLS version fall-back mechanism
> Marked as fixed in versions kio/5.54.1-1.
> > thanks
> Stopping processing here.

Closing the bug then.

Maximilian, please follow the right procedure for closing bugs:
https://www.debian.org/Bugs/Developer.en.html#closing

Thanks,
-- 
Pino Toscano

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


--- End Message ---
Reply to: