From cdfe8c76af1518ccdd497c20ecc189365c733879 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Mon, 17 Jun 2019 14:25:09 +0200 Subject: 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 --- src/network/ssl/qsslsocket_openssl.cpp | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) (limited to 'src/network') 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) { -- cgit v1.2.3