Bug 1313595 Reduce timeout for HSTS priming channels r=mayhemer
authorKate McKinley <kmckinley@mozilla.com>
Tue, 08 Nov 2016 17:49:39 +0900
changeset 322556 3472d9d9dd4777156d9707ec61254eafe9a24950
parent 322555 bc4ca02773a468d8a52b2b1218e8e27f69749b25
child 322557 aa7ad663977bbded0b6d64b1cbd6df88bd6d8065
push id30952
push usercbook@mozilla.com
push dateTue, 15 Nov 2016 11:27:04 +0000
treeherdermozilla-central@e16d1a881481 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1313595
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1313595 Reduce timeout for HSTS priming channels r=mayhemer Default is 3 seconds MozReview-Commit-ID: 47hoaTEL9hV
dom/security/test/hsts/head.js
modules/libpref/init/all.js
netwerk/protocol/http/HSTSPrimerListener.cpp
netwerk/protocol/http/HSTSPrimerListener.h
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsIHstsPrimingCallback.idl
toolkit/components/telemetry/Histograms.json
xpcom/base/ErrorList.h
--- a/dom/security/test/hsts/head.js
+++ b/dom/security/test/hsts/head.js
@@ -267,17 +267,20 @@ function SetupPrefTestEnvironment(which,
 
   var prefs = [["security.mixed_content.block_active_content",
                 settings.block_active],
                ["security.mixed_content.block_display_content",
                 settings.block_display],
                ["security.mixed_content.use_hsts",
                 settings.use_hsts],
                ["security.mixed_content.send_hsts_priming",
-                settings.send_hsts_priming]];
+                settings.send_hsts_priming],
+               ["security.mixed_content.hsts_priming_request_timeout",
+                30000],
+  ];
 
   if (additional_prefs) {
     for (let idx in additional_prefs) {
       prefs.push(additional_prefs[idx]);
     }
   }
 
   console.log("prefs=%s", prefs);
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -5537,17 +5537,20 @@ pref("security.mixed_content.send_hsts_p
 // Don't change the order of evaluation of mixed-content and HSTS upgrades in
 // order to be most compatible with current standards
 pref("security.mixed_content.use_hsts", false);
 #else
 // Change the order of evaluation so HSTS upgrades happen before
 // mixed-content blocking
 pref("security.mixed_content.use_hsts", true);
 #endif
-// Approximately 1 week default cache for HSTS priming failures
+// Approximately 1 week default cache for HSTS priming failures, in seconds
 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
 pref("dom.storageManager.enabled", true);
 #else
 pref("dom.storageManager.enabled", false);
 #endif
--- a/netwerk/protocol/http/HSTSPrimerListener.cpp
+++ b/netwerk/protocol/http/HSTSPrimerListener.cpp
@@ -12,53 +12,81 @@
 #include "nsSecurityHeaderParser.h"
 #include "nsISiteSecurityService.h"
 #include "nsISocketProvider.h"
 #include "nsISSLStatus.h"
 #include "nsISSLStatusProvider.h"
 #include "nsStreamUtils.h"
 #include "nsHttpChannel.h"
 #include "LoadInfo.h"
+#include "mozilla/Unused.h"
 
 namespace mozilla {
 namespace net {
 
 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);
 }
 
-NS_IMETHODIMP
-HSTSPrimingListener::OnStartRequest(nsIRequest *aRequest,
-                                    nsISupports *aContext)
+void
+HSTSPrimingListener::ReportTiming(nsresult aResult)
 {
-  nsresult primingResult = CheckHSTSPrimingRequestStatus(aRequest);
-  nsCOMPtr<nsIHstsPrimingCallback> callback(mCallback);
-  mCallback = nullptr;
-
   nsCOMPtr<nsITimedChannel> timingChannel =
-    do_QueryInterface(callback);
+    do_QueryInterface(mCallback);
   if (timingChannel) {
     TimeStamp channelCreationTime;
     nsresult rv = timingChannel->GetChannelCreation(&channelCreationTime);
     if (NS_SUCCEEDED(rv) && !channelCreationTime.IsNull()) {
       PRUint32 interval =
         (PRUint32) (TimeStamp::Now() - channelCreationTime).ToMilliseconds();
       Telemetry::Accumulate(Telemetry::HSTS_PRIMING_REQUEST_DURATION,
-          (NS_SUCCEEDED(primingResult)) ? NS_LITERAL_CSTRING("success")
-                                        : NS_LITERAL_CSTRING("failure"),
+          (NS_SUCCEEDED(aResult)) ? NS_LITERAL_CSTRING("success")
+                                  : NS_LITERAL_CSTRING("failure"),
           interval);
     }
   }
+}
+
+NS_IMETHODIMP
+HSTSPrimingListener::OnStartRequest(nsIRequest *aRequest,
+                                    nsISupports *aContext)
+{
+  nsCOMPtr<nsIHstsPrimingCallback> 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)"));
     return callback->OnHSTSPrimingFailed(primingResult, false);
   }
 
   LOG(("HSTS Priming Succeeded (request was approved)"));
   return callback->OnHSTSPrimingSucceeded(false);
@@ -134,22 +162,47 @@ HSTSPrimingListener::OnDataAvailable(nsI
                                      nsIInputStream *inStr,
                                      uint64_t sourceOffset,
                                      uint32_t count)
 {
   uint32_t totalRead;
   return inStr->ReadSegments(NS_DiscardSegment, nullptr, count, &totalRead);
 }
 
+/** nsITimerCallback **/
+NS_IMETHODIMP
+HSTSPrimingListener::Notify(nsITimer* timer)
+{
+  nsresult rv;
+  nsCOMPtr<nsIHstsPrimingCallback> 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<nsIURI> finalChannelURI;
   nsresult rv = NS_GetFinalChannelURI(aRequestChannel, getter_AddRefs(finalChannelURI));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIURI> uri;
   rv = NS_GetSecureUpgradedURI(finalChannelURI, getter_AddRefs(uri));
   NS_ENSURE_SUCCESS(rv,rv);
 
@@ -225,16 +278,18 @@ HSTSPrimingListener::StartHSTSPriming(ns
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Set method and headers
   nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(primingChannel);
   if (!httpChannel) {
     NS_ERROR("HSTSPrimingListener: Failed to QI to nsIHttpChannel!");
     return NS_ERROR_FAILURE;
   }
+  nsCOMPtr<nsIHttpChannelInternal> 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.
   rv = httpChannel->SetRequestMethod(NS_LITERAL_CSTRING("HEAD"));
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = httpChannel->
     SetRequestHeader(NS_LITERAL_CSTRING("Upgrade-Insecure-Requests"),
@@ -244,30 +299,51 @@ HSTSPrimingListener::StartHSTSPriming(ns
   // attempt to set the class of service flags on the new channel
   nsCOMPtr<nsIClassOfService> requestClass = do_QueryInterface(aRequestChannel);
   if (!requestClass) {
     NS_ERROR("HSTSPrimingListener: aRequestChannel is not an nsIClassOfService");
     return NS_ERROR_FAILURE;
   }
   nsCOMPtr<nsIClassOfService> primingClass = do_QueryInterface(httpChannel);
   if (!primingClass) {
-    NS_ERROR("HSTSPrimingListener: aRequestChannel is not an nsIClassOfService");
+    NS_ERROR("HSTSPrimingListener: httpChannel is not an nsIClassOfService");
     return NS_ERROR_FAILURE;
   }
 
   uint32_t classFlags = 0;
   rv = requestClass ->GetClassFlags(&classFlags);
   NS_ENSURE_SUCCESS(rv, rv);
   rv = primingClass->SetClassFlags(classFlags);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Set up listener which will start the original channel
-  nsCOMPtr<nsIStreamListener> primingListener(new HSTSPrimingListener(aCallback));
+  // The priming channel should have highest priority so that it completes as
+  // quickly as possible, allowing the load to proceed.
+  nsCOMPtr<nsISupportsPriority> 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);
   // Start priming
-  rv = primingChannel->AsyncOpen2(primingListener);
+  rv = primingChannel->AsyncOpen2(listener);
   NS_ENSURE_SUCCESS(rv, rv);
+  listener->mPrimingChannel.swap(primingChannel);
+
+  nsCOMPtr<nsITimer> 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;
 }
 
 } // namespace net
 } // namespace mozilla
--- a/netwerk/protocol/http/HSTSPrimerListener.h
+++ b/netwerk/protocol/http/HSTSPrimerListener.h
@@ -44,36 +44,41 @@ enum HSTSPrimingResult {
   // of mixed-content and hsts, and mixed-content blocks the load
   eHSTS_PRIMING_SUCCEEDED_BLOCK   = 5,
   // When priming succeeds, but preferences require preservation of the order
   // of mixed-content and hsts, and mixed-content allows the load over http
   eHSTS_PRIMING_SUCCEEDED_HTTP    = 6,
   // 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
+  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
 };
 
 //////////////////////////////////////////////////////////////////////////
 // Class used as streamlistener and notification callback when
 // 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 nsIInterfaceRequestor,
+                                  public nsITimerCallback
 {
 public:
-  explicit HSTSPrimingListener(nsIHstsPrimingCallback* aCallback)
-   : mCallback(aCallback)
-  {
-  }
+  explicit HSTSPrimingListener(nsIHstsPrimingCallback* aCallback);
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSIINTERFACEREQUESTOR
+  NS_DECL_NSITIMERCALLBACK
 
 private:
   ~HSTSPrimingListener() {}
 
   // Only nsHttpChannel can invoke HSTS priming
   friend class mozilla::net::nsHttpChannel;
 
   /**
@@ -91,18 +96,38 @@ private:
                                    nsIHstsPrimingCallback* aCallback);
 
   /**
    * Given a request, return NS_OK if it has resulted in a cached HSTS update.
    * We don't need to check for the header as that has already been done for us.
    */
   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<nsIHstsPrimingCallback> mCallback;
+
+  /**
+   * Keep a handle to the priming channel so we can cancel it on timeout
+   */
+  nsCOMPtr<nsIChannel> mPrimingChannel;
+
+  /**
+   * Keep a handle to the timer around so it can be canceled if we don't time
+   * out.
+   */
+  nsCOMPtr<nsITimer> mHSTSPrimingTimer;
+
+  /**
+   * How long (in ms) before an HSTS Priming channel times out.
+   * Preference: security.mixed_content.hsts_priming_request_timeout
+   */
+  static uint32_t sHSTSPrimingTimeout;
 };
 
 
 }} // mozilla::net
 
 #endif // HSTSPrimingListener_h__
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -7982,29 +7982,34 @@ nsHttpChannel::OnHSTSPrimingSucceeded(bo
  */
 nsresult
 nsHttpChannel::OnHSTSPrimingFailed(nsresult aError, bool aCached)
 {
     bool wouldBlock = mLoadInfo->GetMixedContentWouldBlock();
 
     LOG(("HSTS Priming Failed [this=%p], %s the load", this,
                 (wouldBlock) ? "blocking" : "allowing"));
-    if (aCached) {
+    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) {
         // 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
     // security.mixed_content.hsts_priming_cache_timeout seconds.
     nsISiteSecurityService* sss = gHttpHandler->GetSSService();
     NS_ENSURE_TRUE(sss, NS_ERROR_OUT_OF_MEMORY);
     nsresult rv = sss->CacheNegativeHSTSResult(mURI,
             nsMixedContentBlocker::sHSTSPrimingCacheTimeout);
--- a/netwerk/protocol/http/nsIHstsPrimingCallback.idl
+++ b/netwerk/protocol/http/nsIHstsPrimingCallback.idl
@@ -28,16 +28,17 @@ interface nsIHstsPrimingCallback : nsISu
    *
    * May be invoked synchronously if HSTS priming has already been performed
    * for the host.
    *
    * @param aCached whether the result was already in the HSTS cache
    */
   [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
    * site can be upgraded safely to HTTPS. The request may still be allowed
    * based on the user's preferences.
    *
    * May be invoked synchronously if HSTS priming has already been performed
    * for the host.
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -8138,17 +8138,17 @@
     "description": "How often would blocked mixed content be allowed if HSTS upgrades were allowed, including how often would we send an HSTS priming request? 0=display/no-HSTS, 1=display/HSTS, 2=active/no-HSTS, 3=active/HSTS, 4=display/no-HSTS-priming, 5=display/do-HSTS-priming, 6=active/no-HSTS-priming, 7=active/do-HSTS-priming"
   },
   "MIXED_CONTENT_HSTS_PRIMING_RESULT": {
     "alert_emails": ["seceng@mozilla.org"],
     "bug_numbers": [1246540],
     "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)"
+    "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)"
   },
   "HSTS_PRIMING_REQUEST_DURATION": {
     "alert_emails": ["seceng-telemetry@mozilla.org"],
     "bug_numbers": [1311893],
     "expires_in_version": "58",
     "kind": "exponential",
     "low": 100,
     "high": 30000,
--- a/xpcom/base/ErrorList.h
+++ b/xpcom/base/ErrorList.h
@@ -333,16 +333,20 @@
   ERROR(NS_NET_STATUS_CONNECTED_TO,    FAILURE(4)),
   ERROR(NS_NET_STATUS_SENDING_TO,      FAILURE(5)),
   ERROR(NS_NET_STATUS_WAITING_FOR,     FAILURE(10)),
   ERROR(NS_NET_STATUS_RECEIVING_FROM,  FAILURE(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
 
 
   /* ======================================================================= */
   /* 7: NS_ERROR_MODULE_PLUGINS */
   /* ======================================================================= */
 #define MODULE NS_ERROR_MODULE_PLUGINS
   ERROR(NS_ERROR_PLUGINS_PLUGINSNOTCHANGED,        FAILURE(1000)),