From dc707f0855a7b8a5c98456d4b8cd65d1a7ea3514 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 11 Feb 2019 16:44:14 +0100 Subject: Make client certificate store thread safe If the users manipulate the API from the UI, we need to read from the UI thread as well. Change-Id: I8af787a357954cff4fbdd94bcf27b880fb6aecb4 Reviewed-by: Michal Klocek --- src/core/api/qwebengineclientcertificatestore.cpp | 20 +++------ src/core/api/qwebengineclientcertificatestore.h | 2 +- src/core/net/client_cert_override.cpp | 49 ++++++++++++++++++----- src/core/net/client_cert_override.h | 4 ++ src/core/net/client_cert_store_data.cpp | 21 ++++++++-- src/core/net/client_cert_store_data.h | 8 ++-- 6 files changed, 70 insertions(+), 34 deletions(-) (limited to 'src') diff --git a/src/core/api/qwebengineclientcertificatestore.cpp b/src/core/api/qwebengineclientcertificatestore.cpp index 850dd16d7..40412dfd8 100644 --- a/src/core/api/qwebengineclientcertificatestore.cpp +++ b/src/core/api/qwebengineclientcertificatestore.cpp @@ -112,7 +112,7 @@ void QWebEngineClientCertificateStore::add(const QSslCertificate &certificate, c QList QWebEngineClientCertificateStore::toList() const { QList certificateList; - for (auto data : qAsConst(d_ptr->addedCerts)) { + for (auto data : qAsConst(d_ptr->extraCerts)) { Entry entry; entry.certificate = data->certificate; entry.privateKey = data->key; @@ -123,21 +123,12 @@ QList QWebEngineClientCertificateStore: /*! Deletes all the instances of the client certificate in the in-memory client certificate store - that matches the certificate in the \a entry. + that matches the certificate \a certificate. */ -void QWebEngineClientCertificateStore::remove(Entry entry) +void QWebEngineClientCertificateStore::remove(const QSslCertificate &certificate) { - auto it = d_ptr->addedCerts.begin(); - while (it != d_ptr->addedCerts.end()) { - auto *overrideData = *it; - if (entry.certificate.toDer() == overrideData->certificate.toDer()) { - d_ptr->deletedCerts.append(overrideData); - it = d_ptr->addedCerts.erase(it); - continue; - } - ++it; - } + d_ptr->remove(certificate); } /*! @@ -146,8 +137,7 @@ void QWebEngineClientCertificateStore::remove(Entry entry) void QWebEngineClientCertificateStore::clear() { - d_ptr->deletedCerts.append(d_ptr->addedCerts); - d_ptr->addedCerts.clear(); + d_ptr->clear(); } #endif // QT_CONFIG(ssl) diff --git a/src/core/api/qwebengineclientcertificatestore.h b/src/core/api/qwebengineclientcertificatestore.h index c0bd66e2b..0000299a2 100644 --- a/src/core/api/qwebengineclientcertificatestore.h +++ b/src/core/api/qwebengineclientcertificatestore.h @@ -67,7 +67,7 @@ public: static QWebEngineClientCertificateStore *getInstance(); void add(const QSslCertificate &certificate, const QSslKey &privateKey); QList toList() const; - void remove(Entry entry); + void remove(const QSslCertificate &certificate); void clear(); private: diff --git a/src/core/net/client_cert_override.cpp b/src/core/net/client_cert_override.cpp index caf6adad7..cbcbb03b8 100644 --- a/src/core/net/client_cert_override.cpp +++ b/src/core/net/client_cert_override.cpp @@ -43,6 +43,7 @@ #include "base/bind_helpers.h" #include "base/task/post_task.h" #include "base/callback_forward.h" +#include "content/public/browser/browser_task_traits.h" #include "net/ssl/client_cert_store.h" #include "net/ssl/ssl_cert_request_info.h" #include "net/ssl/ssl_private_key.h" @@ -106,32 +107,58 @@ ClientCertOverrideStore::ClientCertOverrideStore() ClientCertOverrideStore::~ClientCertOverrideStore() = default; -void ClientCertOverrideStore::GetClientCerts(const net::SSLCertRequestInfo &cert_request_info, - const ClientCertListCallback &callback) -{ #if QT_CONFIG(ssl) +// static +net::ClientCertIdentityList ClientCertOverrideStore::GetClientCertsOnUIThread(const net::SSLCertRequestInfo &cert_request_info) +{ + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); QWebEngineClientCertificateStore *clientCertificateStore = QWebEngineClientCertificateStore::getInstance(); - const auto &clientCertOverrideData = clientCertificateStore->d_ptr->addedCerts; + const auto &clientCertOverrideData = clientCertificateStore->d_ptr->extraCerts; // Look for certificates in memory store for (int i = 0; i < clientCertOverrideData.length(); i++) { scoped_refptr cert = clientCertOverrideData[i]->certPtr; if (cert != NULL && cert->IsIssuedByEncoded(cert_request_info.cert_authorities)) { net::ClientCertIdentityList selected_identities; selected_identities.push_back(std::make_unique(cert, clientCertOverrideData[i]->keyPtr)); - callback.Run(std::move(selected_identities)); - return; + return selected_identities; } } -#endif // QT_CONFIG(ssl) + return net::ClientCertIdentityList(); +} - // Continue with native cert store if matching certificate is not found in memory - if (m_nativeStore) { +void ClientCertOverrideStore::GetClientCertsReturn(const net::SSLCertRequestInfo &cert_request_info, + const ClientCertListCallback &callback, + net::ClientCertIdentityList &&result) +{ + // Continue with native cert store if matching certificatse were not found in memory + if (result.empty() && m_nativeStore) m_nativeStore->GetClientCerts(cert_request_info, callback); + else + callback.Run(std::move(result)); +} + +#endif // QT_CONFIG(ssl) + +void ClientCertOverrideStore::GetClientCerts(const net::SSLCertRequestInfo &cert_request_info, + const ClientCertListCallback &callback) +{ +#if QT_CONFIG(ssl) + // Access the user-provided data from the UI thread, but return on whatever thread this is. + if (base::PostTaskWithTraitsAndReplyWithResult( + FROM_HERE, { content::BrowserThread::UI }, + base::BindOnce(&GetClientCertsOnUIThread, base::ConstRef(cert_request_info)), + base::BindOnce(&ClientCertOverrideStore::GetClientCertsReturn, + base::Unretained(this), base::ConstRef(cert_request_info), callback)) + ) { return; } +#endif // QT_CONFIG(ssl) - callback.Run(net::ClientCertIdentityList()); - return; + // Continue with native cert store if we failed to post task + if (m_nativeStore) + m_nativeStore->GetClientCerts(cert_request_info, callback); + else + callback.Run(net::ClientCertIdentityList()); } // static diff --git a/src/core/net/client_cert_override.h b/src/core/net/client_cert_override.h index 4f13c3116..04c3ce66d 100644 --- a/src/core/net/client_cert_override.h +++ b/src/core/net/client_cert_override.h @@ -59,6 +59,10 @@ public: const ClientCertListCallback &callback) override; private: static std::unique_ptr createNativeStore(); + static net::ClientCertIdentityList GetClientCertsOnUIThread(const net::SSLCertRequestInfo &request); + void GetClientCertsReturn(const net::SSLCertRequestInfo &cert_request_info, + const ClientCertListCallback &callback, + net::ClientCertIdentityList &&result); std::unique_ptr m_nativeStore; }; diff --git a/src/core/net/client_cert_store_data.cpp b/src/core/net/client_cert_store_data.cpp index ae4deed1c..d1018a063 100644 --- a/src/core/net/client_cert_store_data.cpp +++ b/src/core/net/client_cert_store_data.cpp @@ -138,12 +138,27 @@ void ClientCertificateStoreData::add(const QSslCertificate &certificate, const Q data->certPtr = net::X509Certificate::CreateFromBytes(certInBytes.data(), certInBytes.length()); data->key = privateKey; data->certificate = certificate; - addedCerts.append(data); + extraCerts.append(data); } -ClientCertificateStoreData::~ClientCertificateStoreData() +void ClientCertificateStoreData::remove(const QSslCertificate &certificate) { - qDeleteAll(deletedCerts); + auto it = extraCerts.begin(); + while (it != extraCerts.end()) { + const QtWebEngineCore::ClientCertificateStoreData::Entry *overrideData = *it; + if (certificate.toDer() == overrideData->certificate.toDer()) { + it = extraCerts.erase(it); + delete overrideData; + continue; + } + ++it; + } +} + +void ClientCertificateStoreData::clear() +{ + qDeleteAll(extraCerts); + extraCerts.clear(); } } // namespace QtWebEngineCore diff --git a/src/core/net/client_cert_store_data.h b/src/core/net/client_cert_store_data.h index 41dc1f8ec..7f83f4b60 100644 --- a/src/core/net/client_cert_store_data.h +++ b/src/core/net/client_cert_store_data.h @@ -46,7 +46,7 @@ #if QT_CONFIG(ssl) #include "base/memory/ref_counted.h" -#include +#include #include #include @@ -65,11 +65,11 @@ struct ClientCertificateStoreData { scoped_refptr keyPtr; }; - ~ClientCertificateStoreData(); void add(const QSslCertificate &certificate, const QSslKey &privateKey); + void remove(const QSslCertificate &certificate); + void clear(); - QList addedCerts; - QList deletedCerts; + QVector extraCerts; }; } // namespace QtWebEngineCore -- cgit v1.2.3