diff --git a/dom/security/test/hsts/head.js b/dom/security/test/hsts/head.js index 34b0ba2184c4..362b364445e2 100644 --- a/dom/security/test/hsts/head.js +++ b/dom/security/test/hsts/head.js @@ -272,10 +272,7 @@ function SetupPrefTestEnvironment(which, additional_prefs) { ["security.mixed_content.use_hsts", settings.use_hsts], ["security.mixed_content.send_hsts_priming", - settings.send_hsts_priming], - ["security.mixed_content.hsts_priming_request_timeout", - 30000], - ]; + settings.send_hsts_priming]]; if (additional_prefs) { for (let idx in additional_prefs) { diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 5509f5d63866..aadd877d0073 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -5528,11 +5528,8 @@ pref("security.mixed_content.use_hsts", false); // mixed-content blocking pref("security.mixed_content.use_hsts", true); #endif -// Approximately 1 week default cache for HSTS priming failures, in seconds +// Approximately 1 week default cache for HSTS priming failures pref ("security.mixed_content.hsts_priming_cache_timeout", 10080); -// Force the channel to timeout in 3 seconds if we have not received -// expects a time in milliseconds -pref ("security.mixed_content.hsts_priming_request_timeout", 3000); // Disable Storage api in release builds. #ifdef NIGHTLY_BUILD diff --git a/netwerk/protocol/http/HSTSPrimerListener.cpp b/netwerk/protocol/http/HSTSPrimerListener.cpp index c1c600c350b2..8c9d28d3637d 100644 --- a/netwerk/protocol/http/HSTSPrimerListener.cpp +++ b/netwerk/protocol/http/HSTSPrimerListener.cpp @@ -17,7 +17,6 @@ #include "nsStreamUtils.h" #include "nsHttpChannel.h" #include "LoadInfo.h" -#include "mozilla/Unused.h" namespace mozilla { namespace net { @@ -27,30 +26,22 @@ using namespace mozilla; NS_IMPL_ISUPPORTS(HSTSPrimingListener, nsIStreamListener, nsIRequestObserver, nsIInterfaceRequestor) -// default to 3000ms, same as the preference -uint32_t HSTSPrimingListener::sHSTSPrimingTimeout = 3000; - - -HSTSPrimingListener::HSTSPrimingListener(nsIHstsPrimingCallback* aCallback) - : mCallback(aCallback) -{ - static nsresult rv = - Preferences::AddUintVarCache(&sHSTSPrimingTimeout, - "security.mixed_content.hsts_priming_request_timeout"); - Unused << rv; -} - NS_IMETHODIMP HSTSPrimingListener::GetInterface(const nsIID & aIID, void **aResult) { return QueryInterface(aIID, aResult); } -void -HSTSPrimingListener::ReportTiming(nsresult aResult) +NS_IMETHODIMP +HSTSPrimingListener::OnStartRequest(nsIRequest *aRequest, + nsISupports *aContext) { + nsresult primingResult = CheckHSTSPrimingRequestStatus(aRequest); + nsCOMPtr callback(mCallback); + mCallback = nullptr; + nsCOMPtr timingChannel = - do_QueryInterface(mCallback); + do_QueryInterface(callback); if (timingChannel) { TimeStamp channelCreationTime; nsresult rv = timingChannel->GetChannelCreation(&channelCreationTime); @@ -58,30 +49,11 @@ HSTSPrimingListener::ReportTiming(nsresult aResult) PRUint32 interval = (PRUint32) (TimeStamp::Now() - channelCreationTime).ToMilliseconds(); Telemetry::Accumulate(Telemetry::HSTS_PRIMING_REQUEST_DURATION, - (NS_SUCCEEDED(aResult)) ? NS_LITERAL_CSTRING("success") - : NS_LITERAL_CSTRING("failure"), + (NS_SUCCEEDED(primingResult)) ? NS_LITERAL_CSTRING("success") + : NS_LITERAL_CSTRING("failure"), interval); } } -} - -NS_IMETHODIMP -HSTSPrimingListener::OnStartRequest(nsIRequest *aRequest, - nsISupports *aContext) -{ - nsCOMPtr callback; - callback.swap(mCallback); - if (mHSTSPrimingTimer) { - Unused << mHSTSPrimingTimer->Cancel(); - mHSTSPrimingTimer = nullptr; - } - - // if callback is null, we have already canceled this request and reported - // the failure - NS_ENSURE_STATE(callback); - - nsresult primingResult = CheckHSTSPrimingRequestStatus(aRequest); - ReportTiming(primingResult); if (NS_FAILED(primingResult)) { LOG(("HSTS Priming Failed (request was not approved)")); @@ -167,37 +139,12 @@ HSTSPrimingListener::OnDataAvailable(nsIRequest *aRequest, return inStr->ReadSegments(NS_DiscardSegment, nullptr, count, &totalRead); } -/** nsITimerCallback **/ -NS_IMETHODIMP -HSTSPrimingListener::Notify(nsITimer* timer) -{ - nsresult rv; - nsCOMPtr callback; - callback.swap(mCallback); - NS_ENSURE_STATE(callback); - ReportTiming(NS_ERROR_HSTS_PRIMING_TIMEOUT); - - if (mPrimingChannel) { - rv = mPrimingChannel->Cancel(NS_ERROR_HSTS_PRIMING_TIMEOUT); - if (NS_FAILED(rv)) { - // do what? - NS_ERROR("HSTS Priming timed out, and we got an error canceling the priming channel."); - } - } - - rv = callback->OnHSTSPrimingFailed(NS_ERROR_HSTS_PRIMING_TIMEOUT, false); - if (NS_FAILED(rv)) { - NS_ERROR("HSTS Priming timed out, and we got an error reporting the failure."); - } - - return NS_OK; // unused -} - // static nsresult HSTSPrimingListener::StartHSTSPriming(nsIChannel* aRequestChannel, nsIHstsPrimingCallback* aCallback) { + nsCOMPtr finalChannelURI; nsresult rv = NS_GetFinalChannelURI(aRequestChannel, getter_AddRefs(finalChannelURI)); NS_ENSURE_SUCCESS(rv, rv); @@ -283,8 +230,6 @@ HSTSPrimingListener::StartHSTSPriming(nsIChannel* aRequestChannel, NS_ERROR("HSTSPrimingListener: Failed to QI to nsIHttpChannel!"); return NS_ERROR_FAILURE; } - nsCOMPtr internal = do_QueryInterface(primingChannel); - NS_ENSURE_STATE(internal); // Currently using HEAD per the draft, but under discussion to change to GET // with credentials so if the upgrade is approved the result is already cached. @@ -304,7 +249,7 @@ HSTSPrimingListener::StartHSTSPriming(nsIChannel* aRequestChannel, } nsCOMPtr primingClass = do_QueryInterface(httpChannel); if (!primingClass) { - NS_ERROR("HSTSPrimingListener: httpChannel is not an nsIClassOfService"); + NS_ERROR("HSTSPrimingListener: aRequestChannel is not an nsIClassOfService"); return NS_ERROR_FAILURE; } @@ -314,33 +259,12 @@ HSTSPrimingListener::StartHSTSPriming(nsIChannel* aRequestChannel, rv = primingClass->SetClassFlags(classFlags); NS_ENSURE_SUCCESS(rv, rv); - // The priming channel should have highest priority so that it completes as - // quickly as possible, allowing the load to proceed. - nsCOMPtr p = do_QueryInterface(primingChannel); - if (p) { - uint32_t priority = nsISupportsPriority::PRIORITY_HIGHEST; - - p->SetPriority(priority); - } - // Set up listener which will start the original channel - HSTSPrimingListener* listener = new HSTSPrimingListener(aCallback); + nsCOMPtr primingListener(new HSTSPrimingListener(aCallback)); + // Start priming - rv = primingChannel->AsyncOpen2(listener); + rv = primingChannel->AsyncOpen2(primingListener); NS_ENSURE_SUCCESS(rv, rv); - listener->mPrimingChannel.swap(primingChannel); - - nsCOMPtr timer = do_CreateInstance(NS_TIMER_CONTRACTID); - NS_ENSURE_STATE(timer); - - rv = timer->InitWithCallback(listener, - sHSTSPrimingTimeout, - nsITimer::TYPE_ONE_SHOT); - if (NS_FAILED(rv)) { - NS_ERROR("HSTS Priming failed to initialize channel cancellation timer"); - } - - listener->mHSTSPrimingTimer.swap(timer); return NS_OK; } diff --git a/netwerk/protocol/http/HSTSPrimerListener.h b/netwerk/protocol/http/HSTSPrimerListener.h index 6e4906dfe2c2..05089911bfe7 100644 --- a/netwerk/protocol/http/HSTSPrimerListener.h +++ b/netwerk/protocol/http/HSTSPrimerListener.h @@ -49,13 +49,7 @@ enum HSTSPrimingResult { // HSTS priming failed, and the load is blocked by mixed-content eHSTS_PRIMING_FAILED_BLOCK = 7, // HSTS priming failed, and the load is allowed by mixed-content - eHSTS_PRIMING_FAILED_ACCEPT = 8, - // The HSTS Priming request timed out, and the load is blocked by - // mixed-content - eHSTS_PRIMING_TIMEOUT_BLOCK = 9, - // The HSTS Priming request timed out, and the load is allowed by - // mixed-content - eHSTS_PRIMING_TIMEOUT_ACCEPT = 10 + eHSTS_PRIMING_FAILED_ACCEPT = 8 }; ////////////////////////////////////////////////////////////////////////// @@ -63,17 +57,18 @@ enum HSTSPrimingResult { // doing the HEAD request for an HSTS Priming check. Needs to be an // nsIStreamListener in order to receive events from AsyncOpen2 class HSTSPrimingListener final : public nsIStreamListener, - public nsIInterfaceRequestor, - public nsITimerCallback + public nsIInterfaceRequestor { public: - explicit HSTSPrimingListener(nsIHstsPrimingCallback* aCallback); + explicit HSTSPrimingListener(nsIHstsPrimingCallback* aCallback) + : mCallback(aCallback) + { + } NS_DECL_ISUPPORTS NS_DECL_NSISTREAMLISTENER NS_DECL_NSIREQUESTOBSERVER NS_DECL_NSIINTERFACEREQUESTOR - NS_DECL_NSITIMERCALLBACK private: ~HSTSPrimingListener() {} @@ -101,30 +96,10 @@ private: */ nsresult CheckHSTSPrimingRequestStatus(nsIRequest* aRequest); - // send telemetry about how long HSTS priming requests take - void ReportTiming(nsresult aResult); - /** * the nsIHttpChannel to notify with the result of HSTS priming. */ nsCOMPtr mCallback; - - /** - * Keep a handle to the priming channel so we can cancel it on timeout - */ - nsCOMPtr mPrimingChannel; - - /** - * Keep a handle to the timer around so it can be canceled if we don't time - * out. - */ - nsCOMPtr mHSTSPrimingTimer; - - /** - * How long (in ms) before an HSTS Priming channel times out. - * Preference: security.mixed_content.hsts_priming_request_timeout - */ - static uint32_t sHSTSPrimingTimeout; }; diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index ca8dcca77507..f380d94c0c62 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -8025,24 +8025,19 @@ nsHttpChannel::OnHSTSPrimingFailed(nsresult aError, bool aCached) LOG(("HSTS Priming Failed [this=%p], %s the load", this, (wouldBlock) ? "blocking" : "allowing")); - if (aError == NS_ERROR_HSTS_PRIMING_TIMEOUT) { - // A priming request was sent, but timed out - Telemetry::Accumulate(Telemetry::MIXED_CONTENT_HSTS_PRIMING_RESULT, - (wouldBlock) ? HSTSPrimingResult::eHSTS_PRIMING_TIMEOUT_BLOCK : - HSTSPrimingResult::eHSTS_PRIMING_TIMEOUT_ACCEPT); - } else if (aCached) { + if (aCached) { // Between the time we marked for priming and started the priming request, // the host was found to not allow the upgrade, probably from another // priming request. Telemetry::Accumulate(Telemetry::MIXED_CONTENT_HSTS_PRIMING_RESULT, (wouldBlock) ? HSTSPrimingResult::eHSTS_PRIMING_CACHED_BLOCK : - HSTSPrimingResult::eHSTS_PRIMING_CACHED_NO_UPGRADE); + HSTSPrimingResult::eHSTS_PRIMING_CACHED_NO_UPGRADE); } else { // A priming request was sent, and no HSTS header was found that allows // the upgrade. Telemetry::Accumulate(Telemetry::MIXED_CONTENT_HSTS_PRIMING_RESULT, (wouldBlock) ? HSTSPrimingResult::eHSTS_PRIMING_FAILED_BLOCK : - HSTSPrimingResult::eHSTS_PRIMING_FAILED_ACCEPT); + HSTSPrimingResult::eHSTS_PRIMING_FAILED_ACCEPT); } // Don't visit again for at least diff --git a/netwerk/protocol/http/nsIHstsPrimingCallback.idl b/netwerk/protocol/http/nsIHstsPrimingCallback.idl index 525248eaa0f6..01f53a5b212d 100644 --- a/netwerk/protocol/http/nsIHstsPrimingCallback.idl +++ b/netwerk/protocol/http/nsIHstsPrimingCallback.idl @@ -33,7 +33,6 @@ interface nsIHstsPrimingCallback : nsISupports */ [noscript, nostdcall] void onHSTSPrimingSucceeded(in bool aCached); - /** * HSTS priming has seen no STS header, the request itself has failed, * or some other failure which does not constitute a positive signal that the diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index c48a6994e2ed..8aa9f0306fab 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -8150,7 +8150,7 @@ "expires_in_version": "60", "kind": "enumerated", "n_values": 16, - "description": "How often do we get back an HSTS priming result which upgrades the connection to HTTPS? 0=cached (no upgrade), 1=cached (do upgrade), 2=cached (blocked), 3=already upgraded, 4=priming succeeded, 5=priming succeeded (block due to pref), 6=priming succeeded (no upgrade due to pref), 7=priming failed (block), 8=priming failed (accept), 9=timeout (block), 10=timeout (accept)" + "description": "How often do we get back an HSTS priming result which upgrades the connection to HTTPS? 0=cached (no upgrade), 1=cached (do upgrade), 2=cached (blocked), 3=already upgraded, 4=priming succeeded, 5=priming succeeded (block due to pref), 6=priming succeeded (no upgrade due to pref), 7=priming failed (block), 8=priming failed (accept)" }, "HSTS_PRIMING_REQUEST_DURATION": { "alert_emails": ["seceng-telemetry@mozilla.org"], diff --git a/xpcom/base/ErrorList.h b/xpcom/base/ErrorList.h index 66ac959459c1..cfa461fe402c 100644 --- a/xpcom/base/ErrorList.h +++ b/xpcom/base/ErrorList.h @@ -338,10 +338,6 @@ /* nsIInterceptedChannel */ /* Generic error for non-specific failures during service worker interception */ ERROR(NS_ERROR_INTERCEPTION_FAILED, FAILURE(100)), - - /* nsIHstsPrimingListener */ - /* Error code for HSTS priming timeout to distinguish from blocking */ - ERROR(NS_ERROR_HSTS_PRIMING_TIMEOUT, FAILURE(110)), #undef MODULE