summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAllan Sandfeld Jensen <allan.jensen@qt.io>2019-02-11 16:44:14 +0100
committerAllan Sandfeld Jensen <allan.jensen@qt.io>2019-02-25 14:08:39 +0000
commitdc707f0855a7b8a5c98456d4b8cd65d1a7ea3514 (patch)
treeb36c91f37f3345f16168b3104d1b841f6a6b22ef
parent678cb710bb07188454b19a70b5e5595b8ea41c2a (diff)
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 <michal.klocek@qt.io>
-rw-r--r--src/core/api/qwebengineclientcertificatestore.cpp20
-rw-r--r--src/core/api/qwebengineclientcertificatestore.h2
-rw-r--r--src/core/net/client_cert_override.cpp49
-rw-r--r--src/core/net/client_cert_override.h4
-rw-r--r--src/core/net/client_cert_store_data.cpp21
-rw-r--r--src/core/net/client_cert_store_data.h8
-rw-r--r--tests/auto/core/qwebengineclientcertificatestore/tst_qwebengineclientcertificatestore.cpp2
7 files changed, 71 insertions, 35 deletions
diff --git a/src/core/api/qwebengineclientcertificatestore.cpp b/src/core/api/qwebengineclientcertificatestore.cpp
index 850dd16d..40412dfd 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::Entry> QWebEngineClientCertificateStore::toList() const
{
QList<Entry> 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::Entry> 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 c0bd66e2..0000299a 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<Entry> 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 caf6adad..cbcbb03b 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<net::X509Certificate> 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<ClientCertIdentityOverride>(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 4f13c311..04c3ce66 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<net::ClientCertStore> 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<net::ClientCertStore> m_nativeStore;
};
diff --git a/src/core/net/client_cert_store_data.cpp b/src/core/net/client_cert_store_data.cpp
index ae4deed1..d1018a06 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 41dc1f8e..7f83f4b6 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 <QtCore/qlist.h>
+#include <QtCore/qvector.h>
#include <QtNetwork/qsslcertificate.h>
#include <QtNetwork/qsslkey.h>
@@ -65,11 +65,11 @@ struct ClientCertificateStoreData {
scoped_refptr<net::SSLPrivateKey> keyPtr;
};
- ~ClientCertificateStoreData();
void add(const QSslCertificate &certificate, const QSslKey &privateKey);
+ void remove(const QSslCertificate &certificate);
+ void clear();
- QList<Entry*> addedCerts;
- QList<Entry*> deletedCerts;
+ QVector<Entry*> extraCerts;
};
} // namespace QtWebEngineCore
diff --git a/tests/auto/core/qwebengineclientcertificatestore/tst_qwebengineclientcertificatestore.cpp b/tests/auto/core/qwebengineclientcertificatestore/tst_qwebengineclientcertificatestore.cpp
index 5c4d82f4..fe2f7141 100644
--- a/tests/auto/core/qwebengineclientcertificatestore/tst_qwebengineclientcertificatestore.cpp
+++ b/tests/auto/core/qwebengineclientcertificatestore/tst_qwebengineclientcertificatestore.cpp
@@ -85,7 +85,7 @@ void tst_QWebEngineClientCertificateStore::removeAndClearCertificates()
// Remove one certificate from in-memory store
auto list = QWebEngineClientCertificateStore::getInstance()->toList();
- QWebEngineClientCertificateStore::getInstance()->remove(list[0]);
+ QWebEngineClientCertificateStore::getInstance()->remove(list[0].certificate);
QCOMPARE(1, QWebEngineClientCertificateStore::getInstance()->toList().length());
// Remove all certificates in-memory store