Bug 1522412 - P1. Replace LOAD_CLASSIFY_URI flag with a heuristic algorithm. r=Ehsan,mayhemer
☠☠ backed out by 747a5da93708 ☠ ☠
authorDimi Lee <dlee@mozilla.com>
Thu, 21 Mar 2019 07:29:48 +0000
changeset 465372 b80098d0a5c4197287582b6db0bd71aaa6bbd05c
parent 465356 184c209dca37de935373d28826a3ceed8e0bc77d
child 465373 91e5b339fe11c8211bced586f5eb31be46a47693
push id35738
push userccoroiu@mozilla.com
push dateThu, 21 Mar 2019 21:59:09 +0000
treeherdermozilla-central@7eb8e627961c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan, mayhemer
bugs1522412, 1321783
milestone68.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 1522412 - P1. Replace LOAD_CLASSIFY_URI flag with a heuristic algorithm. r=Ehsan,mayhemer In this patch, we move from a model where URL classification is opt-in (by specifying LOAD_CLASSIFIER_URI) to a model where it is enforced by default unless a channel opts out or is deemed to be a critical channel based on simple heuristics. The heuristics exempt a channel from classification if it is triggered by system with an exception when it is a top level load. To ensure critical channels are never classified, we also exempt channels which are flagged as "BeConservative" (implemented in bug 1321783). Another load flag LOAD_BYPASS_URL_CLASSIFIER is also introduced for the same reason. Differential Revision: https://phabricator.services.mozilla.com/D22110
netwerk/base/nsBaseChannel.cpp
netwerk/base/nsNetUtil.cpp
netwerk/base/nsNetUtil.h
netwerk/protocol/http/HttpBaseChannel.h
netwerk/protocol/http/nsHttpChannel.cpp
--- a/netwerk/base/nsBaseChannel.cpp
+++ b/netwerk/base/nsBaseChannel.cpp
@@ -327,17 +327,17 @@ void nsBaseChannel::ContinueHandleAsyncR
 
 void nsBaseChannel::ClassifyURI() {
   // For channels created in the child process, delegate to the parent to
   // classify URIs.
   if (!XRE_IsParentProcess()) {
     return;
   }
 
-  if (mLoadFlags & LOAD_CLASSIFY_URI) {
+  if (NS_ShouldClassifyChannel(this)) {
     RefPtr<nsChannelClassifier> classifier = new nsChannelClassifier(this);
     if (classifier) {
       classifier->Start();
     } else {
       Cancel(NS_ERROR_OUT_OF_MEMORY);
     }
   }
 }
--- a/netwerk/base/nsNetUtil.cpp
+++ b/netwerk/base/nsNetUtil.cpp
@@ -3006,16 +3006,72 @@ bool NS_IsOffline() {
   nsCOMPtr<nsIIOService> ios = do_GetIOService();
   if (ios) {
     ios->GetOffline(&offline);
     ios->GetConnectivity(&connectivity);
   }
   return offline || !connectivity;
 }
 
+/**
+ * This function returns true if this channel should be classified by
+ * the URL Classifier, false otherwise.
+ *
+ * The idea of the algorithm to determine if a channel should be
+ * classified is based on:
+ * 1. Channels created by non-privileged code should be classified.
+ * 2. Top-level document’s channels, if loaded by privileged code
+ *    (system principal), should be classified.
+ * 3. Any other channel, created by privileged code, is considered safe.
+ *
+ * A bad/hacked/corrupted safebrowsing database, plus a mistakenly
+ * classified critical channel (this may result from a bug in the exemption
+ * rules or incorrect information being passed into) can cause serious
+ * problems. For example, if the updater channel is classified and blocked
+ * by the Safe Browsing, Firefox can't update itself, and there is no way to
+ * recover from that.
+ *
+ * So two safeguards are added to ensure critical channels are never
+ * automatically classified either because there is a bug in the algorithm
+ * or the data in loadinfo is wrong.
+ * 1. beConservative, this is set by ServiceRequest and we treat
+ *    channel created for ServiceRequest as critical channels.
+ * 2. nsIChannel::LOAD_BYPASS_URL_CLASSIFIER, channel's opener can use this
+ *    flag to enforce bypassing the URL classifier check.
+ */
+bool NS_ShouldClassifyChannel(nsIChannel *aChannel) {
+  nsCOMPtr<nsIHttpChannelInternal> httpChannel(do_QueryInterface(aChannel));
+  if (httpChannel) {
+    bool beConservative;
+    nsresult rv = httpChannel->GetBeConservative(&beConservative);
+
+    // beConservative flag, set by ServiceRequest to ensure channels that fetch
+    // update use conservative TLS setting, are used here to identify channels
+    // are critical to bypass classification. for channels don't support
+    // beConservative, continue to apply the exemption rules.
+    if (NS_SUCCEEDED(rv) && beConservative) {
+      return false;
+    }
+  }
+
+  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
+  if (loadInfo) {
+    nsContentPolicyType type = loadInfo->GetExternalContentPolicyType();
+
+    // Skip classifying channel triggered by system unless it is a top-level
+    // load.
+    if (nsContentUtils::IsSystemPrincipal(loadInfo->TriggeringPrincipal()) &&
+        nsIContentPolicy::TYPE_DOCUMENT != type) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 namespace mozilla {
 namespace net {
 
 bool InScriptableRange(int64_t val) {
   return (val <= kJS_MAX_SAFE_INTEGER) && (val >= kJS_MIN_SAFE_INTEGER);
 }
 
 bool InScriptableRange(uint64_t val) { return val <= kJS_MAX_SAFE_UINTEGER; }
--- a/netwerk/base/nsNetUtil.h
+++ b/netwerk/base/nsNetUtil.h
@@ -932,16 +932,21 @@ nsresult NS_CompareLoadInfoAndLoadContex
  * network.http.referer.defaultPolicy.pbmode for private mode
  * network.http.referer.defaultPolicy.trackers.pbmode for third-party trackers
  * in private mode
  */
 uint32_t NS_GetDefaultReferrerPolicy(nsIHttpChannel *aChannel = nullptr,
                                      nsIURI *aURI = nullptr,
                                      bool privateBrowsing = false);
 
+/**
+ * Return true if this channel should be classified by the URL classifier.
+ */
+bool NS_ShouldClassifyChannel(nsIChannel *aChannel);
+
 namespace mozilla {
 namespace net {
 
 const static uint64_t kJS_MAX_SAFE_UINTEGER = +9007199254740991ULL;
 const static int64_t kJS_MIN_SAFE_INTEGER = -9007199254740991LL;
 const static int64_t kJS_MAX_SAFE_INTEGER = +9007199254740991LL;
 
 // Make sure a 64bit value can be captured by JS MAX_SAFE_INTEGER
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -691,16 +691,19 @@ class HttpBaseChannel : public nsHashPro
   uint32_t mLoadedFromApplicationCache : 1;
   uint32_t mChannelIsForDownload : 1;
   uint32_t mTracingEnabled : 1;
   // True if timing collection is enabled
   uint32_t mTimingEnabled : 1;
   uint32_t mReportTiming : 1;
   uint32_t mAllowSpdy : 1;
   uint32_t mAllowAltSvc : 1;
+  // !!! This is also used by the URL classifier to exempt channels from
+  // classification. If this is changed or removed, make sure we also update
+  // NS_ShouldClassifyChannel accordingly !!!
   uint32_t mBeConservative : 1;
   uint32_t mTRR : 1;
   uint32_t mResponseTimeoutEnabled : 1;
   // A flag that should be false only if a cross-domain redirect occurred
   uint32_t mAllRedirectsSameOrigin : 1;
 
   // Is 1 if no redirects have occured or if all redirects
   // pass the Resource Timing timing-allow-check
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -6594,17 +6594,17 @@ nsresult nsHttpChannel::BeginConnect() {
 
   // We may have been cancelled already, either by on-modify-request
   // listeners or load group observers; in that case, we should not send the
   // request to the server
   if (mCanceled) {
     return mStatus;
   }
 
-  if (!(mLoadFlags & LOAD_CLASSIFY_URI)) {
+  if (!NS_ShouldClassifyChannel(this)) {
     MaybeStartDNSPrefetch();
     return ContinueBeginConnectWithResult();
   }
 
   // We are about to do an async lookup to check if the URI is a
   // tracker. If yes, this channel will be canceled by channel classifier.
   // Chances are the lookup is not needed so CheckIsTrackerWithLocalTable()
   // will return an error and then we can BeginConnectActual() right away.