Bug 1111741 - Enable SafeBrowsing remote lookups for mac and linux. r=mmc
authorFrancois Marier <francois@mozilla.com>
Tue, 24 Mar 2015 20:47:00 -0400
changeset 264863 69c263e8d2b943412eeb0796d8f897feaf2241ba
parent 264862 c58fc129232a3f11b2852d8828b73e23def9b3b1
child 264864 3604d6055fe5404560cbc2b9c47e1cbb93f24bac
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmmc
bugs1111741
milestone39.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 1111741 - Enable SafeBrowsing remote lookups for mac and linux. r=mmc
browser/app/profile/firefox.js
toolkit/components/downloads/ApplicationReputation.cpp
toolkit/components/downloads/test/unit/test_app_rep_maclinux.js
toolkit/components/downloads/test/unit/test_app_rep_windows.js
toolkit/components/downloads/test/unit/xpcshell.ini
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -964,22 +964,17 @@ pref("gecko.handlerService.schemes.ircs.
 
 // By default, we don't want protocol/content handlers to be registered from a different host, see bug 402287
 pref("gecko.handlerService.allowRegisterFromDifferentHost", false);
 
 #ifdef MOZ_SAFE_BROWSING
 pref("browser.safebrowsing.enabled", true);
 pref("browser.safebrowsing.malware.enabled", true);
 pref("browser.safebrowsing.downloads.enabled", true);
-// Remote lookups are only enabled for Windows in Nightly and Aurora
-#if defined(XP_WIN)
 pref("browser.safebrowsing.downloads.remote.enabled", true);
-#else
-pref("browser.safebrowsing.downloads.remote.enabled", false);
-#endif
 pref("browser.safebrowsing.debug", false);
 
 pref("browser.safebrowsing.updateURL", "https://safebrowsing.google.com/safebrowsing/downloads?client=SAFEBROWSING_ID&appver=%VERSION%&pver=2.2&key=%GOOGLE_API_KEY%");
 pref("browser.safebrowsing.gethashURL", "https://safebrowsing.google.com/safebrowsing/gethash?client=SAFEBROWSING_ID&appver=%VERSION%&pver=2.2");
 pref("browser.safebrowsing.reportURL", "https://safebrowsing.google.com/safebrowsing/report?");
 pref("browser.safebrowsing.reportGenericURL", "http://%LOCALE%.phish-generic.mozilla.com/?hl=%LOCALE%");
 pref("browser.safebrowsing.reportErrorURL", "http://%LOCALE%.phish-error.mozilla.com/?hl=%LOCALE%");
 pref("browser.safebrowsing.reportPhishURL", "http://%LOCALE%.phish-report.mozilla.com/?hl=%LOCALE%");
@@ -1010,17 +1005,17 @@ pref("urlclassifier.gethash.timeout_ms",
 // a gethash request will be forced to check that the result is still in
 // the database.
 pref("urlclassifier.max-complete-age", 2700);
 // Tables for application reputation.
 pref("urlclassifier.downloadBlockTable", "goog-badbinurl-shavar");
 #ifdef XP_WIN
 // Only download the whitelist on Windows, since the whitelist is
 // only useful for suppressing remote lookups for signed binaries which we can
-// only verify on Windows (Bug 974579).
+// only verify on Windows (Bug 974579). Other platforms always do remote lookups.
 pref("urlclassifier.downloadAllowTable", "goog-downloadwhite-digest256");
 #endif
 #endif
 
 pref("browser.geolocation.warning.infoURL", "https://www.mozilla.org/%LOCALE%/firefox/geolocation/");
 
 pref("browser.EULA.version", 3);
 pref("browser.rights.version", 3);
--- a/toolkit/components/downloads/ApplicationReputation.cpp
+++ b/toolkit/components/downloads/ApplicationReputation.cpp
@@ -424,33 +424,27 @@ PendingLookup::LookupNext()
   index = mAllowlistSpecs.Length() - 1;
   if (index >= 0) {
     spec = mAllowlistSpecs[index];
     LOG(("PendingLookup::LookupNext: checking %s on allowlist", spec.get()));
     mAllowlistSpecs.RemoveElementAt(index);
     nsRefPtr<PendingDBLookup> lookup(new PendingDBLookup(this));
     return lookup->LookupSpec(spec, true);
   }
-#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
-  LOG(("PendingLookup: Nothing left to check [this=%p]", this));
-  return OnComplete(false, NS_OK);
-#endif
 }
 
 nsCString
 PendingLookup::EscapeCertificateAttribute(const nsACString& aAttribute)
 {
   // Escape '/' because it's a field separator, and '%' because Chrome does
   nsCString escaped;
   escaped.SetCapacity(aAttribute.Length());
@@ -813,39 +807,46 @@ PendingLookup::SendRemoteQuery()
   return rv;
 }
 
 nsresult
 PendingLookup::SendRemoteQueryInternal()
 {
   // If we aren't supposed to do remote lookups, bail.
   if (!Preferences::GetBool(PREF_SB_DOWNLOADS_REMOTE_ENABLED, false)) {
+    LOG(("Remote lookups are disabled [this = %p]", this));
     return NS_ERROR_NOT_AVAILABLE;
   }
   // If the remote lookup URL is empty or absent, bail.
   nsCString serviceUrl;
   NS_ENSURE_SUCCESS(Preferences::GetCString(PREF_SB_APP_REP_URL, &serviceUrl),
                     NS_ERROR_NOT_AVAILABLE);
   if (serviceUrl.EqualsLiteral("")) {
+    LOG(("Remote lookup URL is empty [this = %p]", this));
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // If the blocklist or allowlist is empty (so we couldn't do local lookups),
   // bail
   nsCString table;
   NS_ENSURE_SUCCESS(Preferences::GetCString(PREF_DOWNLOAD_BLOCK_TABLE, &table),
                     NS_ERROR_NOT_AVAILABLE);
   if (table.EqualsLiteral("")) {
+    LOG(("Blocklist is empty [this = %p]", this));
     return NS_ERROR_NOT_AVAILABLE;
   }
+#ifdef XP_WIN
+  // The allowlist is only needed to do signature verification on Windows
   NS_ENSURE_SUCCESS(Preferences::GetCString(PREF_DOWNLOAD_ALLOW_TABLE, &table),
                     NS_ERROR_NOT_AVAILABLE);
   if (table.EqualsLiteral("")) {
+    LOG(("Allowlist is empty [this = %p]", this));
     return NS_ERROR_NOT_AVAILABLE;
   }
+#endif
 
   LOG(("Sending remote query for application reputation [this = %p]",
        this));
   // We did not find a local result, so fire off the query to the
   // application reputation service.
   nsCOMPtr<nsIURI> uri;
   nsresult rv;
   rv = mQuery->GetSourceURI(getter_AddRefs(uri));
@@ -886,16 +887,18 @@ PendingLookup::SendRemoteQueryInternal()
 
   // Serialize the protocol buffer to a string. This can only fail if we are
   // out of memory, or if the protocol buffer req is missing required fields
   // (only the URL for now).
   std::string serialized;
   if (!mRequest.SerializeToString(&serialized)) {
     return NS_ERROR_UNEXPECTED;
   }
+  LOG(("Serialized protocol buffer [this = %p]: %s", this,
+       serialized.c_str()));
 
   // Set the input stream to the serialized protocol buffer
   nsCOMPtr<nsIStringInputStream> sstream =
     do_CreateInstance("@mozilla.org/io/string-input-stream;1", &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = sstream->SetData(serialized.c_str(), serialized.length());
   NS_ENSURE_SUCCESS(rv, rv);
@@ -1015,17 +1018,17 @@ PendingLookup::OnStopRequestInternal(nsI
     Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_SERVER,
       SERVER_RESPONSE_FAILED);
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   std::string buf(mResponse.Data(), mResponse.Length());
   safe_browsing::ClientDownloadResponse response;
   if (!response.ParseFromString(buf)) {
-    NS_WARNING("Could not parse protocol buffer");
+    LOG(("Invalid protocol buffer response [this = %p]: %s", this, buf.c_str()));
     Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_SERVER,
                                    SERVER_RESPONSE_INVALID);
     return NS_ERROR_CANNOT_CONVERT_DATA;
   }
 
   // There are several more verdicts, but we only respect DANGEROUS and
   // DANGEROUS_HOST for now and treat everything else as SAFE.
   Accumulate(mozilla::Telemetry::APPLICATION_REPUTATION_SERVER,
copy from toolkit/components/downloads/test/unit/test_app_rep_windows.js
copy to toolkit/components/downloads/test/unit/test_app_rep_maclinux.js
--- a/toolkit/components/downloads/test/unit/test_app_rep_windows.js
+++ b/toolkit/components/downloads/test/unit/test_app_rep_maclinux.js
@@ -8,134 +8,38 @@
  * downloaded files.
  */
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Globals
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
-XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
-                                  "resource://gre/modules/FileUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Promise",
                                   "resource://gre/modules/Promise.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
 
-const BackgroundFileSaverOutputStream = Components.Constructor(
-      "@mozilla.org/network/background-file-saver;1?mode=outputstream",
-      "nsIBackgroundFileSaver");
-
-const StringInputStream = Components.Constructor(
-      "@mozilla.org/io/string-input-stream;1",
-      "nsIStringInputStream",
-      "setData");
-
-const TEST_FILE_NAME_1 = "test-backgroundfilesaver-1.txt";
-
 const gAppRep = Cc["@mozilla.org/downloads/application-reputation-service;1"].
                   getService(Ci.nsIApplicationReputationService);
 let gStillRunning = true;
 let gTables = {};
 let gHttpServer = null;
 
-/**
- * Returns a reference to a temporary file.  If the file is then created, it
- * will be removed when tests in this file finish.
- */
-function getTempFile(aLeafName) {
-  let file = FileUtils.getFile("TmpD", [aLeafName]);
-  do_register_cleanup(function GTF_cleanup() {
-    if (file.exists()) {
-      file.remove(false);
-    }
-  });
-  return file;
-}
-
 function readFileToString(aFilename) {
   let f = do_get_file(aFilename);
   let stream = Cc["@mozilla.org/network/file-input-stream;1"]
                  .createInstance(Ci.nsIFileInputStream);
   stream.init(f, -1, 0, 0);
   let buf = NetUtil.readInputStreamToString(stream, stream.available());
   return buf;
 }
 
-/**
- * Waits for the given saver object to complete.
- *
- * @param aSaver
- *        The saver, with the output stream or a stream listener implementation.
- * @param aOnTargetChangeFn
- *        Optional callback invoked with the target file name when it changes.
- *
- * @return {Promise}
- * @resolves When onSaveComplete is called with a success code.
- * @rejects With an exception, if onSaveComplete is called with a failure code.
- */
-function promiseSaverComplete(aSaver, aOnTargetChangeFn) {
-  let deferred = Promise.defer();
-  aSaver.observer = {
-    onTargetChange: function BFSO_onSaveComplete(aSaver, aTarget)
-    {
-      if (aOnTargetChangeFn) {
-        aOnTargetChangeFn(aTarget);
-      }
-    },
-    onSaveComplete: function BFSO_onSaveComplete(aSaver, aStatus)
-    {
-      if (Components.isSuccessCode(aStatus)) {
-        deferred.resolve();
-      } else {
-        deferred.reject(new Components.Exception("Saver failed.", aStatus));
-      }
-    },
-  };
-  return deferred.promise;
-}
-
-/**
- * Feeds a string to a BackgroundFileSaverOutputStream.
- *
- * @param aSourceString
- *        The source data to copy.
- * @param aSaverOutputStream
- *        The BackgroundFileSaverOutputStream to feed.
- * @param aCloseWhenDone
- *        If true, the output stream will be closed when the copy finishes.
- *
- * @return {Promise}
- * @resolves When the copy completes with a success code.
- * @rejects With an exception, if the copy fails.
- */
-function promiseCopyToSaver(aSourceString, aSaverOutputStream, aCloseWhenDone) {
-  let deferred = Promise.defer();
-  let inputStream = new StringInputStream(aSourceString, aSourceString.length);
-  let copier = Cc["@mozilla.org/network/async-stream-copier;1"]
-               .createInstance(Ci.nsIAsyncStreamCopier);
-  copier.init(inputStream, aSaverOutputStream, null, false, true, 0x8000, true,
-              aCloseWhenDone);
-  copier.asyncCopy({
-    onStartRequest: function () { },
-    onStopRequest: function (aRequest, aContext, aStatusCode)
-    {
-      if (Components.isSuccessCode(aStatusCode)) {
-        deferred.resolve();
-      } else {
-        deferred.reject(new Components.Exception(aResult));
-      }
-    },
-  }, null);
-  return deferred.promise;
-}
-
-// Registers a table for which to serve update chunks.
 function registerTableUpdate(aTable, aFilename) {
   // If we haven't been given an update for this table yet, add it to the map
   if (!(aTable in gTables)) {
     gTables[aTable] = [];
   }
 
   // The number of chunks associated with this table.
   let numChunks = gTables[aTable].length + 1;
@@ -175,31 +79,27 @@ add_task(function test_setup()
   });
   // Set up a local HTTP server to return bad verdicts.
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
                              "http://localhost:4444/download");
   // Ensure safebrowsing is enabled for this test, even if the app
   // doesn't have it enabled.
   Services.prefs.setBoolPref("browser.safebrowsing.malware.enabled", true);
   Services.prefs.setBoolPref("browser.safebrowsing.downloads.enabled", true);
-  // Set block and allow tables explicitly, since the allowlist is normally
-  // disabled on comm-central.
+  // Set block table explicitly, no need for the allow table though
   Services.prefs.setCharPref("urlclassifier.downloadBlockTable",
                              "goog-badbinurl-shavar");
-  Services.prefs.setCharPref("urlclassifier.downloadAllowTable",
-                             "goog-downloadwhite-digest256");
-  // On Windows SendRemoteQueryInternal needs locale preference.
+  // SendRemoteQueryInternal needs locale preference.
   let locale = Services.prefs.getCharPref("general.useragent.locale");
   Services.prefs.setCharPref("general.useragent.locale", "en-US");
 
   do_register_cleanup(function() {
     Services.prefs.clearUserPref("browser.safebrowsing.malware.enabled");
     Services.prefs.clearUserPref("browser.safebrowsing.downloads.enabled");
     Services.prefs.clearUserPref("urlclassifier.downloadBlockTable");
-    Services.prefs.clearUserPref("urlclassifier.downloadAllowTable");
     Services.prefs.setCharPref("general.useragent.locale", locale);
   });
 
   gHttpServer = new HttpServer();
   gHttpServer.registerDirectory("/", do_get_cwd());
 
   function createVerdict(aShouldBlock) {
     // We can't programmatically create a protocol buffer here, so just
@@ -311,47 +211,16 @@ function promiseQueryReputation(query, e
 }
 
 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.setBoolPref("browser.safebrowsing.downloads.remote.enabled",
-                             true);
-  Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
-                             "http://localhost:4444/throw");
-
-  // 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();
-  saver.setTarget(destFile, false);
-  yield promiseCopyToSaver(data, saver, true);
-
-  saver.finish(Cr.NS_OK);
-  yield completionPromise;
-
-  // Clean up.
-  destFile.remove(false);
-
-  // 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.setBoolPref("browser.safebrowsing.downloads.remote.enabled",
                              true);
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
                              "http://localhost:4444/download");
   // evil.com should return a malware verdict from the remote server.
--- a/toolkit/components/downloads/test/unit/test_app_rep_windows.js
+++ b/toolkit/components/downloads/test/unit/test_app_rep_windows.js
@@ -181,17 +181,17 @@ add_task(function test_setup()
   Services.prefs.setBoolPref("browser.safebrowsing.malware.enabled", true);
   Services.prefs.setBoolPref("browser.safebrowsing.downloads.enabled", true);
   // Set block and allow tables explicitly, since the allowlist is normally
   // disabled on comm-central.
   Services.prefs.setCharPref("urlclassifier.downloadBlockTable",
                              "goog-badbinurl-shavar");
   Services.prefs.setCharPref("urlclassifier.downloadAllowTable",
                              "goog-downloadwhite-digest256");
-  // On Windows SendRemoteQueryInternal needs locale preference.
+  // SendRemoteQueryInternal needs locale preference.
   let locale = Services.prefs.getCharPref("general.useragent.locale");
   Services.prefs.setCharPref("general.useragent.locale", "en-US");
 
   do_register_cleanup(function() {
     Services.prefs.clearUserPref("browser.safebrowsing.malware.enabled");
     Services.prefs.clearUserPref("browser.safebrowsing.downloads.enabled");
     Services.prefs.clearUserPref("urlclassifier.downloadBlockTable");
     Services.prefs.clearUserPref("urlclassifier.downloadAllowTable");
--- a/toolkit/components/downloads/test/unit/xpcshell.ini
+++ b/toolkit/components/downloads/test/unit/xpcshell.ini
@@ -7,16 +7,18 @@ support-files =
   test_downloads.manifest
   data/digest.chunk
   data/block_digest.chunk
   data/signed_win.exe
 
 [test_app_rep.js]
 [test_app_rep_windows.js]
 skip-if = os != "win"
+[test_app_rep_maclinux.js]
+skip-if = os == "win"
 [test_bug_382825.js]
 [test_bug_384744.js]
 [test_bug_395092.js]
 [test_bug_401430.js]
 [test_bug_406857.js]
 [test_bug_420230.js]
 [test_cancel_download_files_removed.js]
 # Bug 676989: test hangs consistently on Android