Bug 977236 - Use referrer for application reputation checks. r=gcp

toolkit/components/downloads/ApplicationReputation.cpp

@@ -113,6 +113,8 @@ private:
   // An array of strings created from certificate information used to whitelist
   // the downloaded file.
   nsTArray<nsCString> mAllowlistSpecs;
+  // The source URI of the download, the referrer and possibly any redirects.
+  nsTArray<nsCString> mAnylistSpecs;
 
   // When we started this query
   TimeStamp mStartTime;
@@ -199,7 +201,7 @@ public:
   // Look up the given URI in the safebrowsing DBs, optionally on both the allow
   // list and the blocklist. If there is a match, call
   // PendingLookup::OnComplete. Otherwise, call PendingLookup::LookupNext.
-  nsresult LookupSpec(const nsACString& aSpec, bool aAllowListOnly);
+  nsresult LookupSpec(const nsACString& aSpec, bool aAllowlistOnly);
 private:
   // The download appeared on the allowlist, blocklist, or no list (and thus
   // could trigger a remote query.
@@ -210,7 +212,7 @@ private:
   };
 
   nsCString mSpec;
-  bool mAllowListOnly;
+  bool mAllowlistOnly;
   nsRefPtr<PendingLookup> mPendingLookup;
   nsresult LookupSpecInternal(const nsACString& aSpec);
 };
@@ -219,7 +221,7 @@ NS_IMPL_ISUPPORTS1(PendingDBLookup,
                    nsIUrlClassifierCallback)
 
 PendingDBLookup::PendingDBLookup(PendingLookup* aPendingLookup) :
-  mAllowListOnly(false),
+  mAllowlistOnly(false),
   mPendingLookup(aPendingLookup)
 {
   LOG(("Created pending DB lookup [this = %p]", this));
@@ -233,11 +235,11 @@ PendingDBLookup::~PendingDBLookup()
 
 nsresult
 PendingDBLookup::LookupSpec(const nsACString& aSpec,
-                            bool aAllowListOnly)
+                            bool aAllowlistOnly)
 {
   LOG(("Checking principal %s", aSpec.Data()));
   mSpec = aSpec;
-  mAllowListOnly = aAllowListOnly;
+  mAllowlistOnly = aAllowlistOnly;
   nsresult rv = LookupSpecInternal(aSpec);
   if (NS_FAILED(rv)) {
     LOG(("Error in LookupSpecInternal"));
@@ -280,7 +282,15 @@ PendingDBLookup::HandleEvent(const nsACString& tables)
   // HandleEvent is guaranteed to call either:
   // 1) PendingLookup::OnComplete if the URL can be classified locally, or
   // 2) PendingLookup::LookupNext if the URL can be cannot classified locally.
-  // Allow listing trumps block listing.
+  // Blocklisting trumps allowlisting.
+  nsAutoCString blockList;
+  Preferences::GetCString(PREF_DOWNLOAD_BLOCK_TABLE, &blockList);
+  if (!mAllowlistOnly && FindInReadable(tables, blockList)) {
+    Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_LOCAL, BLOCK_LIST);
+    LOG(("Found principal %s on blocklist [this = %p]", mSpec.get(), this));
+    return mPendingLookup->OnComplete(true, NS_OK);
+  }
+
   nsAutoCString allowList;
   Preferences::GetCString(PREF_DOWNLOAD_ALLOW_TABLE, &allowList);
   if (FindInReadable(tables, allowList)) {
@@ -289,14 +299,6 @@ PendingDBLookup::HandleEvent(const nsACString& tables)
     return mPendingLookup->OnComplete(false, NS_OK);
   }
 
-  nsAutoCString blockList;
-  Preferences::GetCString(PREF_DOWNLOAD_BLOCK_TABLE, &blockList);
-  if (!mAllowListOnly && FindInReadable(tables, blockList)) {
-    Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_LOCAL, BLOCK_LIST);
-    LOG(("Found principal %s on blocklist [this = %p]", mSpec.get(), this));
-    return mPendingLookup->OnComplete(true, NS_OK);
-  }
-
   LOG(("Didn't find principal %s on any list [this = %p]", mSpec.get(), this));
   Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_LOCAL, NO_LIST);
   return mPendingLookup->LookupNext();
@@ -324,18 +326,26 @@ PendingLookup::LookupNext()
 {
   // We must call LookupNext or SendRemoteQuery upon return.
   // Look up all of the URLs that could whitelist this download.
-  int index = mAllowlistSpecs.Length() - 1;
+  // Blacklist first.
+  int index = mAnylistSpecs.Length() - 1;
+  nsCString spec;
+  bool allowlistOnly = false;
   if (index >= 0) {
-    nsCString spec = mAllowlistSpecs[index];
-    mAllowlistSpecs.RemoveElementAt(index);
-    nsRefPtr<PendingDBLookup> lookup(new PendingDBLookup(this));
-    bool allowListOnly = true;
-    if (index == 0) {
-      // The last URI is the target URI, which may be used for blacklisting as
-      // well as whitelisting.
-      allowListOnly = false;
+    // Check the source URI and referrer.
+    spec = mAnylistSpecs[index];
+    mAnylistSpecs.RemoveElementAt(index);
+  } else {
+    // Check the allowlists next.
+    index = mAllowlistSpecs.Length() - 1;
+    if (index >= 0) {
+      allowlistOnly = true;
+      spec = mAllowlistSpecs[index];
+      mAllowlistSpecs.RemoveElementAt(index);
     }
-    return lookup->LookupSpec(spec, allowListOnly);
+  }
+  if (index >= 0) {
+    nsRefPtr<PendingDBLookup> lookup(new PendingDBLookup(this));
+    return lookup->LookupSpec(spec, allowlistOnly);
   }
   // There are no more URIs to check against local list, so send the remote
   // query if we can.
@@ -506,7 +516,16 @@ PendingLookup::DoLookupInternal()
   nsCString spec;
   rv = uri->GetSpec(spec);
   NS_ENSURE_SUCCESS(rv, rv);
-  mAllowlistSpecs.AppendElement(spec);
+  mAnylistSpecs.AppendElement(spec);
+
+  nsCOMPtr<nsIURI> referrer = nullptr;
+  rv = mQuery->GetReferrerURI(getter_AddRefs(referrer));
+  if (referrer) {
+    nsCString spec;
+    rv = referrer->GetSpec(spec);
+    NS_ENSURE_SUCCESS(rv, rv);
+    mAnylistSpecs.AppendElement(spec);
+  }
 
   // Extract the signature and parse certificates so we can use it to check
   // whitelists.

toolkit/components/downloads/nsIApplicationReputation.idl

@@ -44,13 +44,18 @@ interface nsIApplicationReputationService : nsISupports {
  * downloaded file. nsIApplicationReputationService.Start() may only be called
  * once with a single query.
  */
-[scriptable, uuid(5a054991-e489-4a1c-a0aa-ea7c69b20e3d)]
+[scriptable, uuid(2c781cbe-ab0c-4c53-b06e-f0cb56f8a30b)]
 interface nsIApplicationReputationQuery : nsISupports {
   /*
    * The nsIURI from which the file was downloaded. This may not be null.
    */
   readonly attribute nsIURI sourceURI;
 
+  /*
+   * The reference, if any.
+   */
+  readonly attribute nsIURI referrerURI;
+
   /*
    * The target filename for the downloaded file, as inferred from the source
    * URI or provided by the Content-Disposition attachment file name. If this

toolkit/components/downloads/test/unit/data/block_digest.chunk

@@ -0,0 +1,2 @@
+a:5:32:37
+,AÎJ
,AÎJ„ä8æW´<57>bbòñ_e‹;OÏÏ„CVù 

toolkit/components/downloads/test/unit/test_app_rep.js

@@ -134,8 +134,10 @@ add_test(function test_local_list() {
     .getService(Ci.nsIUrlClassifierStreamUpdater);
   streamUpdater.updateUrl = "http://localhost:4444/downloads";
 
-  // Load up some update chunks for the safebrowsing server to serve. This
-  // particular chunk contains the hash of whitelisted.com/.
+  // Load up some update chunks for the safebrowsing server to serve.
+  // This chunk contains the hash of whitelisted.com/.
+  registerTableUpdate("goog-badbinurl-shavar", "data/block_digest.chunk");
+  // This chunk contains the hash of blocklisted.com/.
   registerTableUpdate("goog-downloadwhite-digest256", "data/digest.chunk");
 
   // Download some updates, and don't continue until the downloads are done.
@@ -151,22 +153,61 @@ add_test(function test_local_list() {
     do_throw("We didn't download or update correctly: " + aEvent);
   }
   streamUpdater.downloadUpdates(
-    "goog-downloadwhite-digest256",
-    "goog-downloadwhite-digest256;\n",
+    "goog-downloadwhite-digest256,goog-badbinurl-shavar",
+    "goog-downloadwhite-digest256,goog-badbinurl-shavar;\n",
     updateSuccess, handleError, handleError);
 });
 
-// After being whitelisted, we shouldn't throw.
-add_test(function test_local_whitelist() {
+add_test(function test_unlisted() {
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
                              "http://localhost:4444/download");
   gAppRep.queryReputation({
-    sourceURI: createURI("http://whitelisted.com"),
+    sourceURI: createURI("http://example.com"),
     fileSize: 12,
   }, function onComplete(aShouldBlock, aStatus) {
-    // We would get garbage if this query made it to the remote server.
     do_check_eq(Cr.NS_OK, aStatus);
     do_check_false(aShouldBlock);
     run_next_test();
   });
 });
+
+add_test(function test_local_blacklist() {
+  Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
+                             "http://localhost:4444/download");
+  gAppRep.queryReputation({
+    sourceURI: createURI("http://blocklisted.com"),
+    fileSize: 12,
+  }, function onComplete(aShouldBlock, aStatus) {
+    do_check_eq(Cr.NS_OK, aStatus);
+    do_check_true(aShouldBlock);
+    run_next_test();
+  });
+});
+
+add_test(function test_referer_blacklist() {
+  Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
+                             "http://localhost:4444/download");
+  gAppRep.queryReputation({
+    sourceURI: createURI("http://example.com"),
+    referrerURI: createURI("http://blocklisted.com"),
+    fileSize: 12,
+  }, function onComplete(aShouldBlock, aStatus) {
+    do_check_eq(Cr.NS_OK, aStatus);
+    do_check_true(aShouldBlock);
+    run_next_test();
+  });
+});
+
+add_test(function test_blocklist_trumps_allowlist() {
+  Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
+                             "http://localhost:4444/download");
+  gAppRep.queryReputation({
+    sourceURI: createURI("http://whitelisted.com"),
+    referrerURI: createURI("http://blocklisted.com"),
+    fileSize: 12,
+  }, function onComplete(aShouldBlock, aStatus) {
+    do_check_eq(Cr.NS_OK, aStatus);
+    do_check_true(aShouldBlock);
+    run_next_test();
+  });
+});

toolkit/components/downloads/test/unit/xpcshell.ini

@@ -6,6 +6,7 @@ support-files =
   downloads_manifest.js
   test_downloads.manifest
   data/digest.chunk
+  data/block_digest.chunk
   data/signed_win.exe
 
 [test_app_rep.js]

toolkit/components/jsdownloads/src/DownloadIntegration.jsm

@@ -513,7 +513,7 @@ this.DownloadIntegration = {
     let sigInfo;
     try {
       hash = aDownload.saver.getSha256Hash();
-       sigInfo = aDownload.saver.getSignatureInfo();
+      sigInfo = aDownload.saver.getSignatureInfo();
     } catch (ex) {
       // Bail if DownloadSaver doesn't have a hash.
       return Promise.resolve(false);
@@ -522,8 +522,13 @@ this.DownloadIntegration = {
       return Promise.resolve(false);
     }
     let deferred = Promise.defer();
+    let aReferrer = null;
+    if (aDownload.source.referrer) {
+      aReferrer: NetUtil.newURI(aDownload.source.referrer);
+    }
     gApplicationReputationService.queryReputation({
       sourceURI: NetUtil.newURI(aDownload.source.url),
+      referrerURI: aReferrer,
       fileSize: aDownload.currentBytes,
       sha256Hash: hash,
       signatureInfo: sigInfo },