AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

checkOcspResponse - remove unneeded locking

Browse Source 
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>
This commit is contained in:
Timur Pocheptsov 2019-06-17 14:25:09 +02:00
parent 4ad915425d
commit cdfe8c76af
1 changed files with 2 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/network/ssl/qsslsocket_openssl.cpp
View File

@ -1575,27 +1575,15 @@ bool QSslSocketBackendPrivate::checkOcspStatus()
// 3) It checks CertID in response. // 3) It checks CertID in response.
// 4) Ensures the responder is authorized to sign the status respond. // 4) Ensures the responder is authorized to sign the status respond.
// //
// Here it's important to notice that it calls X509_cert_verify and // Note, OpenSSL prior to 1.0.2b would only use bs->certs to
// 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
// verify the responder's chain (see their commit 4ba9a4265bd). // verify the responder's chain (see their commit 4ba9a4265bd).
// Working this around - is too much fuss for ancient versions we // Working this around - is too much fuss for ancient versions we
// are dropping quite soon anyway. // are dropping quite soon anyway.
{ {
const unsigned long verificationFlags = 0; 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); const int success = q_OCSP_basic_verify(basicResponse, peerChain, store, verificationFlags);
if (success <= 0 || _q_sslErrorList()->errors.size()) { if (success <= 0)
_q_sslErrorList()->errors.clear();
ocspErrors.push_back(QSslError::OcspResponseCannotBeTrusted); ocspErrors.push_back(QSslError::OcspResponseCannotBeTrusted);
}
} }
if (q_OCSP_resp_count(basicResponse) != 1) { if (q_OCSP_resp_count(basicResponse) != 1) {