Return both application and system certificates

For the certificate choice return both application and
system certificates.

Add unit test to cover the case on Linux.
Unfortunately it requires adding the user certificate to
the nss data store, which is not not nice, however porting
the certificate manger from Chromium is a bigger task.

Test runs only if the machine has pk12utils installed.

During the test the user certificate is imported into
the nss database with the nickname 'qwebengineclientcertificatestore'.

This can be removed later with:
ninja remove-user-personal-certificate

and verified with:
certutil -d sql:$HOME/.pki/nssdb -L

Pick-to: 6.4
Change-Id: I475fddc68ea56304980f6c835ed4cfed4b093ad4
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

6 files changed, 158 insertions, 13 deletions

diff --git a/src/core/net/client_cert_override.cpp b/src/core/net/client_cert_override.cpp
index 4ef08e91b..d1946de5d 100644
--- a/src/core/net/client_cert_override.cpp
+++ b/src/core/net/client_cert_override.cpp
@@ -72,7 +72,6 @@ net::ClientCertIdentityList ClientCertOverrideStore::GetClientCertsOnUIThread(co
 
     // Look for certificates in memory store
     net::ClientCertIdentityList selected_identities;
-
     for (int i = 0; i < clientCertOverrideData.length(); i++) {
         scoped_refptr<net::X509Certificate> cert = clientCertOverrideData[i]->certPtr;
         if (cert) {
@@ -94,11 +93,22 @@ void ClientCertOverrideStore::GetClientCertsReturn(const net::SSLCertRequestInfo
                                                    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, std::move(callback));
-    else
+    // Continue with native cert store and append them after memory certificates
+    if (m_nativeStore) {
+        ClientCertListCallback callback2 = base::BindOnce(
+                [](ClientCertOverrideStore::ClientCertListCallback callback,
+                   net::ClientCertIdentityList result1, net::ClientCertIdentityList result2) {
+                    while (!result2.empty()) {
+                        result1.push_back(std::move(result2.back()));
+                        result2.pop_back();
+                    }
+                    std::move(callback).Run(std::move(result1));
+                },
+                std::move(callback), std::move(result));
+        m_nativeStore->GetClientCerts(cert_request_info, std::move(callback2));
+    } else {
         std::move(callback).Run(std::move(result));
+    }
 }
 
 #endif // QT_CONFIG(ssl)
diff --git a/tests/auto/core/qwebengineclientcertificatestore/CMakeLists.txt b/tests/auto/core/qwebengineclientcertificatestore/CMakeLists.txt
index 72ebe3f9e..5b920f999 100644
--- a/tests/auto/core/qwebengineclientcertificatestore/CMakeLists.txt
+++ b/tests/auto/core/qwebengineclientcertificatestore/CMakeLists.txt
@@ -22,6 +22,8 @@ set(tst_qwebengineclientcertificatestore_resource_files
     "resources/server.key"
     "resources/client.pem"
     "resources/client.key"
+    "resources/client2.pem"
+    "resources/client2.key"
     "resources/ca.pem"
 )
 
@@ -32,3 +34,41 @@ qt_internal_add_resource(tst_qwebengineclientcertificatestore "tst_qwebenginecli
         ${tst_qwebengineclientcertificatestore_resource_files}
 )
 
+if(LINUX)
+
+    get_filename_component(homePath $ENV{HOME} ABSOLUTE)
+
+    find_program(pk12util_EXECUTABLE NAMES pk12util)
+
+    if(pk12util_EXECUTABLE)
+        add_custom_command(
+            DEPENDS resources/client2.p12
+            COMMAND ${pk12util_EXECUTABLE}
+                -d sql:"${homePath}/.pki/nssdb"
+                -n qwebengineclientcertificatestore
+                -i "${CMAKE_CURRENT_LIST_DIR}/resources/client2.p12"
+                -W \"\"
+            COMMAND ${CMAKE_COMMAND} -E touch pk12util.stamp
+            OUTPUT pk12util.stamp
+        )
+        add_custom_target(
+            add-user-personal-certificate
+            DEPENDS pk12util.stamp
+    )
+    qt_internal_extend_target(tst_qwebengineclientcertificatestore DEFINES TEST_NSS)
+    add_dependencies(tst_qwebengineclientcertificatestore  add-user-personal-certificate)
+    endif()
+
+    find_program(certutil_EXECUTABLE NAMES certutil)
+
+    if(certutil_EXECUTABLE)
+        add_custom_target(remove-user-personal-certificate
+            COMMAND ${certutil_EXECUTABLE}
+                -d sql:"${homePath}/.pki/nssdb"
+                -D
+                -n qwebengineclientcertificatestore
+            COMMAND ${CMAKE_COMMAND} -E remove pk12util.stamp
+        )
+    endif()
+endif()
+
diff --git a/tests/auto/core/qwebengineclientcertificatestore/resources/client2.key b/tests/auto/core/qwebengineclientcertificatestore/resources/client2.key
new file mode 100644
index 000000000..3c1346519
--- /dev/null
+++ b/tests/auto/core/qwebengineclientcertificatestore/resources/client2.key
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEowIBAAKCAQEAv0vrzULGwDJBoZgnGXdkMFxCvkTqqQYCE/LlNtStLJfJH7Fo
+CgenVFcJ8RIFHdkL7HeFAIZjDLSjIp2Ud41fd+VsaGgB/+j1/UeEN8nkArvYB9ol
+OnKGq6CbSrCocrLo2o2X+6eyLtrtLG6RLr8/UiqB2OWNAdnw70S5RCvnbV6phr8z
+bgYqPdPSBaedfZk5Kj6yM6XvIKSK6IjgZuo+Z5SyabJqk2VhaBlB7mjCf3Mj4zPD
+XvQXsAq0ZNQXQVwKRfJ2I9uAeNAZiQP5i00pBqe2kIJEKnk8qbP4/Jho2Tp8XSBC
+jHMn0oWrAZyO9vw3W940qmqmdRftyt+J8DO9xwIDAQABAoIBAGBpXTCYRR88tQNC
+cgJNv/r3pNPMXBBP7OAs/QUDbzwYS89jVDIp5VWGgIY1NMr0RyQooKnBEU6oA8hA
+b0FJySHeSSLduJRHzyKV1rdfU0Fldt2OPlEUw3bgfSPJoTwdm2n7DuxQemdPA1Xv
+a9CJpto8fjDYkJasRtfwZQdMsVjXCfQ/cCzkOkblUDZcc7yTx3uiBKF8Jy8C+0qc
+98btotYU88KWoE9A0ucWt/ik68MjYmccO6PYXKerNW2Ijgd1kik35G3TbEWxOFWW
+y3zLFtfoD+21SdUgTMzM06owDVfSt/MER4tOxFyUPRuze7BJXrBofGQfuPiGiPuK
+f5QZP8ECgYEA+x1PkClsqtRnjrzmRfi3OFez1Kbbzneucg5ssWR+Hd4EUFhhO42q
+te1ZYoydy09tEqd002U7e5hob0/o+rVK9jldpZszMCBfVDYCDqdtw5rNI89bL1Uz
+8krn6nk3BBx42lgAFU4C1JEaur4r14OOUtoFfRTAwjogQHcDmpyPNjcCgYEAwwSv
+FJAKRjw1oOXKlGotoeYEAREVxH9HFnfM5IcVwcwMt+KUFEyrMtXeH1gk7jo+2ev4
+87njQ8hU3VPObCUcnTJHi2a6D9JIY+zA9bKTJjc8drcBathipmwtak14TsX2qe14
+JBIKlC3V0h1FqM3ep76p4dnt7sTmVc7ZOqBR7PECgYEA1HQE94wEkzdnch0hmbuG
+kBWrYNPXDgS1w2uuzBqglPZcoflUMkV2U7s+r6EWc4d8WZbxwVRZkgTs/pgWHd66
+UD1SnKUFFsecv6t97BX9SMu0mYJ6vD4S2ABF3Fu3jzPjj596WowI2vz1J19zyj9U
+b4ZjtGKVfv4cgU3v76RbidsCgYAx4CvKzX/jMJjimoJx7KnZAxO5Fh6ED60loOQE
++ktlMgN6r/cBLg6GxM23JHrldn4Gi+QyqTLnbf/OTxW28NLdnTNRAqfJThV3gOBk
+thQOLQhIsEsrgUXRnE8NJd0EAHsyQGp+hyKvfP13bEcZgfVU311hRrQkYbUq8uj5
+pnDtcQKBgEFIpP7EzdJWrVOUjnjMQloqBhW8KVVtNwI5bmlcsUvVYjfZph016SiF
+UTfZss1KkBmQClAVtyZsrKIfObIJ9KJ4hPAzzk+ca1D6XTLsYjxPwtB/U0ewB2Dm
+yMxkXpT1kAiJ2Tdr1hZ8OcQhvnGWmrhtz+AkjyLXiYgST7Hubrxt
+-----END RSA PRIVATE KEY-----
diff --git a/tests/auto/core/qwebengineclientcertificatestore/resources/client2.p12 b/tests/auto/core/qwebengineclientcertificatestore/resources/client2.p12
new file mode 100644
index 000000000..feccd77e1
--- /dev/null
+++ b/tests/auto/core/qwebengineclientcertificatestore/resources/client2.p12
diff --git a/tests/auto/core/qwebengineclientcertificatestore/resources/client2.pem b/tests/auto/core/qwebengineclientcertificatestore/resources/client2.pem
new file mode 100644
index 000000000..39c0b3f09
--- /dev/null
+++ b/tests/auto/core/qwebengineclientcertificatestore/resources/client2.pem
@@ -0,0 +1,22 @@
+-----BEGIN CERTIFICATE-----
+MIIDsTCCApkCFFNQAgGBu5nr81tUMdXXLGkm8LjBMA0GCSqGSIb3DQEBCwUAMIGU
+MQswCQYDVQQGEwJERTEPMA0GA1UECAwGQmVybGluMQ8wDQYDVQQHDAZCZXJsaW4x
+FzAVBgNVBAoMDlRoZSBRdCBDb21wYW55MRQwEgYDVQQLDAtRdFdlYkVuZ2luZTES
+MBAGA1UEAwwJd3d3LnF0LmlvMSAwHgYJKoZIhvcNAQkBFhFxdHdlYmVuZ2luZUBx
+dC5pbzAeFw0yMjExMTYxOTIwMzBaFw0zMjExMTMxOTIwMzBaMIGUMQswCQYDVQQG
+EwJERTEPMA0GA1UECAwGQmVybGluMQ8wDQYDVQQHDAZCZXJsaW4xFzAVBgNVBAoM
+DlRoZSBRdCBDb21wYW55MRQwEgYDVQQLDAtRdFdlYkVuZ2luZTEWMBQGA1UEAwwN
+Y2xpZW50Mi5xdC5pbzEcMBoGCSqGSIb3DQEJARYNY2xpZW50MkBxdC5pbzCCASIw
+DQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL9L681CxsAyQaGYJxl3ZDBcQr5E
+6qkGAhPy5TbUrSyXyR+xaAoHp1RXCfESBR3ZC+x3hQCGYwy0oyKdlHeNX3flbGho
+Af/o9f1HhDfJ5AK72AfaJTpyhqugm0qwqHKy6NqNl/unsi7a7SxukS6/P1Iqgdjl
+jQHZ8O9EuUQr521eqYa/M24GKj3T0gWnnX2ZOSo+sjOl7yCkiuiI4GbqPmeUsmmy
+apNlYWgZQe5own9zI+Mzw170F7AKtGTUF0FcCkXydiPbgHjQGYkD+YtNKQantpCC
+RCp5PKmz+PyYaNk6fF0gQoxzJ9KFqwGcjvb8N1veNKpqpnUX7crfifAzvccCAwEA
+ATANBgkqhkiG9w0BAQsFAAOCAQEAic8F8q1TpP2ufnBRbrBp54Jgddl/zdVb7O3M
+AAK67KiEpEr9xPPVcIowfns1ZTIsIB8D4VS4NQGJXBrwvGWL08SpSmi76I1E156x
+9Hql0PHXCjqsJTOSEvljIgQ4sp33zs0DTmlyejSSGnG9sw2FtcYAGZNV+ImAhTO2
+DNxw3BnF++ilHsQbiWIKD5z14bOXb77SJrimup0YBzfwBWJO013k8g8lkiRRs5Ng
+XYVr3NoTLcIJQ7BTFu4W1Wegxwrw3fQZ98BBlCVh0htrOcLpWKelJeI16MgZA/7T
+P4MwvN5tkyjqrcsrDORldR6JKdX8i+GLF49MgRW4QispcZzoYA==
+-----END CERTIFICATE-----
diff --git a/tests/auto/core/qwebengineclientcertificatestore/tst_qwebengineclientcertificatestore.cpp b/tests/auto/core/qwebengineclientcertificatestore/tst_qwebengineclientcertificatestore.cpp
index 404791332..7d82a5640 100644
--- a/tests/auto/core/qwebengineclientcertificatestore/tst_qwebengineclientcertificatestore.cpp
+++ b/tests/auto/core/qwebengineclientcertificatestore/tst_qwebengineclientcertificatestore.cpp
@@ -19,8 +19,11 @@ public:
     ~tst_QWebEngineClientCertificateStore();
 
 private Q_SLOTS:
+    void init();
+    void cleanup();
     void addAndListCertificates();
     void removeAndClearCertificates();
+    void clientAuthentication_data();
     void clientAuthentication();
 };
 
@@ -32,6 +35,19 @@ tst_QWebEngineClientCertificateStore::~tst_QWebEngineClientCertificateStore()
 {
 }
 
+void tst_QWebEngineClientCertificateStore::init()
+{
+    QCOMPARE(0,
+             QWebEngineProfile::defaultProfile()->clientCertificateStore()->certificates().size());
+}
+
+void tst_QWebEngineClientCertificateStore::cleanup()
+{
+    QWebEngineProfile::defaultProfile()->clientCertificateStore()->clear();
+    QCOMPARE(0,
+             QWebEngineProfile::defaultProfile()->clientCertificateStore()->certificates().size());
+}
+
 void tst_QWebEngineClientCertificateStore::addAndListCertificates()
 {
     // Load QSslCertificate
@@ -63,6 +79,7 @@ void tst_QWebEngineClientCertificateStore::addAndListCertificates()
 
 void tst_QWebEngineClientCertificateStore::removeAndClearCertificates()
 {
+    addAndListCertificates();
     QCOMPARE(2, QWebEngineProfile::defaultProfile()->clientCertificateStore()->certificates().size());
 
     // Remove one certificate from in-memory store
@@ -75,8 +92,29 @@ void tst_QWebEngineClientCertificateStore::removeAndClearCertificates()
     QCOMPARE(0, QWebEngineProfile::defaultProfile()->clientCertificateStore()->certificates().size());
 }
 
+void tst_QWebEngineClientCertificateStore::clientAuthentication_data()
+{
+    QTest::addColumn<QString>("client_certificate");
+    QTest::addColumn<QString>("client_key");
+    QTest::addColumn<bool>("in_memory");
+    QTest::addColumn<bool>("add_more_in_memory_certificates");
+    QTest::newRow("in_memory") << ":/resources/client.pem"
+                               << ":/resources/client.key" << true << false;
+#if defined(TEST_NSS)
+    QTest::newRow("nss") << ":/resources/client2.pem"
+                         << ":/resources/client2.key" << false << false;
+    QTest::newRow("in_memory + nss") << ":/resources/client2.pem"
+                                     << ":/resources/client2.key" << false << true;
+#endif
+}
+
 void tst_QWebEngineClientCertificateStore::clientAuthentication()
 {
+    QFETCH(QString, client_certificate);
+    QFETCH(QString, client_key);
+    QFETCH(bool, in_memory);
+    QFETCH(bool, add_more_in_memory_certificates);
+
     HttpsServer server(":/resources/server.pem", ":/resources/server.key", ":resources/ca.pem");
     server.setExpectError(false);
     QVERIFY(server.start());
@@ -86,17 +124,21 @@ void tst_QWebEngineClientCertificateStore::clientAuthentication()
         rr->sendResponse();
     });
 
-    QFile certFile(":/resources/client.pem");
+    QFile certFile(client_certificate);
     certFile.open(QIODevice::ReadOnly);
     const QSslCertificate cert(certFile.readAll(), QSsl::Pem);
 
-    QFile keyFile(":/resources/client.key");
+    QFile keyFile(client_key);
     keyFile.open(QIODevice::ReadOnly);
     const QSslKey sslKey(keyFile.readAll(), QSsl::Rsa, QSsl::Pem, QSsl::PrivateKey, "");
 
-    QWebEngineProfile profile("clientAuthentication");
-    profile.clientCertificateStore()->add(cert, sslKey);
-    QWebEnginePage page(&profile);
+    if (in_memory)
+        QWebEngineProfile::defaultProfile()->clientCertificateStore()->add(cert, sslKey);
+
+    if (add_more_in_memory_certificates)
+        addAndListCertificates();
+
+    QWebEnginePage page;
     connect(&page, &QWebEnginePage::certificateError, [](QWebEngineCertificateError e) {
         // ca is self signed in this test simply accept the certificate error
         e.acceptCertificate();
@@ -104,9 +146,13 @@ void tst_QWebEngineClientCertificateStore::clientAuthentication()
     connect(&page, &QWebEnginePage::selectClientCertificate, &page,
             [&cert](QWebEngineClientCertificateSelection selection) {
                 QVERIFY(!selection.certificates().isEmpty());
-                const QSslCertificate &sCert = selection.certificates().at(0);
-                QVERIFY(cert == sCert);
-                selection.select(sCert);
+                for (const QSslCertificate &sCert : selection.certificates()) {
+                    if (cert == sCert) {
+                        selection.select(sCert);
+                        return;
+                    }
+                }
+                QFAIL("No certificate found.");
             });
     QSignalSpy loadFinishedSpy(&page, SIGNAL(loadFinished(bool)));
     page.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, false);