Bug 1269531 - Make the Geolocation test suite secure-context aware. r?jdm draft
authorMichelangelo De Simone <mdesimone@mozilla.com>
Thu, 26 Jan 2017 15:38:16 -0700
changeset 466960 f596a6c0635e1126aea04d7595e8a6925d150f4c
parent 466816 d92fd6b6d6bfc5b566222ae2957e55772d60151a
child 543588 2b0737500957f898884fd078b546ed58bc3a87cc
push id43070
push userbmo:mdesimone@mozilla.com
push dateThu, 26 Jan 2017 23:46:08 +0000
reviewersjdm
bugs1269531
milestone54.0a1
Bug 1269531 - Make the Geolocation test suite secure-context aware. r?jdm MozReview-Commit-ID: 4WPUsGAO7xF
browser/app/profile/firefox.js
dom/geolocation/nsGeolocation.cpp
dom/geolocation/nsGeolocation.h
dom/locales/en-US/chrome/dom/dom.properties
dom/tests/mochitest/geolocation/mochitest.ini
dom/tests/mochitest/geolocation/test_geoGetCurrentPositionBlockedInInsecureContext.html
dom/tests/mochitest/geolocation/test_geoWatchPositionBlockedInInsecureContext.html
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1250,16 +1250,18 @@ pref("security.cert_pinning.enforcement_
 
 // Override the Gecko-default value of false for Firefox.
 pref("plain_text.wrap_long_lines", true);
 
 // If this turns true, Moz*Gesture events are not called stopPropagation()
 // before content.
 pref("dom.debug.propagate_gesture_events_through_content", false);
 
+// All the Geolocation preferences are here.
+//
 // The request URL of the GeoLocation backend.
 #ifdef RELEASE_OR_BETA
 pref("geo.wifi.uri", "https://www.googleapis.com/geolocation/v1/geolocate?key=%GOOGLE_API_KEY%");
 #else
 pref("geo.wifi.uri", "https://location.services.mozilla.com/v1/geolocate?key=%MOZILLA_API_KEY%");
 #endif
 
 #ifdef XP_MACOSX
@@ -1279,16 +1281,21 @@ pref("geo.provider.ms-windows-location",
 #ifdef RELEASE_OR_BETA
 pref("geo.provider.use_gpsd", false);
 #else
 pref("geo.provider.use_gpsd", true);
 #endif
 #endif
 #endif
 
+// We keep allowing non-HTTPS geo requests on all the release
+// channels, for now.
+// TODO: default to false (or remove altogether) for #1072859.
+pref("geo.security.allowinsecure", true);
+
 // Necko IPC security checks only needed for app isolation for cookies/cache/etc:
 // currently irrelevant for desktop e10s
 pref("network.disable.ipc.security", true);
 
 // CustomizableUI debug logging.
 pref("browser.uiCustomization.debug", false);
 
 // CustomizableUI state of the browser's user interface
--- a/dom/geolocation/nsGeolocation.cpp
+++ b/dom/geolocation/nsGeolocation.cpp
@@ -1,36 +1,37 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "nsXULAppAPI.h"
+#include "nsGeolocation.h"
 
+#include "mozilla/ClearOnShutdown.h"
 #include "mozilla/dom/ContentChild.h"
+#include "mozilla/dom/PermissionMessageUtils.h"
+#include "mozilla/Preferences.h"
+#include "mozilla/Services.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/UniquePtr.h"
-
-#include "nsGeolocation.h"
-#include "nsDOMClassInfoID.h"
+#include "mozilla/Unused.h"
+#include "mozilla/WeakPtr.h"
 #include "nsComponentManagerUtils.h"
-#include "nsServiceManagerUtils.h"
+#include "nsContentPermissionHelper.h"
 #include "nsContentUtils.h"
-#include "nsContentPermissionHelper.h"
+#include "nsDOMClassInfoID.h"
+#include "nsGlobalWindow.h"
 #include "nsIDocument.h"
 #include "nsIObserverService.h"
+#include "nsIScriptError.h"
 #include "nsPIDOMWindow.h"
+#include "nsServiceManagerUtils.h"
 #include "nsThreadUtils.h"
-#include "mozilla/Services.h"
-#include "mozilla/Unused.h"
-#include "mozilla/Preferences.h"
-#include "mozilla/ClearOnShutdown.h"
-#include "mozilla/WeakPtr.h"
-#include "mozilla/dom/PermissionMessageUtils.h"
+#include "nsXULAppAPI.h"
 
 class nsIPrincipal;
 
 #ifdef MOZ_WIDGET_ANDROID
 #include "AndroidLocationProvider.h"
 #endif
 
 #ifdef MOZ_WIDGET_GONK
@@ -49,16 +50,20 @@ class nsIPrincipal;
 #include "WindowsLocationProvider.h"
 #include "mozilla/WindowsVersion.h"
 #endif
 
 // Some limit to the number of get or watch geolocation requests
 // that a window can make.
 #define MAX_GEO_REQUESTS_PER_WINDOW  1500
 
+// This preference allows to override the "secure context" by
+// default policy.
+#define PREF_GEO_SECURITY_ALLOWINSECURE "geo.security.allowinsecure"
+
 using mozilla::Unused;          // <snicker>
 using namespace mozilla;
 using namespace mozilla::dom;
 
 class nsGeolocationRequest final
  : public nsIContentPermissionRequest
  , public nsIGeolocationUpdate
  , public SupportsWeakPtr<nsGeolocationRequest>
@@ -723,21 +728,21 @@ nsresult nsGeolocationService::Init()
   }
 
   // Override platform-specific providers with the default (network)
   // provider while testing. Our tests are currently not meant to exercise
   // the provider, and some tests rely on the network provider being used.
   // "geo.provider.testing" is always set for all plain and browser chrome
   // mochitests, and also for xpcshell tests.
   if (!mProvider || Preferences::GetBool("geo.provider.testing", false)) {
-    nsCOMPtr<nsIGeolocationProvider> override =
+    nsCOMPtr<nsIGeolocationProvider> geo_net_provider =
       do_GetService(NS_GEOLOCATION_PROVIDER_CONTRACTID);
 
-    if (override) {
-      mProvider = override;
+    if (geo_net_provider) {
+      mProvider = geo_net_provider;
     }
   }
 
   return NS_OK;
 }
 
 nsGeolocationService::~nsGeolocationService() = default;
 
@@ -1169,16 +1174,46 @@ Geolocation::IsAlreadyCleared(nsGeolocat
       return true;
     }
   }
 
   return false;
 }
 
 bool
+Geolocation::ShouldBlockInsecureRequests() const
+{
+  // TODO: Also remove all the *_SECURE_ORIGIN Telemetry probes before
+  // landing the patch for #1072859. Also default to false.
+  if (Preferences::GetBool(PREF_GEO_SECURITY_ALLOWINSECURE, true)) {
+    return false;
+  }
+
+  nsCOMPtr<nsPIDOMWindowInner> win = do_QueryReferent(mOwner);
+  if (!win) {
+    return false;
+  }
+
+  nsCOMPtr<nsIDocument> doc = win->GetDoc();
+  if (!doc) {
+    return false;
+  }
+
+  if (!nsGlobalWindow::Cast(win)->IsSecureContext()) {
+    nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
+                                    NS_LITERAL_CSTRING("DOM"), doc,
+                                    nsContentUtils::eDOM_PROPERTIES,
+                                    "GeolocationInsecureRequestIsForbidden");
+    return true;
+  }
+
+  return false;
+}
+
+bool
 Geolocation::ClearPendingRequest(nsGeolocationRequest* aRequest)
 {
   if (aRequest->IsWatch() && this->IsAlreadyCleared(aRequest)) {
     this->NotifyAllowedRequest(aRequest);
     this->ClearWatch(aRequest->WatchId());
     return true;
   }
 
@@ -1220,17 +1255,17 @@ Geolocation::GetCurrentPosition(GeoPosit
   Telemetry::Accumulate(Telemetry::GEOLOCATION_GETCURRENTPOSITION_SECURE_ORIGIN,
                         static_cast<uint8_t>(mProtocolType));
 
   RefPtr<nsGeolocationRequest> request =
     new nsGeolocationRequest(this, Move(callback), Move(errorCallback),
                              Move(options), static_cast<uint8_t>(mProtocolType),
                              false);
 
-  if (!sGeoEnabled) {
+  if (!sGeoEnabled || ShouldBlockInsecureRequests()) {
     nsCOMPtr<nsIRunnable> ev = new RequestAllowEvent(false, request);
     NS_DispatchToMainThread(ev);
     return NS_OK;
   }
 
   if (!mOwner && aCallerType != CallerType::System) {
     return NS_ERROR_FAILURE;
   }
@@ -1306,17 +1341,17 @@ Geolocation::WatchPosition(GeoPositionCa
   // The watch ID:
   *aRv = mLastWatchId++;
 
   RefPtr<nsGeolocationRequest> request =
     new nsGeolocationRequest(this, Move(aCallback), Move(aErrorCallback),
                              Move(aOptions),
                              static_cast<uint8_t>(mProtocolType), true, *aRv);
 
-  if (!sGeoEnabled) {
+  if (!sGeoEnabled || ShouldBlockInsecureRequests()) {
     nsCOMPtr<nsIRunnable> ev = new RequestAllowEvent(false, request);
     NS_DispatchToMainThread(ev);
     return NS_OK;
   }
 
   if (!mOwner && aCallerType != CallerType::System) {
     return NS_ERROR_FAILURE;
   }
--- a/dom/geolocation/nsGeolocation.h
+++ b/dom/geolocation/nsGeolocation.h
@@ -194,16 +194,20 @@ private:
                          CallerType aCallerType,
                          int32_t* aRv);
 
   bool RegisterRequestWithPrompt(nsGeolocationRequest* request);
 
   // Check if clearWatch is already called
   bool IsAlreadyCleared(nsGeolocationRequest* aRequest);
 
+  // Returns whether the Geolocation object should block requests
+  // within a context that is not secure.
+  bool ShouldBlockInsecureRequests() const;
+
   // Two callback arrays.  The first |mPendingCallbacks| holds objects for only
   // one callback and then they are released/removed from the array.  The second
   // |mWatchingCallbacks| holds objects until the object is explictly removed or
   // there is a page change. All requests held by either array are active, that
   // is, they have been allowed and expect to be fulfilled.
 
   nsTArray<RefPtr<nsGeolocationRequest> > mPendingCallbacks;
   nsTArray<RefPtr<nsGeolocationRequest> > mWatchingCallbacks;
--- a/dom/locales/en-US/chrome/dom/dom.properties
+++ b/dom/locales/en-US/chrome/dom/dom.properties
@@ -313,8 +313,9 @@ GenericFileName=file
 # LOCALIZATION NOTE: Do not translate "Large-Allocation", as it is a literal header name
 LargeAllocationSuccess=This page was loaded in a new process due to a Large-Allocation header.
 # LOCALIZATION NOTE: Do not translate "Large-Allocation", as it is a literal header name. Do not translate GET.
 LargeAllocationNonGetRequest=A Large-Allocation header was ignored due to the load being triggered by a non-GET request.
 # LOCALIZATION NOTE: Do not translate "Large-Allocation", as it is a literal header name. Do not translate `window.opener`.
 LargeAllocationNotOnlyToplevelInTabGroup=A Large-Allocation header was ignored due to the presence of windows which have a reference to this browsing context through the frame hierarchy or window.opener.
 # LOCALIZATION NOTE: Do not translate "Large-Allocation", as it is a literal header name
 LargeAllocationNonE10S=A Large-Allocation header was ignored due to the document not being loaded out of process.
+GeolocationInsecureRequestIsForbidden=A Geolocation request can only be fulfilled in a secure context.
--- a/dom/tests/mochitest/geolocation/mochitest.ini
+++ b/dom/tests/mochitest/geolocation/mochitest.ini
@@ -1,9 +1,10 @@
 [DEFAULT]
+scheme = https
 support-files =
   geolocation.html
   geolocation_common.js
   network_geolocation.sjs
   windowTest.html
 
 [test_allowCurrent.html]
 [test_allowWatch.html]
@@ -21,8 +22,16 @@ support-files =
 [test_manyWatchSerial.html]
 [test_manyWindows.html]
 [test_optional_api_params.html]
 [test_shutdown.html]
 [test_timeoutCurrent.html]
 [test_timerRestartWatch.html]
 [test_windowClose.html]
 [test_worseAccuracyDoesNotBlockCallback.html]
+
+# This test REQUIRES to run on HTTP (_NOT_ HTTPS).
+[test_geoWatchPositionBlockedInInsecureContext.html]
+scheme = http
+
+# This test REQUIRES to run on HTTP (_NOT_ HTTPS).
+[test_geoGetCurrentPositionBlockedInInsecureContext.html]
+scheme = http
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/geolocation/test_geoGetCurrentPositionBlockedInInsecureContext.html
@@ -0,0 +1,46 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1269531
+-->
+<head>
+  <title>Test for Bug 1269531</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="geolocation_common.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1269531">Mozilla Bug 1269531</a>
+  <p id="display"></p>
+  <div id="content" style="display: none">
+  </div>
+  <pre id="test">
+  <script class="testbody" type="text/javascript">
+
+  SimpleTest.waitForExplicitFinish();
+
+  // The test succeeds if the error callback is called because we expect
+  // failure when a getCurrentPosition() request is submitted in a non
+  // secure context.
+  function successCallback(position) {
+    ok(false, "Success callback is not expected to be called");
+    SimpleTest.finish();
+  }
+
+  function errorCallback(error) {
+    ok(true, "Check for the error callback to be called for insecure requests");
+    SimpleTest.finish();
+  }
+
+  // Insecure requests should be blocked, for that we enable the relevant pref.
+  SpecialPowers.pushPrefEnv({"set": [["geo.security.allowinsecure", false]]}, function() {
+    force_prompt(true, function() {
+      navigator.geolocation.getCurrentPosition(successCallback, errorCallback);
+    })
+  });
+
+  </script>
+  </pre>
+</body>
+</html>
+
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/geolocation/test_geoWatchPositionBlockedInInsecureContext.html
@@ -0,0 +1,50 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1269531
+-->
+<head>
+  <title>Test for Bug 1269531</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="geolocation_common.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1269531">Mozilla Bug 1269531</a>
+  <p id="display"></p>
+  <div id="content" style="display: none">
+  </div>
+  <pre id="test">
+  <script class="testbody" type="text/javascript">
+
+  SimpleTest.waitForExplicitFinish();
+
+  var wid;
+
+  // The test succeeds if the error callback is called because we expect
+  // failure when a watchPosition() request is submitted in a non
+  // secure context.
+  function successCallback(position) {
+    ok(false, "Success callback is not expected to be called");
+    navigator.geolocation.clearWatch(wid);
+    SimpleTest.finish();
+  }
+
+  function errorCallback(error) {
+    ok(true, "Check for the error callback to be called for insecure requests");
+    navigator.geolocation.clearWatch(wid);
+    SimpleTest.finish();
+  }
+
+  // Insecure requests should be blocked, for that we enable the relevant pref.
+  SpecialPowers.pushPrefEnv({"set": [["geo.security.allowinsecure", false]]}, function() {
+    force_prompt(true, function() {
+      wid = navigator.geolocation.watchPosition(successCallback, errorCallback);
+    })
+  });
+
+  </script>
+  </pre>
+</body>
+</html>
+