[TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket
ClosedPublic

Authored by ahmadsamir on Oct 24 2019, 4:20 PM.

Details

Summary

In setSslMetaData(), it's sufficient to add the cipher name

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.
ahmadsamir created this revision.Oct 24 2019, 4:20 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 24 2019, 4:20 PM
ahmadsamir requested review of this revision.Oct 24 2019, 4:20 PM

Is cipherDigestMethod() (method @ line 96) needed? the cipher name is inserted in sslMetaData a couple of lines after that.

Using some test code:
QList<QSslCipher> list = QSslConfiguration::supportedCiphers();
for (QSslCipher c : list) {

qInfo() << c.name();

}

I get:
"TLS_AES_256_GCM_SHA384"
"TLS_CHACHA20_POLY1305_SHA256"
"TLS_AES_128_GCM_SHA256"
"ECDHE-ECDSA-AES256-GCM-SHA384"
"ECDHE-RSA-AES256-GCM-SHA384"
"DHE-RSA-AES256-GCM-SHA384"
"ECDHE-ECDSA-CHACHA20-POLY1305"
"ECDHE-RSA-CHACHA20-POLY1305"
"DHE-RSA-CHACHA20-POLY1305"
"ECDHE-ECDSA-AES128-GCM-SHA256"
"ECDHE-RSA-AES128-GCM-SHA256"
"DHE-RSA-AES128-GCM-SHA256"
"ECDHE-ECDSA-AES256-SHA384"
"ECDHE-RSA-AES256-SHA384"
"DHE-RSA-AES256-SHA256"
"ECDHE-ECDSA-AES128-SHA256"
"ECDHE-RSA-AES128-SHA256"
"DHE-RSA-AES128-SHA256"
"ECDHE-ECDSA-AES256-SHA"
"ECDHE-RSA-AES256-SHA"
"DHE-RSA-AES256-SHA"
"ECDHE-ECDSA-AES128-SHA"
"ECDHE-RSA-AES128-SHA"
"DHE-RSA-AES128-SHA"
"RSA-PSK-AES256-GCM-SHA384"
"DHE-PSK-AES256-GCM-SHA384"
"RSA-PSK-CHACHA20-POLY1305"
"DHE-PSK-CHACHA20-POLY1305"
"ECDHE-PSK-CHACHA20-POLY1305"
"AES256-GCM-SHA384"
"PSK-AES256-GCM-SHA384"
"PSK-CHACHA20-POLY1305"
"RSA-PSK-AES128-GCM-SHA256"
"DHE-PSK-AES128-GCM-SHA256"
"AES128-GCM-SHA256"
"PSK-AES128-GCM-SHA256"
"AES256-SHA256"
"AES128-SHA256"
"ECDHE-PSK-AES256-CBC-SHA384"
"ECDHE-PSK-AES256-CBC-SHA"
"SRP-RSA-AES-256-CBC-SHA"
"SRP-AES-256-CBC-SHA"
"RSA-PSK-AES256-CBC-SHA384"
"DHE-PSK-AES256-CBC-SHA384"
"RSA-PSK-AES256-CBC-SHA"
"DHE-PSK-AES256-CBC-SHA"
"AES256-SHA"
"PSK-AES256-CBC-SHA384"
"PSK-AES256-CBC-SHA"
"ECDHE-PSK-AES128-CBC-SHA256"
"ECDHE-PSK-AES128-CBC-SHA"
"SRP-RSA-AES-128-CBC-SHA"
"SRP-AES-128-CBC-SHA"
"RSA-PSK-AES128-CBC-SHA256"
"DHE-PSK-AES128-CBC-SHA256"
"RSA-PSK-AES128-CBC-SHA"
"DHE-PSK-AES128-CBC-SHA"
"AES128-SHA"
"PSK-AES128-CBC-SHA256"
"PSK-AES128-CBC-SHA"

In general I think this looks very good already!
The cipher naming stuff looks very fishy though, that's not due to your changes though but coming from old KSslCipher code. I'm wondering where the ssl_cipher meta data is being consumed? If this is purely used for display, maybe let's just replace the entire string in there by QSslCipher::name()?

The cipher naming stuff looks very fishy though, that's not due to your changes though but coming from old KSslCipher code. I'm wondering where the ssl_cipher meta data is being consumed? If this is purely used for display, maybe let's just replace the entire string in there by QSslCipher::name()?

Looking at how ktorrent uses KIO::MetaData: https://lxr.kde.org/source/extragear/network/libktorrent/src/tracker/httptracker.cpp#0480

gave me an idea to search for "ssl_cipher" on lxr.k.o, I found no hits at all. So nothing uses it in KDE, I don't know about 3rd party users. So maybe we just remove the digest method (or the whole sslCipher string), especially since cipher.name() is pretty descriptive.

The cipher naming stuff looks very fishy though, that's not due to your changes though but coming from old KSslCipher code. I'm wondering where the ssl_cipher meta data is being consumed? If this is purely used for display, maybe let's just replace the entire string in there by QSslCipher::name()?

Looking at how ktorrent uses KIO::MetaData: https://lxr.kde.org/source/extragear/network/libktorrent/src/tracker/httptracker.cpp#0480

gave me an idea to search for "ssl_cipher" on lxr.k.o, I found no hits at all. So nothing uses it in KDE, I don't know about 3rd party users. So maybe we just remove the digest method (or the whole sslCipher string), especially since cipher.name() is pretty descriptive.

I did find uses, but they merely pass on the value, or display it in some way, ie. I found nothing that requires a specific format in there. I'd say let's go with QSslCipher::name() here.

So, we ditch digestMethod but keep the rest? ecryptionMethod/authenticationMethod/keyExchangeMethod can be deduced from the name() sometimes, but some others, not so obvious:

Name:  "TLS_AES_256_GCM_SHA384"
Encryption method:  "AESGCM(256)"
Auth method: "any"
Key exchange method:  "any"
Protocol:  "TLSv1.3" 

Name:  "TLS_CHACHA20_POLY1305_SHA256"
Encryption method:  "CHACHA20/POLY1305(256)"
Auth method: "any"
Key exchange method:  "any"
Protocol:  "TLSv1.3" 

Name:  "TLS_AES_128_GCM_SHA256"
Encryption method:  "AESGCM(128)"
Auth method: "any"
Key exchange method:  "any"
Protocol:  "TLSv1.3" 

Name:  "ECDHE-ECDSA-AES256-GCM-SHA384"
Encryption method:  "AESGCM(256)"
Auth method: "ECDSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-RSA-AES256-GCM-SHA384"
Encryption method:  "AESGCM(256)"
Auth method: "RSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "DHE-RSA-AES256-GCM-SHA384"
Encryption method:  "AESGCM(256)"
Auth method: "RSA"
Key exchange method:  "DH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-ECDSA-CHACHA20-POLY1305"
Encryption method:  "CHACHA20/POLY1305(256)"
Auth method: "ECDSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-RSA-CHACHA20-POLY1305"
Encryption method:  "CHACHA20/POLY1305(256)"
Auth method: "RSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "DHE-RSA-CHACHA20-POLY1305"
Encryption method:  "CHACHA20/POLY1305(256)"
Auth method: "RSA"
Key exchange method:  "DH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-ECDSA-AES128-GCM-SHA256"
Encryption method:  "AESGCM(128)"
Auth method: "ECDSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-RSA-AES128-GCM-SHA256"
Encryption method:  "AESGCM(128)"
Auth method: "RSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "DHE-RSA-AES128-GCM-SHA256"
Encryption method:  "AESGCM(128)"
Auth method: "RSA"
Key exchange method:  "DH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-ECDSA-AES256-SHA384"
Encryption method:  "AES(256)"
Auth method: "ECDSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-RSA-AES256-SHA384"
Encryption method:  "AES(256)"
Auth method: "RSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "DHE-RSA-AES256-SHA256"
Encryption method:  "AES(256)"
Auth method: "RSA"
Key exchange method:  "DH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-ECDSA-AES128-SHA256"
Encryption method:  "AES(128)"
Auth method: "ECDSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-RSA-AES128-SHA256"
Encryption method:  "AES(128)"
Auth method: "RSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1.2" 

Name:  "DHE-RSA-AES128-SHA256"
Encryption method:  "AES(128)"
Auth method: "RSA"
Key exchange method:  "DH"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-ECDSA-AES256-SHA"
Encryption method:  "AES(256)"
Auth method: "ECDSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1" 

Name:  "ECDHE-RSA-AES256-SHA"
Encryption method:  "AES(256)"
Auth method: "RSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1" 

Name:  "DHE-RSA-AES256-SHA"
Encryption method:  "AES(256)"
Auth method: "RSA"
Key exchange method:  "DH"
Protocol:  "SSLv3" 

Name:  "ECDHE-ECDSA-AES128-SHA"
Encryption method:  "AES(128)"
Auth method: "ECDSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1" 

Name:  "ECDHE-RSA-AES128-SHA"
Encryption method:  "AES(128)"
Auth method: "RSA"
Key exchange method:  "ECDH"
Protocol:  "TLSv1" 

Name:  "DHE-RSA-AES128-SHA"
Encryption method:  "AES(128)"
Auth method: "RSA"
Key exchange method:  "DH"
Protocol:  "SSLv3" 

Name:  "RSA-PSK-AES256-GCM-SHA384"
Encryption method:  "AESGCM(256)"
Auth method: "RSA"
Key exchange method:  "RSAPSK"
Protocol:  "TLSv1.2" 

Name:  "DHE-PSK-AES256-GCM-SHA384"
Encryption method:  "AESGCM(256)"
Auth method: "PSK"
Key exchange method:  "DHEPSK"
Protocol:  "TLSv1.2" 

Name:  "RSA-PSK-CHACHA20-POLY1305"
Encryption method:  "CHACHA20/POLY1305(256)"
Auth method: "RSA"
Key exchange method:  "RSAPSK"
Protocol:  "TLSv1.2" 

Name:  "DHE-PSK-CHACHA20-POLY1305"
Encryption method:  "CHACHA20/POLY1305(256)"
Auth method: "PSK"
Key exchange method:  "DHEPSK"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-PSK-CHACHA20-POLY1305"
Encryption method:  "CHACHA20/POLY1305(256)"
Auth method: "PSK"
Key exchange method:  "ECDHEPSK"
Protocol:  "TLSv1.2" 

Name:  "AES256-GCM-SHA384"
Encryption method:  "AESGCM(256)"
Auth method: "RSA"
Key exchange method:  "RSA"
Protocol:  "TLSv1.2" 

Name:  "PSK-AES256-GCM-SHA384"
Encryption method:  "AESGCM(256)"
Auth method: "PSK"
Key exchange method:  "PSK"
Protocol:  "TLSv1.2" 

Name:  "PSK-CHACHA20-POLY1305"
Encryption method:  "CHACHA20/POLY1305(256)"
Auth method: "PSK"
Key exchange method:  "PSK"
Protocol:  "TLSv1.2" 

Name:  "RSA-PSK-AES128-GCM-SHA256"
Encryption method:  "AESGCM(128)"
Auth method: "RSA"
Key exchange method:  "RSAPSK"
Protocol:  "TLSv1.2" 

Name:  "DHE-PSK-AES128-GCM-SHA256"
Encryption method:  "AESGCM(128)"
Auth method: "PSK"
Key exchange method:  "DHEPSK"
Protocol:  "TLSv1.2" 

Name:  "AES128-GCM-SHA256"
Encryption method:  "AESGCM(128)"
Auth method: "RSA"
Key exchange method:  "RSA"
Protocol:  "TLSv1.2" 

Name:  "PSK-AES128-GCM-SHA256"
Encryption method:  "AESGCM(128)"
Auth method: "PSK"
Key exchange method:  "PSK"
Protocol:  "TLSv1.2" 

Name:  "AES256-SHA256"
Encryption method:  "AES(256)"
Auth method: "RSA"
Key exchange method:  "RSA"
Protocol:  "TLSv1.2" 

Name:  "AES128-SHA256"
Encryption method:  "AES(128)"
Auth method: "RSA"
Key exchange method:  "RSA"
Protocol:  "TLSv1.2" 

Name:  "ECDHE-PSK-AES256-CBC-SHA384"
Encryption method:  "AES(256)"
Auth method: "PSK"
Key exchange method:  "ECDHEPSK"
Protocol:  "TLSv1" 

Name:  "ECDHE-PSK-AES256-CBC-SHA"
Encryption method:  "AES(256)"
Auth method: "PSK"
Key exchange method:  "ECDHEPSK"
Protocol:  "TLSv1" 

Name:  "SRP-RSA-AES-256-CBC-SHA"
Encryption method:  "AES(256)"
Auth method: "RSA"
Key exchange method:  "SRP"
Protocol:  "SSLv3" 

Name:  "SRP-AES-256-CBC-SHA"
Encryption method:  "AES(256)"
Auth method: "SRP"
Key exchange method:  "SRP"
Protocol:  "SSLv3" 

Name:  "RSA-PSK-AES256-CBC-SHA384"
Encryption method:  "AES(256)"
Auth method: "RSA"
Key exchange method:  "RSAPSK"
Protocol:  "TLSv1" 

Name:  "DHE-PSK-AES256-CBC-SHA384"
Encryption method:  "AES(256)"
Auth method: "PSK"
Key exchange method:  "DHEPSK"
Protocol:  "TLSv1" 

Name:  "RSA-PSK-AES256-CBC-SHA"
Encryption method:  "AES(256)"
Auth method: "RSA"
Key exchange method:  "RSAPSK"
Protocol:  "SSLv3" 

Name:  "DHE-PSK-AES256-CBC-SHA"
Encryption method:  "AES(256)"
Auth method: "PSK"
Key exchange method:  "DHEPSK"
Protocol:  "SSLv3" 

Name:  "AES256-SHA"
Encryption method:  "AES(256)"
Auth method: "RSA"
Key exchange method:  "RSA"
Protocol:  "SSLv3" 

Name:  "PSK-AES256-CBC-SHA384"
Encryption method:  "AES(256)"
Auth method: "PSK"
Key exchange method:  "PSK"
Protocol:  "TLSv1" 

Name:  "PSK-AES256-CBC-SHA"
Encryption method:  "AES(256)"
Auth method: "PSK"
Key exchange method:  "PSK"
Protocol:  "SSLv3" 

Name:  "ECDHE-PSK-AES128-CBC-SHA256"
Encryption method:  "AES(128)"
Auth method: "PSK"
Key exchange method:  "ECDHEPSK"
Protocol:  "TLSv1" 

Name:  "ECDHE-PSK-AES128-CBC-SHA"
Encryption method:  "AES(128)"
Auth method: "PSK"
Key exchange method:  "ECDHEPSK"
Protocol:  "TLSv1" 

Name:  "SRP-RSA-AES-128-CBC-SHA"
Encryption method:  "AES(128)"
Auth method: "RSA"
Key exchange method:  "SRP"
Protocol:  "SSLv3" 

Name:  "SRP-AES-128-CBC-SHA"
Encryption method:  "AES(128)"
Auth method: "SRP"
Key exchange method:  "SRP"
Protocol:  "SSLv3" 

Name:  "RSA-PSK-AES128-CBC-SHA256"
Encryption method:  "AES(128)"
Auth method: "RSA"
Key exchange method:  "RSAPSK"
Protocol:  "TLSv1" 

Name:  "DHE-PSK-AES128-CBC-SHA256"
Encryption method:  "AES(128)"
Auth method: "PSK"
Key exchange method:  "DHEPSK"
Protocol:  "TLSv1" 

Name:  "RSA-PSK-AES128-CBC-SHA"
Encryption method:  "AES(128)"
Auth method: "RSA"
Key exchange method:  "RSAPSK"
Protocol:  "SSLv3" 

Name:  "DHE-PSK-AES128-CBC-SHA"
Encryption method:  "AES(128)"
Auth method: "PSK"
Key exchange method:  "DHEPSK"
Protocol:  "SSLv3" 

Name:  "AES128-SHA"
Encryption method:  "AES(128)"
Auth method: "RSA"
Key exchange method:  "RSA"
Protocol:  "SSLv3" 

Name:  "PSK-AES128-CBC-SHA256"
Encryption method:  "AES(128)"
Auth method: "PSK"
Key exchange method:  "PSK"
Protocol:  "TLSv1" 

Name:  "PSK-AES128-CBC-SHA"
Encryption method:  "AES(128)"
Auth method: "PSK"
Key exchange method:  "PSK"
Protocol:  "SSLv3"

So, we ditch digestMethod but keep the rest? ecryptionMethod/authenticationMethod/keyExchangeMethod can be deduced from the name() sometimes, but some others, not so obvious:

I was thinking of just:

sslMetaData.insert(QStringLiteral("ssl_cipher"), cipher.name());

It's the same as ssl_cipher_name then, ie. we'd be setting both for compatibility. Not every detail might be obviously readable out of this, but neither is that possible from the old format which adds no lables, just abbreviations separated by a line break.

ahmadsamir updated this revision to Diff 68834.Oct 27 2019, 2:58 PM

cipher.name() is deemed sufficient when setting the ssl MetaData

ahmadsamir updated this revision to Diff 68836.Oct 27 2019, 3:17 PM
ahmadsamir edited the summary of this revision. (Show Details)

Verbatim

So, we ditch digestMethod but keep the rest? ecryptionMethod/authenticationMethod/keyExchangeMethod can be deduced from the name() sometimes, but some others, not so obvious:

I was thinking of just:

sslMetaData.insert(QStringLiteral("ssl_cipher"), cipher.name());

It's the same as ssl_cipher_name then, ie. we'd be setting both for compatibility. Not every detail might be obviously readable out of this, but neither is that possible from the old format which adds no lables, just abbreviations separated by a line break.

That makes sense.

vkrause accepted this revision.Nov 10 2019, 9:30 AM

Let's get this in now that 5.64 is out.

This revision is now accepted and ready to land.Nov 10 2019, 9:30 AM
This revision was automatically updated to reflect the committed changes.