D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:14c6c0007c42: [TcpSlaveBase] port from KTcpSocket 
(deprecated) to QSslSocket (authored by ahmadsamir).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24928?vs=68836&id=69515

REVISION DETAIL
  https://phabricator.kde.org/D24928

AFFECTED FILES
  src/core/tcpslavebase.cpp

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Volker Krause]

vkrause accepted this revision.
vkrause added a comment.
This revision is now accepted and ready to land.


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

REPOSITORY
  R241 KIO

BRANCH
  ahmad/tcpslavebase (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24928

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

ahmadsamir added a comment.


  In D24928#554731 , @vkrause wrote:
  
  > In D24928#554720 , @ahmadsamir 
wrote:
  >
  > > 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.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24928

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

ahmadsamir updated this revision to Diff 68836.
ahmadsamir edited the summary of this revision.
ahmadsamir added a comment.


  Verbatim

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24928?vs=68834&id=68836

BRANCH
  ahmad/tcpslavebase (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24928

AFFECTED FILES
  src/core/tcpslavebase.cpp

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

ahmadsamir updated this revision to Diff 68834.
ahmadsamir added a comment.


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

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24928?vs=68701&id=68834

BRANCH
  ahmad/tcpslavebase (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24928

AFFECTED FILES
  src/core/tcpslavebase.cpp

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Volker Krause]

vkrause added a comment.


  In D24928#554720 , @ahmadsamir 
wrote:
  
  > 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.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24928

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

ahmadsamir added a comment.


  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

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Volker Krause]

vkrause added a comment.


  In D24928#554514 , @ahmadsamir 
wrote:
  
  > In D24928#553952 , @vkrause 
wrote:
  >
  > > 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.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24928

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

ahmadsamir added a comment.


  In D24928#553952 , @vkrause wrote:
  
  > 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.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24928

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Volker Krause]

vkrause added a comment.


  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()?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24928

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

ahmadsamir added a comment.


  Is cipherDigestMethod() (method @ line 96) needed? the cipher name is 
inserted in sslMetaData a couple of lines after that.
  
  Using some test code:
  QList 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"

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24928

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

ahmadsamir added a dependent revision: D24929: [pop3 kioslave] Port KTcpSocket 
(deprecated) to QSslSocket.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24928

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

ahmadsamir added a dependent revision: D24873: [http kio slave] use QSslSocket 
instead of KTcpSocket (deprecated).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24928

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: dfaure, vkrause.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  Also add a method similar to KSslCipher::digestMethod() to TcpSlaveBase.

REPOSITORY
  R241 KIO

BRANCH
  ahmad/tcpslavebase (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24928

AFFECTED FILES
  src/core/tcpslavebase.cpp

To: ahmadsamir, dfaure, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns