From d97a1a7436817796438e9fdba237c95af59201e6 Mon Sep 17 00:00:00 2001 From: Dimi Lee Date: Thu, 21 Mar 2019 07:29:48 +0000 Subject: [PATCH] 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 --HG-- extra : moz-landing-system : lando --- netwerk/base/nsBaseChannel.cpp | 2 +- netwerk/base/nsNetUtil.cpp | 56 +++++++++++++++++++++++++ netwerk/base/nsNetUtil.h | 5 +++ netwerk/protocol/http/HttpBaseChannel.h | 3 ++ netwerk/protocol/http/nsHttpChannel.cpp | 2 +- 5 files changed, 66 insertions(+), 2 deletions(-) diff --git a/netwerk/base/nsBaseChannel.cpp b/netwerk/base/nsBaseChannel.cpp index 09118e73a6a7..2614461516f9 100644 --- a/netwerk/base/nsBaseChannel.cpp +++ b/netwerk/base/nsBaseChannel.cpp @@ -332,7 +332,7 @@ void nsBaseChannel::ClassifyURI() { return; } - if (mLoadFlags & LOAD_CLASSIFY_URI) { + if (NS_ShouldClassifyChannel(this)) { RefPtr classifier = new nsChannelClassifier(this); if (classifier) { classifier->Start(); diff --git a/netwerk/base/nsNetUtil.cpp b/netwerk/base/nsNetUtil.cpp index 982ab8f931f4..634ef5360ad1 100644 --- a/netwerk/base/nsNetUtil.cpp +++ b/netwerk/base/nsNetUtil.cpp @@ -3011,6 +3011,62 @@ bool NS_IsOffline() { 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 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 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 { diff --git a/netwerk/base/nsNetUtil.h b/netwerk/base/nsNetUtil.h index 1cafde95ad9f..60cf02aab19d 100644 --- a/netwerk/base/nsNetUtil.h +++ b/netwerk/base/nsNetUtil.h @@ -937,6 +937,11 @@ 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 { diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h index c35eba0a70b5..51badda07357 100644 --- a/netwerk/protocol/http/HttpBaseChannel.h +++ b/netwerk/protocol/http/HttpBaseChannel.h @@ -696,6 +696,9 @@ class HttpBaseChannel : public nsHashPropertyBag, 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; diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 16ffffa68afb..8faddaa358cf 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -6599,7 +6599,7 @@ nsresult nsHttpChannel::BeginConnect() { return mStatus; } - if (!(mLoadFlags & LOAD_CLASSIFY_URI)) { + if (!NS_ShouldClassifyChannel(this)) { MaybeStartDNSPrefetch(); return ContinueBeginConnectWithResult(); }