Bug 977236: Use referrer for application reputation checks (r=gcp)
☠☠ backed out by db1dcce906fe ☠ ☠
authorMonica Chew <mmc@mozilla.com>
Wed, 12 Mar 2014 20:31:51 -0700
changeset 190696 cd5ce2a9aceb0f63189172920e3aacbbf33080bc
parent 190695 7fbb7b36d87c0b11aac97afd7fd9214f5c41991c
child 190697 047a0b9a8fb800695d131867cfd5a164fcdbd4cf
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs977236
milestone30.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 977236: Use referrer for application reputation checks (r=gcp)
toolkit/components/downloads/ApplicationReputation.cpp
toolkit/components/downloads/nsIApplicationReputation.idl
toolkit/components/downloads/test/unit/data/block_digest.chunk
toolkit/components/downloads/test/unit/test_app_rep.js
toolkit/components/downloads/test/unit/xpcshell.ini
toolkit/components/jsdownloads/src/DownloadIntegration.jsm
--- a/toolkit/components/downloads/ApplicationReputation.cpp
+++ b/toolkit/components/downloads/ApplicationReputation.cpp
@@ -108,16 +108,18 @@ private:
   nsCOMPtr<nsIApplicationReputationQuery> mQuery;
 
   // The callback with which to report the verdict.
   nsCOMPtr<nsIApplicationReputationCallback> mCallback;
 
   // 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;
 
   // The protocol buffer used to store signature information extracted using
   // the Windows Authenticode API, if the binary is signed.
   ClientDownloadRequest_SignatureInfo mSignatureInfo;
 
@@ -194,55 +196,55 @@ public:
 
   // Constructor and destructor
   PendingDBLookup(PendingLookup* aPendingLookup);
   ~PendingDBLookup();
 
   // 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.
   enum LIST_TYPES {
     ALLOW_LIST = 0,
     BLOCK_LIST = 1,
     NO_LIST = 2,
   };
 
   nsCString mSpec;
-  bool mAllowListOnly;
+  bool mAllowlistOnly;
   nsRefPtr<PendingLookup> mPendingLookup;
   nsresult LookupSpecInternal(const nsACString& aSpec);
 };
 
 NS_IMPL_ISUPPORTS1(PendingDBLookup,
                    nsIUrlClassifierCallback)
 
 PendingDBLookup::PendingDBLookup(PendingLookup* aPendingLookup) :
-  mAllowListOnly(false),
+  mAllowlistOnly(false),
   mPendingLookup(aPendingLookup)
 {
   LOG(("Created pending DB lookup [this = %p]", this));
 }
 
 PendingDBLookup::~PendingDBLookup()
 {
   LOG(("Destroying pending DB lookup [this = %p]", this));
   mPendingLookup = nullptr;
 }
 
 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"));
     return mPendingLookup->OnComplete(false, NS_OK);
   }
   // LookupSpecInternal has called nsIUrlClassifierCallback.lookup, which is
   // guaranteed to call HandleEvent.
   return rv;
@@ -275,33 +277,33 @@ PendingDBLookup::LookupSpecInternal(cons
 }
 
 NS_IMETHODIMP
 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)) {
     Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_LOCAL, ALLOW_LIST);
     LOG(("Found principal %s on allowlist [this = %p]", mSpec.get(), this));
     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();
 }
 
 NS_IMPL_ISUPPORTS2(PendingLookup,
                    nsIStreamListener,
                    nsIRequestObserver)
@@ -319,28 +321,36 @@ PendingLookup::~PendingLookup()
   LOG(("Destroying pending lookup [this = %p]", this));
 }
 
 nsresult
 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);
+    // 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);
+    }
+  }
+  if (index >= 0) {
     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;
-    }
-    return lookup->LookupSpec(spec, allowListOnly);
+    return lookup->LookupSpec(spec, allowlistOnly);
   }
   // There are no more URIs to check against local list, so send the remote
   // query if we can.
   // Revert to just ifdef XP_WIN when remote lookups are enabled (bug 933432)
 #if 0
   nsresult rv = SendRemoteQuery();
   if (NS_FAILED(rv)) {
     return OnComplete(false, rv);
@@ -501,17 +511,26 @@ PendingLookup::DoLookupInternal()
   // We want to check the target URI against the local lists.
   nsCOMPtr<nsIURI> uri = nullptr;
   nsresult rv = mQuery->GetSourceURI(getter_AddRefs(uri));
   NS_ENSURE_SUCCESS(rv, rv);
 
   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.
   nsCOMPtr<nsIArray> sigArray = nullptr;
   rv = mQuery->GetSignatureInfo(getter_AddRefs(sigArray));
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (sigArray) {
--- a/toolkit/components/downloads/nsIApplicationReputation.idl
+++ b/toolkit/components/downloads/nsIApplicationReputation.idl
@@ -47,16 +47,21 @@ interface nsIApplicationReputationServic
 [scriptable, uuid(5a054991-e489-4a1c-a0aa-ea7c69b20e3d)]
 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
    * is not set by the caller, it will be passed as an empty string but the
    * query won't produce any useful information.
    */
   readonly attribute AString suggestedFileName;
 
   /*
new file mode 100644
--- /dev/null
+++ b/toolkit/components/downloads/test/unit/data/block_digest.chunk
@@ -0,0 +1,2 @@
+a:5:32:37
+,AJ,AJ8Wbb_e;OτCV
\ No newline at end of file
--- a/toolkit/components/downloads/test/unit/test_app_rep.js
+++ b/toolkit/components/downloads/test/unit/test_app_rep.js
@@ -129,44 +129,85 @@ add_test(function test_local_list() {
     response.setStatusLine(request.httpVersion, 200, "OK");
     response.bodyOutputStream.write(blob, blob.length);
   });
 
   let streamUpdater = Cc["@mozilla.org/url-classifier/streamupdater;1"]
     .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.
   function updateSuccess(aEvent) {
     // Timeout of n:1000 is constructed in processUpdateRequest above and
     // passed back in the callback in nsIUrlClassifierStreamUpdater on success.
     do_check_eq("1000", aEvent);
     do_print("All data processed");
     run_next_test();
   }
   // Just throw if we ever get an update or download error.
   function handleError(aEvent) {
     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();
+  });
+});
--- a/toolkit/components/downloads/test/unit/xpcshell.ini
+++ b/toolkit/components/downloads/test/unit/xpcshell.ini
@@ -1,16 +1,17 @@
 [DEFAULT]
 head = head_download_manager.js
 tail = tail_download_manager.js
 firefox-appdir = browser
 support-files =
   downloads_manifest.js
   test_downloads.manifest
   data/digest.chunk
+  data/block_digest.chunk
   data/signed_win.exe
 
 [test_app_rep.js]
 [test_app_rep_windows.js]
 run-if = os == "win"
 [test_bug_382825.js]
 [test_bug_384744.js]
 [test_bug_395092.js]
--- a/toolkit/components/jsdownloads/src/DownloadIntegration.jsm
+++ b/toolkit/components/jsdownloads/src/DownloadIntegration.jsm
@@ -508,27 +508,32 @@ this.DownloadIntegration = {
   shouldBlockForReputationCheck: function (aDownload) {
     if (this.dontCheckApplicationReputation) {
       return Promise.resolve(this.shouldBlockInTestForApplicationReputation);
     }
     let hash;
     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);
     }
     if (!hash || !sigInfo) {
       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 },
       function onComplete(aShouldBlock, aRv) {
         deferred.resolve(aShouldBlock);
       });
     return deferred.promise;
   },