Bug 933432: Enable remote lookups for application on Windows (r=gcp)
authorMonica Chew <mmc@mozilla.com>
Thu, 17 Apr 2014 14:01:47 -0700
changeset 197709 8d25da069190f1fbefac5e1fb26121c40d565938
parent 197708 05d2713df1fdf916082ad6e0ad9a78a68ac81c0d
child 197710 7c18257d0163e793150777c8e2812cb660501adf
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs933432
milestone31.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 933432: Enable remote lookups for application on Windows (r=gcp)
toolkit/components/downloads/ApplicationReputation.cpp
toolkit/components/downloads/test/unit/test_app_rep_windows.js
--- a/toolkit/components/downloads/ApplicationReputation.cpp
+++ b/toolkit/components/downloads/ApplicationReputation.cpp
@@ -44,19 +44,17 @@
 
 using mozilla::Preferences;
 using mozilla::TimeStamp;
 using mozilla::Telemetry::Accumulate;
 using safe_browsing::ClientDownloadRequest;
 using safe_browsing::ClientDownloadRequest_SignatureInfo;
 using safe_browsing::ClientDownloadRequest_CertificateChain;
 
-// Preferences that we need to initialize the query. We may need another
-// preference than browser.safebrowsing.malware.enabled, or simply use
-// browser.safebrowsing.appRepURL. See bug 887041.
+// Preferences that we need to initialize the query.
 #define PREF_SB_APP_REP_URL "browser.safebrowsing.appRepURL"
 #define PREF_SB_MALWARE_ENABLED "browser.safebrowsing.malware.enabled"
 #define PREF_GENERAL_LOCALE "general.useragent.locale"
 #define PREF_DOWNLOAD_BLOCK_TABLE "urlclassifier.downloadBlockTable"
 #define PREF_DOWNLOAD_ALLOW_TABLE "urlclassifier.downloadAllowTable"
 
 // NSPR_LOG_MODULES=ApplicationReputation:5
 #if defined(PR_LOGGING)
@@ -123,16 +121,19 @@ private:
   // the Windows Authenticode API, if the binary is signed.
   ClientDownloadRequest_SignatureInfo mSignatureInfo;
 
   // The response from the application reputation query. This is read in chunks
   // as part of our nsIStreamListener implementation and may contain embedded
   // NULLs.
   nsCString mResponse;
 
+  // Returns true if the file is likely to be binary on Windows.
+  bool IsBinaryFile();
+
   // Clean up and call the callback. PendingLookup must not be used after this
   // function is called.
   nsresult OnComplete(bool shouldBlock, nsresult rv);
 
   // Wrapper function for nsIStreamListener.onStopRequest to make it easy to
   // guarantee calling the callback
   nsresult OnStopRequestInternal(nsIRequest *aRequest,
                                  nsISupports *aContext,
@@ -328,16 +329,44 @@ PendingLookup::PendingLookup(nsIApplicat
   LOG(("Created pending lookup [this = %p]", this));
 }
 
 PendingLookup::~PendingLookup()
 {
   LOG(("Destroying pending lookup [this = %p]", this));
 }
 
+bool
+PendingLookup::IsBinaryFile()
+{
+  nsString fileName;
+  nsresult rv = mQuery->GetSuggestedFileName(fileName);
+  if (NS_FAILED(rv)) {
+    return false;
+  }
+  return
+    // Executable extensions for MS Windows, from
+    // https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/safe_browsing/download_protection_util.cc&l=14
+    StringEndsWith(fileName, NS_LITERAL_STRING(".apk")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".bas")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".bat")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".cab")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".cmd")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".com")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".exe")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".hta")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".msi")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".pif")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".reg")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".scr")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".vb")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".vbs")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".zip"));
+}
+
 nsresult
 PendingLookup::LookupNext()
 {
   // We must call LookupNext or SendRemoteQuery upon return.
   // Look up all of the URLs that could whitelist this download.
   // Blacklist first.
   int index = mAnylistSpecs.Length() - 1;
   nsCString spec;
@@ -354,20 +383,24 @@ PendingLookup::LookupNext()
       spec = mAllowlistSpecs[index];
       mAllowlistSpecs.RemoveElementAt(index);
     }
   }
   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.
-  // Revert to just ifdef XP_WIN when remote lookups are enabled (bug 933432)
-#if 0
+#ifdef XP_WIN
+  // There are no more URIs to check against local list. If the file is not
+  // eligible for remote lookup, bail.
+  if (!IsBinaryFile()) {
+    LOG(("Not eligible for remote lookups [this=%x]", this));
+    return OnComplete(false, NS_OK);
+  }
+  // Send the remote query if we are on Windows.
   nsresult rv = SendRemoteQuery();
   if (NS_FAILED(rv)) {
     return OnComplete(false, rv);
   }
   return NS_OK;
 #else
   return OnComplete(false, NS_OK);
 #endif
--- a/toolkit/components/downloads/test/unit/test_app_rep_windows.js
+++ b/toolkit/components/downloads/test/unit/test_app_rep_windows.js
@@ -200,30 +200,31 @@ add_task(function test_setup()
     return blob;
   }
 
   gHttpServer.registerPathHandler("/throw", function(request, response) {
     do_throw("We shouldn't be getting here");
   });
 
   gHttpServer.registerPathHandler("/download", function(request, response) {
+    do_print("Querying remote server for verdict");
     response.setHeader("Content-Type", "application/octet-stream", false);
     let buf = NetUtil.readInputStreamToString(
       request.bodyInputStream,
       request.bodyInputStream.available());
     do_print("Request length: " + buf.length);
     // A garbage response. By default this produces NS_CANNOT_CONVERT_DATA as
     // the callback status.
     let blob = "this is not a serialized protocol buffer";
     // We can't actually parse the protocol buffer here, so just switch on the
     // length instead of inspecting the contents.
-    if (buf.length == 35) {
+    if (buf.length == 45) {
       // evil.com
       blob = createVerdict(true);
-    } else if (buf.length == 38) {
+    } else if (buf.length == 48) {
       // mozilla.com
       blob = createVerdict(false);
     }
     response.bodyOutputStream.write(blob, blob.length);
   });
 
   gHttpServer.start(4444);
 });
@@ -289,23 +290,27 @@ function promiseQueryReputation(query, e
     do_check_eq(Cr.NS_OK, aStatus);
     do_check_eq(aShouldBlock, expectedShouldBlock);
     deferred.resolve(true);
   }
   gAppRep.queryReputation(query, onComplete);
   return deferred.promise;
 }
 
+add_task(function()
+{
+  // Wait for Safebrowsing local list updates to complete.
+  yield waitForUpdates();
+});
+
 add_task(function test_signature_whitelists()
 {
   // We should never get to the remote server.
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
                              "http://localhost:4444/throw");
-  // Wait for Safebrowsing local list updates to complete.
-  yield waitForUpdates();
 
   // Use BackgroundFileSaver to extract the signature on Windows.
   let destFile = getTempFile(TEST_FILE_NAME_1);
 
   let data = readFileToString("data/signed_win.exe");
   let saver = new BackgroundFileSaverOutputStream();
   let completionPromise = promiseSaverComplete(saver);
   saver.enableSignatureInfo();
@@ -320,12 +325,44 @@ add_task(function test_signature_whiteli
 
   // evil.com is not on the allowlist, but this binary is signed by an entity
   // whose certificate information is on the allowlist.
   yield promiseQueryReputation({sourceURI: createURI("http://evil.com"),
                                 signatureInfo: saver.signatureInfo,
                                 fileSize: 12}, false);
 });
 
+add_task(function test_blocked_binary()
+{
+  // We should reach the remote server for a verdict.
+  Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
+                             "http://localhost:4444/download");
+  // evil.com should return a malware verdict from the remote server.
+  yield promiseQueryReputation({sourceURI: createURI("http://evil.com"),
+                                suggestedFileName: "noop.bat",
+                                fileSize: 12}, true);
+});
+
+add_task(function test_non_binary()
+{
+  // We should not reach the remote server for a verdict for non-binary files.
+  Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
+                             "http://localhost:4444/throw");
+  yield promiseQueryReputation({sourceURI: createURI("http://evil.com"),
+                                suggestedFileName: "noop.txt",
+                                fileSize: 12}, false);
+});
+
+add_task(function test_good_binary()
+{
+  // We should reach the remote server for a verdict.
+  Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
+                             "http://localhost:4444/download");
+  // mozilla.com should return a not-guilty verdict from the remote server.
+  yield promiseQueryReputation({sourceURI: createURI("http://mozilla.com"),
+                                suggestedFileName: "noop.bat",
+                                fileSize: 12}, false);
+});
+
 add_task(function test_teardown()
 {
   gStillRunning = false;
 });