Do not try to fallback to "less secure" protocols
ClosedPublic

Authored by aacid on Oct 20 2018, 9:30 PM.

Details

Summary

Both Firefox and Chrom[e|ium] do this for a while so most
sites have already make sure they are better compliant
than when we introduced this workaround

Test Plan

Contact a few https sites, still works

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
aacid created this revision.Oct 20 2018, 9:30 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 20 2018, 9:30 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
aacid requested review of this revision.Oct 20 2018, 9:30 PM
aacid added a comment.Oct 26 2018, 2:58 PM

I'll commit this next tuesday unless someone disagrees.

What protocol does KTcpSocket::SecureProtocols implement (I can't guess it)? If it is the same as QSsl:SecureProtocols
it does:
On the client side, this will send a TLS 1.0 Client Hello, enabling TLSv1_0 and SSLv3 connections. On the server side, this will enable both SSLv3 and TLSv1_0 connections.

Shouldn't it try with TLS 1.3 when available and fall back to TLS 1.2, but not lower (for security reason)?

Line 89 in https://api.kde.org/frameworks/kio/html/ktcpsocket_8cpp_source.html suggests that it is the same as QSsl:SecureProtocols.

Can you confirm that it works with TLSv1.2 only sites? (e.g. https://stikonas.eu:5281/admin/). Ideally we should test with TLSv1.3 too.

What protocol does KTcpSocket::SecureProtocols implement (I can't guess it)? If it is the same as QSsl:SecureProtocols

Yes, see ./src/core/ktcpsocket.cpp:89:

it does:
On the client side, this will send a TLS 1.0 Client Hello, enabling TLSv1_0 and SSLv3 connections. On the server side, this will enable both SSLv3 and TLSv1_0 connections.

Shouldn't it try with TLS 1.3 when available and fall back to TLS 1.2, but not lower (for security reason)?

My opinion is that we should not try to be smarter than Qt here, that involves:

  • Trusting them on what "SecureProtocols" means (according to qsslsocket_openssl.cpp i'd say that SecureProtocols is TlsV1_0OrLater, which makes sense to me)
  • Don't do fallbacks "client side" and let openssl do the actual protocol negotiation by itself

Besides that, AFAICS from my about:config settings in Firefox, the min version it supports is 1 that according to https://support.mozilla.org/en-US/questions/1101896 it's TLS 1.0, so supporting less than that seems to aggressive to me at this point, yes there may be bugs, but we also need to let people use the web.

Can you confirm that it works with TLSv1.2 only sites? (e.g. https://stikonas.eu:5281/admin/). Ideally we should test with TLSv1.3 too.

Yes, it works. And https://tls13.crypto.mozilla.org/ works too (on Konqueror using KHTML, sadly not on falkon because i guess it doesn't use the kioslave infrastructure, but that's a different story i guess)

stikonas accepted this revision.Oct 26 2018, 11:06 PM

Can you confirm that it works with TLSv1.2 only sites? (e.g. https://stikonas.eu:5281/admin/). Ideally we should test with TLSv1.3 too.

Yes, it works. And https://tls13.crypto.mozilla.org/ works too (on Konqueror using KHTML, sadly not on falkon because i guess it doesn't use the kioslave infrastructure, but that's a different story i guess)

Thanks for testing. Yes, we definitely want to support TLS 1.0.

Is it just me or that Qt description of their QSsl enums is a bit confusing. Presumably QSsl::SecureProtocols mean works with TLS 1.0 or later now but can be restricted to higher protocols in the future if TLS 1.0 is found to be insecure. But that's not what description of QSsl::SecureProtocols says. They refer to TlsV1Ssl3 which I guess is misspelled TlsV1SslV3...

This revision is now accepted and ready to land.Oct 26 2018, 11:06 PM
This revision was automatically updated to reflect the committed changes.