diff options
author | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2019-06-17 14:25:09 +0200 |
---|---|---|
committer | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2019-06-17 14:55:55 +0200 |
commit | cdfe8c76af1518ccdd497c20ecc189365c733879 (patch) | |
tree | 59d3af5d55c9384079214870fc661c9bd8d37d3d | |
parent | 4ad915425d804df798e12031c9b2fc13534dcb97 (diff) |
checkOcspResponse - remove unneeded locking
and also plain wrong comments: since we don't set verification callback
on a store - we don't have to lock (our q_X509Callback never gets called).
This change should simplify the merge with change I have in 5.12 (where
I completely got rid of locking). Since I don't care about exact errors
found (relying on the fact it's the same chain of certs we check in
SSL_connect/SSL_accept), for now we don't try to extract them from
OCSP_basic_verify. In fufure, if these chains are different, we
can create a temporary store (see how it's done in 'verify', for example)
and set a VF callback on this store.
Change-Id: I4a36e19836d19c2ea95c869dcfe85f49fe723ff0
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
-rw-r--r-- | src/network/ssl/qsslsocket_openssl.cpp | 16 |
1 files changed, 2 insertions, 14 deletions
diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp index 977d8a6742..7c04feb5f8 100644 --- a/src/network/ssl/qsslsocket_openssl.cpp +++ b/src/network/ssl/qsslsocket_openssl.cpp @@ -1575,27 +1575,15 @@ bool QSslSocketBackendPrivate::checkOcspStatus() // 3) It checks CertID in response. // 4) Ensures the responder is authorized to sign the status respond. // - // Here it's important to notice that it calls X509_cert_verify and - // as a result, possibly, our verification callback. Given this callback - // at the moment uses a global variable, we have to lock. This will change - // as soon as we fix our verification procedure. - // Also note, OpenSSL prior to 1.0.2b would only use bs->certs to + // Note, OpenSSL prior to 1.0.2b would only use bs->certs to // verify the responder's chain (see their commit 4ba9a4265bd). // Working this around - is too much fuss for ancient versions we // are dropping quite soon anyway. { const unsigned long verificationFlags = 0; - const QMutexLocker locker(&_q_sslErrorList()->mutex); - // Before unlocking the mutex, startHandshake() stores errors (found in SSL_connect() - // or SSL_accept()) into the local variable, so it's safe to clear it here - as soon - // as we managed to lock, whoever had the lock before, already stored their own copy - // of errors. - _q_sslErrorList()->errors.clear(); const int success = q_OCSP_basic_verify(basicResponse, peerChain, store, verificationFlags); - if (success <= 0 || _q_sslErrorList()->errors.size()) { - _q_sslErrorList()->errors.clear(); + if (success <= 0) ocspErrors.push_back(QSslError::OcspResponseCannotBeTrusted); - } } if (q_OCSP_resp_count(basicResponse) != 1) { |