From 7822c999e1b98ad0231c23905bb7d56a9267ccd0 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Tue, 19 Sep 2017 01:51:41 +0200 Subject: [PATCH] Bug 910207 - Disable preconnect when user certificates are installed r=keeler MozReview-Commit-ID: 1vGPxDCAcQR --HG-- extra : rebase_source : 3dda6f50ddbe1e03c7b7625c6039cb20896ef05e --- netwerk/protocol/http/moz.build | 1 + netwerk/protocol/http/nsHttpHandler.cpp | 54 ++++++++++++++++++++ netwerk/protocol/http/nsHttpHandler.h | 4 ++ security/manager/ssl/nsNSSCertificateDB.cpp | 28 +++++++++-- security/manager/ssl/nsNSSComponent.cpp | 55 +++++++++++++++++++++ security/manager/ssl/nsNSSComponent.h | 8 +++ 6 files changed, 147 insertions(+), 3 deletions(-) diff --git a/netwerk/protocol/http/moz.build b/netwerk/protocol/http/moz.build index 8d4baa53a75b..c272b645de52 100644 --- a/netwerk/protocol/http/moz.build +++ b/netwerk/protocol/http/moz.build @@ -123,6 +123,7 @@ LOCAL_INCLUDES += [ '/dom/base', '/netwerk/base', '/netwerk/cookie', + '/security/pkix/include', ] EXTRA_COMPONENTS += [ diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index b957d835b52f..72e696386865 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -67,6 +67,8 @@ #include "mozilla/dom/ContentParent.h" +#include "nsNSSComponent.h" + #if defined(XP_UNIX) #include #endif @@ -242,6 +244,7 @@ nsHttpHandler::nsHttpHandler() , mSpdyPingTimeout(PR_SecondsToInterval(8)) , mConnectTimeout(90000) , mParallelSpeculativeConnectLimit(6) + , mSpeculativeConnectEnabled(true) , mRequestTokenBucketEnabled(true) , mRequestTokenBucketMinParallelism(6) , mRequestTokenBucketHz(100) @@ -526,6 +529,8 @@ nsHttpHandler::Init() obsService->AddObserver(this, "browser:purge-session-history", true); obsService->AddObserver(this, NS_NETWORK_LINK_TOPIC, true); obsService->AddObserver(this, "application-background", true); + obsService->AddObserver(this, "psm:user-certificate-added", true); + obsService->AddObserver(this, "psm:user-certificate-deleted", true); if (!IsNeckoChild()) { obsService->AddObserver(this, @@ -2279,6 +2284,8 @@ nsHttpHandler::GetMisc(nsACString &value) // nsHttpHandler::nsIObserver //----------------------------------------------------------------------------- +static bool CanEnableSpeculativeConnect(); // forward declaration + NS_IMETHODIMP nsHttpHandler::Observe(nsISupports *subject, const char *topic, @@ -2424,6 +2431,14 @@ nsHttpHandler::Observe(nsISupports *subject, // We have detected a captive portal and we will reset the Fast Open // failure counter. ResetFastOpenConsecutiveFailureCounter(); + } else if (!strcmp(topic, "psm:user-certificate-added")) { + // A user certificate has just been added. + // We should immediately disable speculative connect + mSpeculativeConnectEnabled = false; + } else if (!strcmp(topic, "psm:user-certificate-deleted")) { + // If a user certificate has been removed, we need to check if there + // are others installed + mSpeculativeConnectEnabled = CanEnableSpeculativeConnect(); } return NS_OK; @@ -2431,6 +2446,35 @@ nsHttpHandler::Observe(nsISupports *subject, // nsISpeculativeConnect +static bool +CanEnableSpeculativeConnect() +{ + MOZ_ASSERT(NS_IsMainThread(), "Main thread only"); + + nsCOMPtr component(do_GetService(PSM_COMPONENT_CONTRACTID)); + if (!component) { + return false; + } + + // Check if any 3rd party PKCS#11 module are installed, as they may produce + // client certificates + bool activeSmartCards = false; + nsresult rv = component->HasActiveSmartCards(activeSmartCards); + if (NS_FAILED(rv) || activeSmartCards) { + return false; + } + + // If there are any client certificates installed, we can't enable speculative + // connect, as it may pop up the certificate chooser at any time. + bool hasUserCerts = false; + rv = component->HasUserCertsInstalled(hasUserCerts); + if (NS_FAILED(rv) || hasUserCerts) { + return false; + } + + return true; +} + nsresult nsHttpHandler::SpeculativeConnectInternal(nsIURI *aURI, nsIPrincipal *aPrincipal, @@ -2521,6 +2565,16 @@ nsHttpHandler::SpeculativeConnectInternal(nsIURI *aURI, if (NS_FAILED(rv)) return rv; + static bool sCheckedIfSpeculativeEnabled = false; + if (!sCheckedIfSpeculativeEnabled) { + sCheckedIfSpeculativeEnabled = true; + mSpeculativeConnectEnabled = CanEnableSpeculativeConnect(); + } + + if (usingSSL && !mSpeculativeConnectEnabled) { + return NS_ERROR_UNEXPECTED; + } + nsAutoCString host; rv = aURI->GetAsciiHost(host); if (NS_FAILED(rv)) diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h index 7b8f730c52af..80dfc9c210a7 100644 --- a/netwerk/protocol/http/nsHttpHandler.h +++ b/netwerk/protocol/http/nsHttpHandler.h @@ -579,6 +579,10 @@ private: // when starting a new speculative connection. uint32_t mParallelSpeculativeConnectLimit; + // We may disable speculative connect if the browser has user certificates + // installed as that might randomly popup the certificate choosing window. + bool mSpeculativeConnectEnabled; + // For Rate Pacing of HTTP/1 requests through a netwerk/base/EventTokenBucket // Active requests <= *MinParallelism are not subject to the rate pacing bool mRequestTokenBucketEnabled; diff --git a/security/manager/ssl/nsNSSCertificateDB.cpp b/security/manager/ssl/nsNSSCertificateDB.cpp index 94c1994e520e..47bdf17fc48b 100644 --- a/security/manager/ssl/nsNSSCertificateDB.cpp +++ b/security/manager/ssl/nsNSSCertificateDB.cpp @@ -768,13 +768,20 @@ nsNSSCertificateDB::ImportUserCertificate(uint8_t* data, uint32_t length, DisplayCertificateAlert(ctx, "UserCertImported", certToShow, locker); } + nsresult rv = NS_OK; int numCACerts = collectArgs->numcerts - 1; if (numCACerts) { SECItem* caCerts = collectArgs->rawCerts + 1; - return ImportValidCACerts(numCACerts, caCerts, ctx, locker); + rv = ImportValidCACerts(numCACerts, caCerts, ctx, locker); } - return NS_OK; + nsCOMPtr observerService = + mozilla::services::GetObserverService(); + if (observerService) { + observerService->NotifyObservers(nullptr, "psm:user-certificate-added", nullptr); + } + + return rv; } NS_IMETHODIMP @@ -811,6 +818,13 @@ nsNSSCertificateDB::DeleteCertificate(nsIX509Cert *aCert) nullptr); } MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("cert deleted: %d", srv)); + + nsCOMPtr observerService = + mozilla::services::GetObserverService(); + if (observerService) { + observerService->NotifyObservers(nullptr, "psm:user-certificate-deleted", nullptr); + } + return (srv) ? NS_ERROR_FAILURE : NS_OK; } @@ -990,7 +1004,15 @@ nsNSSCertificateDB::ImportPKCS12File(nsIFile* aFile) NS_ENSURE_ARG(aFile); nsPKCS12Blob blob; - return blob.ImportFromFile(aFile); + rv = blob.ImportFromFile(aFile); + + nsCOMPtr observerService = + mozilla::services::GetObserverService(); + if (NS_SUCCEEDED(rv) && observerService) { + observerService->NotifyObservers(nullptr, "psm:user-certificate-added", nullptr); + } + + return rv; } NS_IMETHODIMP diff --git a/security/manager/ssl/nsNSSComponent.cpp b/security/manager/ssl/nsNSSComponent.cpp index 78531d18048d..a598fb1557b3 100644 --- a/security/manager/ssl/nsNSSComponent.cpp +++ b/security/manager/ssl/nsNSSComponent.cpp @@ -1138,6 +1138,61 @@ LoadLoadableRootsTask::Run() return NS_OK; } +nsresult +nsNSSComponent::HasActiveSmartCards(bool& result) +{ + MOZ_ASSERT(NS_IsMainThread(), "Main thread only"); + if (!NS_IsMainThread()) { + return NS_ERROR_NOT_SAME_THREAD; + } + +#ifndef MOZ_NO_SMART_CARDS + nsNSSShutDownPreventionLock lock; + MutexAutoLock nsNSSComponentLock(mMutex); + + // A non-null list means at least one smart card thread was active + if (mThreadList) { + result = true; + return NS_OK; + } +#endif + result = false; + return NS_OK; +} + +nsresult +nsNSSComponent::HasUserCertsInstalled(bool& result) +{ + MOZ_ASSERT(NS_IsMainThread(), "Main thread only"); + if (!NS_IsMainThread()) { + return NS_ERROR_NOT_SAME_THREAD; + } + + nsNSSShutDownPreventionLock lock; + MutexAutoLock nsNSSComponentLock(mMutex); + + if (!mNSSInitialized) { + return NS_ERROR_NOT_INITIALIZED; + } + + result = false; + UniqueCERTCertList certList( + CERT_FindUserCertsByUsage(CERT_GetDefaultCertDB(), certUsageSSLClient, + false, true, nullptr)); + if (!certList) { + return NS_OK; + } + + // check if the list is empty + if (CERT_LIST_END(CERT_LIST_HEAD(certList), certList)) { + return NS_OK; + } + + // The list is not empty, meaning at least one cert is installed + result = true; + return NS_OK; +} + nsresult nsNSSComponent::BlockUntilLoadableRootsLoaded() { diff --git a/security/manager/ssl/nsNSSComponent.h b/security/manager/ssl/nsNSSComponent.h index 48433bbcd753..5643238b4df6 100644 --- a/security/manager/ssl/nsNSSComponent.h +++ b/security/manager/ssl/nsNSSComponent.h @@ -85,6 +85,10 @@ public: NS_IMETHOD BlockUntilLoadableRootsLoaded() = 0; + // Main thread only + NS_IMETHOD HasActiveSmartCards(bool& result) = 0; + NS_IMETHOD HasUserCertsInstalled(bool& result) = 0; + virtual ::already_AddRefed GetDefaultCertVerifier() = 0; }; @@ -144,6 +148,10 @@ public: NS_IMETHOD BlockUntilLoadableRootsLoaded() override; + // Main thread only + NS_IMETHOD HasActiveSmartCards(bool& result) override; + NS_IMETHOD HasUserCertsInstalled(bool& result) override; + ::already_AddRefed GetDefaultCertVerifier() override;