4 ms·

Also, you're using the "verify" function incorrectly. Take a look at the documentation for that function; it doesn't do what you think it does anyway. So even

Also, you're using the "verify" function incorrectly. Take a look at the documentation for that function; it doesn't do what you think it does anyway. So even on non-Mac platforms, you weren't verifying the peer certificate. I'm going to stop looking at that QSsl API. It looks very poorly designed. http://doc.qt.io/qt-5/qsslcertificate.html#verify http://doc.qt.io/qt-5/qsslcertificate.html#verify

justinclift 9y ago
Ahh cool. The code we've written for this bit so far was just an initial concept implementation. The server side code is taking most of the effort, so we'll return to the client side bits later when we have something more solid (less changing) for it to talk to. :)
justinclift 9y ago
Thinking about this more, that's not clear. When you say it doesn't do what we think it does... what are we misunderstanding? Our desire is to make sure the server we're connecting to is "one of ours". eg to validate the cert provided by the server against a cert chain we've bundled with the client just for this purpose The docs for the function seem to indicate that what it's for. Er... suggestions/assistance (etc) on what we should be doing instead are definitely welcome. :)
- tomcam 9y ago
  > suggestions... are definitely welcome From someone named tartbooger?
  - justinclift 9y ago
    Errr... does someone name make their words invalid for some reason? :)
- tartboogers 9y ago
  So the way you're using it: auto verificationErrors = reply->sslConfiguration().peerCertificate().verify(m_sslConfiguration.caCertificates()); implies that you want to see if "peerCertificate" validates under "m_sslConfiguration.caCertificates()". That's a reasonable (and necessary) thing to do, but the "verify" function doesn't do that. It's a static function, thus it ignores "peerCertificate". The way you're using it, it simply verifies "m_sslConfiguration.caCertificates" using the system's default CA certificates (I think; the docs are somewhat confusing about this). Also, further down in that code, you ignore self-signed errors. Well that pretty much negates the point of checking in the first place doesn't it? Because if someone MITM's your connection with a self-signed certificate, your code will say that's fine. However, according to the docs for "QSslConfiguration::setCaCertificates", those CA certificates are used to verify the peer certificate during the SSL handshake. So do you even need to be doing any kind of verification manually? I'm not a Qt programmer, but it seems to me that since you're using the appropriate "QSslConfiguration::peerVerifyMode", it should verify the server certificate during the handshake using the CA certificates you set (it happens in the Mac code at [1]). Bottom line: your "RemoteDatabase::gotEncrypted" function can simply be eliminated. This would both provide proper peer verification and eliminate your problem on the Mac platform. [1] https://github.com/qt/qtbase/blob/dev/src/network/ssl/qsslsocket_mac.cpp#L1121 https://github.com/qt/qtbase/blob/dev/src/network/ssl/qsslso...
  - justinclift 9y ago
    > Bottom line: your "RemoteDatabase::gotEncrypted" function can simply be eliminated. (and the rest) Awesome! Thank you. :)

justinclift 9y ago

Ahh cool. The code we've written for this bit so far was just an initial concept implementation. The server side code is taking most of the effort, so we'll return to the client side bits later when we have something more solid (less changing) for it to talk to. :)

justinclift 9y ago

Thinking about this more, that's not clear. When you say it doesn't do what we think it does... what are we misunderstanding? Our desire is to make sure the server we're connecting to is "one of ours". eg to validate the cert provided by the server against a cert chain we've bundled with the client just for this purpose The docs for the function seem to indicate that what it's for. Er... suggestions/assistance (etc) on what we should be doing instead are definitely welcome. :)

tomcam 9y ago
> suggestions... are definitely welcome From someone named tartbooger?
- justinclift 9y ago
  Errr... does someone name make their words invalid for some reason? :)
tartboogers 9y ago
So the way you're using it: auto verificationErrors = reply->sslConfiguration().peerCertificate().verify(m_sslConfiguration.caCertificates()); implies that you want to see if "peerCertificate" validates under "m_sslConfiguration.caCertificates()". That's a reasonable (and necessary) thing to do, but the "verify" function doesn't do that. It's a static function, thus it ignores "peerCertificate". The way you're using it, it simply verifies "m_sslConfiguration.caCertificates" using the system's default CA certificates (I think; the docs are somewhat confusing about this). Also, further down in that code, you ignore self-signed errors. Well that pretty much negates the point of checking in the first place doesn't it? Because if someone MITM's your connection with a self-signed certificate, your code will say that's fine. However, according to the docs for "QSslConfiguration::setCaCertificates", those CA certificates are used to verify the peer certificate during the SSL handshake. So do you even need to be doing any kind of verification manually? I'm not a Qt programmer, but it seems to me that since you're using the appropriate "QSslConfiguration::peerVerifyMode", it should verify the server certificate during the handshake using the CA certificates you set (it happens in the Mac code at [1]). Bottom line: your "RemoteDatabase::gotEncrypted" function can simply be eliminated. This would both provide proper peer verification and eliminate your problem on the Mac platform. [1] https://github.com/qt/qtbase/blob/dev/src/network/ssl/qsslsocket_mac.cpp#L1121 https://github.com/qt/qtbase/blob/dev/src/network/ssl/qsslso...
- justinclift 9y ago
  > Bottom line: your "RemoteDatabase::gotEncrypted" function can simply be eliminated. (and the rest) Awesome! Thank you. :)