Bug 1100024: Don't call Connect if the principal is on a local or remote blocklist (r=mcmanus)
authorMonica Chew <mmc@mozilla.com>
Fri, 09 Jan 2015 13:25:13 -0800
changeset 223130 671ad56e6e12927323e4d89d0076e7c04f58b55c
parent 223129 6c8be2f5b065ea8f72082b7d7cdbe53f0f53b9f6
child 223131 bfab295de6328219709e34060bf41c2c5af6916c
push id10769
push usercbook@mozilla.com
push dateMon, 12 Jan 2015 14:15:52 +0000
treeherderfx-team@0e9765732906 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1100024
milestone37.0a1
Bug 1100024: Don't call Connect if the principal is on a local or remote blocklist (r=mcmanus)
netwerk/base/src/nsBaseChannel.cpp
netwerk/base/src/nsChannelClassifier.cpp
netwerk/base/src/nsChannelClassifier.h
netwerk/protocol/http/HttpBaseChannel.cpp
netwerk/protocol/http/HttpBaseChannel.h
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
netwerk/protocol/http/nsIHttpChannelInternal.idl
--- a/netwerk/base/src/nsBaseChannel.cpp
+++ b/netwerk/base/src/nsBaseChannel.cpp
@@ -284,25 +284,20 @@ void
 nsBaseChannel::ClassifyURI()
 {
   // For channels created in the child process, delegate to the parent to
   // classify URIs.
   if (XRE_GetProcessType() != GeckoProcessType_Default) {
     return;
   }
 
-  nsresult rv;
-
   if (mLoadFlags & LOAD_CLASSIFY_URI) {
     nsRefPtr<nsChannelClassifier> classifier = new nsChannelClassifier();
     if (classifier) {
-      rv = classifier->Start(this);
-      if (NS_FAILED(rv)) {
-        Cancel(rv);
-      }
+      classifier->Start(this);
     } else {
       Cancel(NS_ERROR_OUT_OF_MEMORY);
     }
   }
 }
 
 //-----------------------------------------------------------------------------
 // nsBaseChannel::nsISupports
--- a/netwerk/base/src/nsChannelClassifier.cpp
+++ b/netwerk/base/src/nsChannelClassifier.cpp
@@ -44,17 +44,18 @@ static PRLogModuleInfo *gChannelClassifi
 #endif
 #undef LOG
 #define LOG(args)     PR_LOG(gChannelClassifierLog, PR_LOG_DEBUG, args)
 
 NS_IMPL_ISUPPORTS(nsChannelClassifier,
                   nsIURIClassifierCallback)
 
 nsChannelClassifier::nsChannelClassifier()
-  : mIsAllowListed(false)
+  : mIsAllowListed(false),
+    mSuspendedChannel(false)
 {
 #if defined(PR_LOGGING)
     if (!gChannelClassifierLog)
         gChannelClassifierLog = PR_NewLogModule("nsChannelClassifier");
 #endif
 }
 
 nsresult
@@ -204,71 +205,83 @@ nsChannelClassifier::NotifyTrackingProte
     doc->SetHasTrackingContentLoaded(true);
     securityUI->GetState(&state);
     state |= nsIWebProgressListener::STATE_LOADED_TRACKING_CONTENT;
     eventSink->OnSecurityChange(nullptr, state);
 
     return NS_OK;
 }
 
+void
+nsChannelClassifier::Start(nsIChannel *aChannel)
+{
+  mChannel = aChannel;
+  nsresult rv = StartInternal(aChannel);
+  if (NS_FAILED(rv)) {
+    // If we aren't getting a callback for any reason, assume a good verdict and
+    // make sure we resume the channel if necessary.
+    OnClassifyComplete(NS_OK);
+  }
+}
+
 nsresult
-nsChannelClassifier::Start(nsIChannel *aChannel)
+nsChannelClassifier::StartInternal(nsIChannel *aChannel)
 {
     // Should only be called in the parent process.
     MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
 
     // Don't bother to run the classifier on a load that has already failed.
     // (this might happen after a redirect)
     nsresult status;
     aChannel->GetStatus(&status);
     if (NS_FAILED(status))
-        return NS_OK;
+        return status;
 
     // Don't bother to run the classifier on a cached load that was
-    // previously classified.
+    // previously classified as good.
     if (HasBeenClassified(aChannel)) {
-        return NS_OK;
+        return NS_ERROR_UNEXPECTED;
     }
 
     nsCOMPtr<nsIURI> uri;
     nsresult rv = aChannel->GetURI(getter_AddRefs(uri));
     NS_ENSURE_SUCCESS(rv, rv);
 
     // Don't bother checking certain types of URIs.
     bool hasFlags;
     rv = NS_URIChainHasFlags(uri,
                              nsIProtocolHandler::URI_DANGEROUS_TO_LOAD,
                              &hasFlags);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (hasFlags) return NS_OK;
+    if (hasFlags) return NS_ERROR_UNEXPECTED;
 
     rv = NS_URIChainHasFlags(uri,
                              nsIProtocolHandler::URI_IS_LOCAL_FILE,
                              &hasFlags);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (hasFlags) return NS_OK;
+    if (hasFlags) return NS_ERROR_UNEXPECTED;
 
     rv = NS_URIChainHasFlags(uri,
                              nsIProtocolHandler::URI_IS_UI_RESOURCE,
                              &hasFlags);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (hasFlags) return NS_OK;
+    if (hasFlags) return NS_ERROR_UNEXPECTED;
 
     rv = NS_URIChainHasFlags(uri,
                              nsIProtocolHandler::URI_IS_LOCAL_RESOURCE,
                              &hasFlags);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (hasFlags) return NS_OK;
+    if (hasFlags) return NS_ERROR_UNEXPECTED;
 
     nsCOMPtr<nsIURIClassifier> uriClassifier =
         do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID, &rv);
     if (rv == NS_ERROR_FACTORY_NOT_REGISTERED ||
         rv == NS_ERROR_NOT_AVAILABLE) {
         // no URI classifier, ignore this failure.
-        return NS_OK;
+        return NS_ERROR_NOT_AVAILABLE;
     }
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIScriptSecurityManager> securityManager =
         do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIPrincipal> principal;
@@ -277,34 +290,38 @@ nsChannelClassifier::Start(nsIChannel *a
     NS_ENSURE_SUCCESS(rv, rv);
 
     bool expectCallback;
     bool trackingProtectionEnabled = false;
     (void)ShouldEnableTrackingProtection(aChannel, &trackingProtectionEnabled);
 
     rv = uriClassifier->Classify(principal, trackingProtectionEnabled, this,
                                  &expectCallback);
-    if (NS_FAILED(rv)) return rv;
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
 
     if (expectCallback) {
         // Suspend the channel, it will be resumed when we get the classifier
         // callback.
         rv = aChannel->Suspend();
         if (NS_FAILED(rv)) {
             // Some channels (including nsJSChannel) fail on Suspend.  This
             // shouldn't be fatal, but will prevent malware from being
             // blocked on these channels.
-            return NS_OK;
+            LOG(("nsChannelClassifier[%p]: Couldn't suspend channel", this));
+            return rv;
         }
 
-        mSuspendedChannel = aChannel;
-#ifdef DEBUG
+        mSuspendedChannel = true;
         LOG(("nsChannelClassifier[%p]: suspended channel %p",
-             this, mSuspendedChannel.get()));
-#endif
+             this, mChannel.get()));
+    } else {
+        LOG(("nsChannelClassifier[%p]: not expecting callback", this));
+        return NS_ERROR_FAILURE;
     }
 
     return NS_OK;
 }
 
 // Note in the cache entry that this URL was classified, so that future
 // cached loads don't need to be checked.
 void
@@ -313,18 +330,17 @@ nsChannelClassifier::MarkEntryClassified
     // Should only be called in the parent process.
     MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
 
     // Don't cache tracking classifications because we support allowlisting.
     if (status == NS_ERROR_TRACKING_URI || mIsAllowListed) {
         return;
     }
 
-    nsCOMPtr<nsICachingChannel> cachingChannel =
-        do_QueryInterface(mSuspendedChannel);
+    nsCOMPtr<nsICachingChannel> cachingChannel = do_QueryInterface(mChannel);
     if (!cachingChannel) {
         return;
     }
 
     nsCOMPtr<nsISupports> cacheToken;
     cachingChannel->GetCacheToken(getter_AddRefs(cacheToken));
     if (!cacheToken) {
         return;
@@ -439,41 +455,47 @@ nsChannelClassifier::SetBlockedTrackingC
 }
 
 NS_IMETHODIMP
 nsChannelClassifier::OnClassifyComplete(nsresult aErrorCode)
 {
     // Should only be called in the parent process.
     MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
 
+    LOG(("nsChannelClassifier[%p]:OnClassifyComplete", this));
     if (mSuspendedChannel) {
         MarkEntryClassified(aErrorCode);
 
         if (NS_FAILED(aErrorCode)) {
 #ifdef DEBUG
             nsCOMPtr<nsIURI> uri;
-            mSuspendedChannel->GetURI(getter_AddRefs(uri));
+            mChannel->GetURI(getter_AddRefs(uri));
             nsCString spec;
             uri->GetSpec(spec);
             LOG(("nsChannelClassifier[%p]: cancelling channel %p for %s "
-                 "with error code: %x", this, mSuspendedChannel.get(),
+                 "with error code: %x", this, mChannel.get(),
                  spec.get(), aErrorCode));
 #endif
 
             // Channel will be cancelled (page element blocked) due to tracking.
             // Do update the security state of the document and fire a security
             // change event.
             if (aErrorCode == NS_ERROR_TRACKING_URI) {
-              SetBlockedTrackingContent(mSuspendedChannel);
+              SetBlockedTrackingContent(mChannel);
             }
 
-            mSuspendedChannel->Cancel(aErrorCode);
-          }
-#ifdef DEBUG
+            mChannel->Cancel(aErrorCode);
+        }
         LOG(("nsChannelClassifier[%p]: resuming channel %p from "
-             "OnClassifyComplete", this, mSuspendedChannel.get()));
-#endif
-        mSuspendedChannel->Resume();
-        mSuspendedChannel = nullptr;
+             "OnClassifyComplete", this, mChannel.get()));
+        mChannel->Resume();
     }
+    nsresult rv;
+    nsCOMPtr<nsIHttpChannelInternal> channel = do_QueryInterface(mChannel, &rv);
+    // Even if we have cancelled the channel, we need to call
+    // ContinueBeginConnect so that we abort appropriately.
+    if (NS_SUCCEEDED(rv)) {
+        channel->ContinueBeginConnect();
+    }
+    mChannel = nullptr;
 
     return NS_OK;
 }
--- a/netwerk/base/src/nsChannelClassifier.h
+++ b/netwerk/base/src/nsChannelClassifier.h
@@ -15,26 +15,37 @@ class nsIChannel;
 class nsChannelClassifier MOZ_FINAL : public nsIURIClassifierCallback
 {
 public:
     nsChannelClassifier();
 
     NS_DECL_ISUPPORTS
     NS_DECL_NSIURICLASSIFIERCALLBACK
 
-    nsresult Start(nsIChannel *aChannel);
+    // Calls nsIURIClassifier.Classify with the principal of the given channel,
+    // and cancels the channel on a bad verdict.  If aChannel is
+    // nsIHttpChannelInternal, nsChannelClassifier must call
+    // ContinueBeginConnect once Start has successfully returned.
+    void Start(nsIChannel *aChannel);
 
 private:
-    nsCOMPtr<nsIChannel> mSuspendedChannel;
-    // Set true if the channel is on the allow list.
+    // True if the channel is on the allow list.
     bool mIsAllowListed;
+    // True if the channel has been suspended.
+    bool mSuspendedChannel;
+    nsCOMPtr<nsIChannel> mChannel;
 
     ~nsChannelClassifier() {}
+    // Caches good classifications for the channel principal.
     void MarkEntryClassified(nsresult status);
     bool HasBeenClassified(nsIChannel *aChannel);
+    // Helper function so that we ensure we call ContinueBeginConnect once
+    // Start is called. Returns NS_OK if and only if we will get a callback
+    // from the classifier service.
+    nsresult StartInternal(nsIChannel *aChannel);
     // Whether or not tracking protection should be enabled on this channel.
     nsresult ShouldEnableTrackingProtection(nsIChannel *aChannel, bool *result);
 
 public:
     // If we are blocking tracking content, update the corresponding flag in
     // the respective docshell and call nsISecurityEventSink::onSecurityChange.
     static nsresult SetBlockedTrackingContent(nsIChannel *channel);
     static nsresult NotifyTrackingProtectionDisabled(nsIChannel *aChannel);
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -1405,16 +1405,25 @@ HttpBaseChannel::RedirectTo(nsIURI *newU
   return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // HttpBaseChannel::nsIHttpChannelInternal
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
+HttpBaseChannel::ContinueBeginConnect()
+{
+  MOZ_ASSERT(XRE_GetProcessType() != GeckoProcessType_Default,
+             "The parent overrides this");
+  MOZ_ASSERT(false, "This method must be overridden");
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
 HttpBaseChannel::GetTopWindowURI(nsIURI **aTopWindowURI)
 {
   nsresult rv = NS_OK;
   nsCOMPtr<mozIThirdPartyUtil> util;
   // Only compute the top window URI once. In e10s, this must be computed in the
   // child. The parent gets the top window URI through HttpChannelOpenArgs.
   if (!mTopWindowURI) {
     util = do_GetService(THIRDPARTYUTIL_CONTRACTID);
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -183,16 +183,17 @@ public:
   NS_IMETHOD TakeAllSecurityMessages(nsCOMArray<nsISecurityConsoleMessage> &aMessages) MOZ_OVERRIDE;
   NS_IMETHOD GetResponseTimeoutEnabled(bool *aEnable) MOZ_OVERRIDE;
   NS_IMETHOD SetResponseTimeoutEnabled(bool aEnable) MOZ_OVERRIDE;
   NS_IMETHOD AddRedirect(nsIPrincipal *aRedirect) MOZ_OVERRIDE;
   NS_IMETHOD ForcePending(bool aForcePending) MOZ_OVERRIDE;
   NS_IMETHOD GetLastModifiedTime(PRTime* lastModifiedTime) MOZ_OVERRIDE;
   NS_IMETHOD ForceNoIntercept() MOZ_OVERRIDE;
   NS_IMETHOD GetTopWindowURI(nsIURI **aTopWindowURI) MOZ_OVERRIDE;
+  NS_IMETHOD ContinueBeginConnect();
 
   inline void CleanRedirectCacheChainIfNecessary()
   {
       mRedirectedCachekeys = nullptr;
   }
   NS_IMETHOD HTTPUpgrade(const nsACString & aProtocolName,
                          nsIHttpUpgradeListener *aListener) MOZ_OVERRIDE;
 
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -17,16 +17,17 @@
 #include "nsICacheEntry.h"
 #include "nsICryptoHash.h"
 #include "nsINetworkInterceptController.h"
 #include "nsIStringBundle.h"
 #include "nsIStreamListenerTee.h"
 #include "nsISeekableStream.h"
 #include "nsILoadGroupChild.h"
 #include "nsIProtocolProxyService2.h"
+#include "nsIURIClassifier.h"
 #include "nsMimeTypes.h"
 #include "nsNetUtil.h"
 #include "prprf.h"
 #include "prnetdb.h"
 #include "nsEscape.h"
 #include "nsStreamUtils.h"
 #include "nsIOService.h"
 #include "nsDNSPrefetch.h"
@@ -34,16 +35,17 @@
 #include "nsIRedirectResultListener.h"
 #include "mozilla/TimeStamp.h"
 #include "nsError.h"
 #include "nsPrintfCString.h"
 #include "nsAlgorithm.h"
 #include "GeckoProfiler.h"
 #include "nsIConsoleService.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/Preferences.h"
 #include "mozilla/VisualEventTracer.h"
 #include "nsISSLSocketControl.h"
 #include "sslt.h"
 #include "nsContentUtils.h"
 #include "nsIClassOfService.h"
 #include "nsIPermissionManager.h"
 #include "nsIPrincipal.h"
 #include "nsIScriptSecurityManager.h"
@@ -235,16 +237,17 @@ nsHttpChannel::nsHttpChannel()
     , mCacheEntryIsReadOnly(false)
     , mCacheEntryIsWriteOnly(false)
     , mCacheEntriesToWaitFor(0)
     , mHasQueryString(0)
     , mConcurentCacheAccess(0)
     , mIsPartialRequest(0)
     , mHasAutoRedirectVetoNotifier(0)
     , mPushedStream(nullptr)
+    , mLocalBlocklist(false)
     , mDidReval(false)
 {
     LOG(("Creating nsHttpChannel [this=%p]\n", this));
     mChannelCreationTime = PR_Now();
     mChannelCreationTimestamp = TimeStamp::Now();
 }
 
 nsHttpChannel::~nsHttpChannel()
@@ -321,17 +324,17 @@ nsHttpChannel::Connect()
     if (!net_IsValidHostName(nsDependentCString(mConnectionInfo->Host())))
         return NS_ERROR_UNKNOWN_HOST;
 
     // Finalize ConnectionInfo flags before SpeculativeConnect
     mConnectionInfo->SetAnonymous((mLoadFlags & LOAD_ANONYMOUS) != 0);
     mConnectionInfo->SetPrivate(mPrivateBrowsing);
     mConnectionInfo->SetNoSpdy(mCaps & NS_HTTP_DISALLOW_SPDY);
 
-    // Consider opening a TCP connection right away
+    // Consider opening a TCP connection right away.
     SpeculativeConnect();
 
     // Don't allow resuming when cache must be used
     if (mResuming && (mLoadFlags & LOAD_ONLY_FROM_CACHE)) {
         LOG(("Resuming from cache is not supported yet"));
         return NS_ERROR_DOCUMENT_NOT_CACHED;
     }
 
@@ -428,21 +431,21 @@ nsHttpChannel::ContinueConnect()
 }
 
 void
 nsHttpChannel::SpeculativeConnect()
 {
     // Before we take the latency hit of dealing with the cache, try and
     // get the TCP (and SSL) handshakes going so they can overlap.
 
-    // don't speculate on uses of the offline application cache,
-    // if we are offline, when doing http upgrade (i.e. websockets bootstrap),
-    // or if we can't do keep-alive (because then we couldn't reuse
-    // the speculative connection anyhow).
-    if (mApplicationCache || gIOService->IsOffline() ||
+    // don't speculate if we are on a local blocklist, on uses of the offline
+    // application cache, if we are offline, when doing http upgrade (i.e.
+    // websockets bootstrap), or if we can't do keep-alive (because then we
+    // couldn't reuse the speculative connection anyhow).
+    if (mLocalBlocklist || mApplicationCache || gIOService->IsOffline() ||
         mUpgradeProtocolCallback || !(mCaps & NS_HTTP_ALLOW_KEEPALIVE))
         return;
 
     // LOAD_ONLY_FROM_CACHE and LOAD_NO_NETWORK_IO must not hit network.
     // LOAD_FROM_CACHE and LOAD_CHECK_OFFLINE_CACHE are unlikely to hit network,
     // so skip preconnects for them.
     if (mLoadFlags & (LOAD_ONLY_FROM_CACHE | LOAD_FROM_CACHE |
                       LOAD_NO_NETWORK_IO | LOAD_CHECK_OFFLINE_CACHE))
@@ -4868,16 +4871,31 @@ nsHttpChannel::BeginConnect()
     // notify "http-on-modify-request" observers
     CallOnModifyRequestObservers();
 
     // Check to see if we should redirect this channel elsewhere by
     // nsIHttpChannel.redirectTo API request
     if (mAPIRedirectToURI) {
         return AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect);
     }
+    // Check to see if this principal exists on local blocklists.
+    if (mLoadFlags & LOAD_CLASSIFY_URI) {
+        nsCOMPtr<nsIURIClassifier> classifier = do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID);
+        if (classifier) {
+            nsCOMPtr<nsIPrincipal> principal = GetPrincipal(false);
+            bool tp = Preferences::GetBool("privacy.trackingprotection.enabled",
+                                           false);
+            nsresult response = NS_OK;
+            classifier->ClassifyLocal(principal, tp, &response);
+            if (NS_FAILED(response)) {
+                LOG(("nsHttpChannel::Found principal on local blocklist [this=%p]", this));
+                mLocalBlocklist = true;
+            }
+        }
+    }
 
     // If mTimingEnabled flag is not set after OnModifyRequest() then
     // clear the already recorded AsyncOpen value for consistency.
     if (!mTimingEnabled)
         mAsyncOpenTime = TimeStamp();
 
     // when proxying only use the pipeline bit if ProxyPipelining() allows it.
     if (!mConnectionInfo->UsingConnect() && mConnectionInfo->UsingHttpProxy()) {
@@ -4887,17 +4905,17 @@ nsHttpChannel::BeginConnect()
     }
 
     // if this somehow fails we can go on without it
     gHttpHandler->AddConnectionHeader(&mRequestHead.Headers(), mCaps);
 
     if (mLoadFlags & VALIDATE_ALWAYS || BYPASS_LOCAL_CACHE(mLoadFlags))
         mCaps |= NS_HTTP_REFRESH_DNS;
 
-    if (!mConnectionInfo->UsingHttpProxy() &&
+    if (!mLocalBlocklist && !mConnectionInfo->UsingHttpProxy() &&
         !(mLoadFlags & (LOAD_NO_NETWORK_IO | LOAD_ONLY_FROM_CACHE))) {
         // Start a DNS lookup very early in case the real open is queued the DNS can
         // happen in parallel. Do not do so in the presence of an HTTP proxy as
         // all lookups other than for the proxy itself are done by the proxy.
         // Also we don't do a lookup if the LOAD_NO_NETWORK_IO or
         // LOAD_ONLY_FROM_CACHE flags are set.
         //
         // We keep the DNS prefetch object around so that we can retrieve
@@ -4931,37 +4949,27 @@ nsHttpChannel::BeginConnect()
     // Force-Reload should reset the persistent connection pool for this host
     if (mLoadFlags & LOAD_FRESH_CONNECTION) {
         // just the initial document resets the whole pool
         if (mLoadFlags & LOAD_INITIAL_DOCUMENT_URI) {
             gHttpHandler->ConnMgr()->DoShiftReloadConnectionCleanup(mConnectionInfo);
         }
         mCaps &= ~NS_HTTP_ALLOW_PIPELINING;
     }
-
-    // We may have been cancelled already, either by on-modify-request
-    // listeners or by load group observers; in that case, we should
-    // not send the request to the server
-    if (mCanceled)
-        rv = mStatus;
-    else
-        rv = Connect();
-    if (NS_FAILED(rv)) {
-        LOG(("Calling AsyncAbort [rv=%x mCanceled=%i]\n", rv, mCanceled));
-        CloseCacheEntry(true);
-        AsyncAbort(rv);
-    } else if (mLoadFlags & LOAD_CLASSIFY_URI) {
-        nsRefPtr<nsChannelClassifier> classifier = new nsChannelClassifier();
-        rv = classifier->Start(this);
-        if (NS_FAILED(rv)) {
-            Cancel(rv);
-            return rv;
-        }
-    }
-
+    if (mCanceled || !mLocalBlocklist) {
+        return ContinueBeginConnect();
+    }
+    MOZ_ASSERT(!mCanceled && mLocalBlocklist);
+    // nsChannelClassifier must call ContinueBeginConnect after optionally
+    // cancelling the channel once we have a remote verdict. We call a concrete
+    // class instead of an nsI* that might be overridden.
+    nsRefPtr<nsChannelClassifier> classifier = new nsChannelClassifier();
+    LOG(("nsHttpChannel::Starting nsChannelClassifier %p [this=%p]",
+         classifier.get(), this));
+    classifier->Start(this);
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsIHttpChannelInternal
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
@@ -4989,16 +4997,40 @@ nsHttpChannel::SetPriority(int32_t value
         return NS_OK;
     mPriority = newValue;
     if (mTransaction)
         gHttpHandler->RescheduleTransaction(mTransaction, mPriority);
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
+// nsHttpChannel::nsIHttpChannelInternal
+//-----------------------------------------------------------------------------
+NS_IMETHODIMP
+nsHttpChannel::ContinueBeginConnect()
+{
+    LOG(("nsHttpChannel::ContinueBeginConnect [this=%p]", this));
+    nsresult rv;
+    // We may have been cancelled already, either by on-modify-request
+    // listeners or load group observers or nsChannelClassifier; in that case,
+    // we should not send the request to the server
+    if (mCanceled) {
+        rv = mStatus;
+    } else {
+        rv = Connect();
+    }
+    if (NS_FAILED(rv)) {
+        LOG(("Calling AsyncAbort [rv=%x mCanceled=%i]\n", rv, mCanceled));
+        CloseCacheEntry(true);
+        AsyncAbort(rv);
+    }
+    return rv;
+}
+
+//-----------------------------------------------------------------------------
 // HttpChannel::nsIClassOfService
 //-----------------------------------------------------------------------------
 NS_IMETHODIMP
 nsHttpChannel::SetClassFlags(uint32_t inFlags)
 {
     mClassOfService = inFlags;
     return NS_OK;
 }
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -114,16 +114,17 @@ public:
     NS_IMETHOD Cancel(nsresult status) MOZ_OVERRIDE;
     NS_IMETHOD Suspend() MOZ_OVERRIDE;
     NS_IMETHOD Resume() MOZ_OVERRIDE;
     // nsIChannel
     NS_IMETHOD GetSecurityInfo(nsISupports **aSecurityInfo) MOZ_OVERRIDE;
     NS_IMETHOD AsyncOpen(nsIStreamListener *listener, nsISupports *aContext) MOZ_OVERRIDE;
     // nsIHttpChannelInternal
     NS_IMETHOD SetupFallbackChannel(const char *aFallbackKey) MOZ_OVERRIDE;
+    NS_IMETHOD ContinueBeginConnect();
     // nsISupportsPriority
     NS_IMETHOD SetPriority(int32_t value) MOZ_OVERRIDE;
     // nsIClassOfService
     NS_IMETHOD SetClassFlags(uint32_t inFlags) MOZ_OVERRIDE;
     NS_IMETHOD AddClassFlags(uint32_t inFlags) MOZ_OVERRIDE;
     NS_IMETHOD ClearClassFlags(uint32_t inFlags) MOZ_OVERRIDE;
 
     // nsIResumableChannel
@@ -460,16 +461,19 @@ private:
     uint32_t                          mHasAutoRedirectVetoNotifier : 1;
 
     nsTArray<nsContinueRedirectionFunc> mRedirectFuncStack;
 
     // Needed for accurate DNS timing
     nsRefPtr<nsDNSPrefetch>           mDNSPrefetch;
 
     Http2PushedStream                 *mPushedStream;
+    // True if the channel's principal was found on a phishing, malware, or
+    // tracking (if tracking protection is enabled) blocklist
+    bool                              mLocalBlocklist;
 
     nsresult WaitForRedirectCallback();
     void PushRedirectAsyncFunc(nsContinueRedirectionFunc func);
     void PopRedirectAsyncFunc(nsContinueRedirectionFunc func);
 
     nsCString mUsername;
 
 protected:
--- a/netwerk/protocol/http/nsIHttpChannelInternal.idl
+++ b/netwerk/protocol/http/nsIHttpChannelInternal.idl
@@ -219,9 +219,15 @@ interface nsIHttpChannelInternal : nsISu
      * interception and proceed immediately to the network/cache.
      */
     void forceNoIntercept();
 
     /**
      * The URI of the top-level window that's associated with this channel.
      */
     readonly attribute nsIURI topWindowURI;
+
+    /**
+     * Used only by nsChannelClassifier to resume connecting or abort the
+     * channel after a remote classification verdict.
+     */
+    void continueBeginConnect();
 };