Bug 1455078: Implement [SecureContext] for ServiceWorkerContainer r=asuth
authorYaron Tausky <ytausky@mozilla.com>
Wed, 22 Aug 2018 15:22:40 +0000
changeset 487963 7472e52f6d1288b773d771918cd23ed951464bf5
parent 487962 9e44b02e94ca5f7e896be3eb02b6d27a40d7054b
child 487964 98b5ff9533ee519674b8c441083470d91e1080f8
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1455078
milestone63.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 1455078: Implement [SecureContext] for ServiceWorkerContainer r=asuth The spec mandates that ServiceWorkerContainer only be visible in a secure context, yet currently it is (almost) always visible, but rejects calls to register() in non-secure contexts. This commit moves the context check to a [Func] function, thus implementing the behavior exactly as specified. This commit uses the same mechanism used by [SecureContext] bindings instead of the current ad hoc implementation. Differential Revision: https://phabricator.services.mozilla.com/D2950
dom/serviceworkers/ServiceWorkerContainer.cpp
dom/serviceworkers/test/register_https.html
dom/serviceworkers/test/test_installation_simple.html
dom/serviceworkers/test/test_register_base.html
dom/tests/mochitest/general/test_interfaces.js
testing/web-platform/meta/service-workers/service-worker/http-to-https-redirect-and-register.https.html.ini
testing/web-platform/meta/service-workers/service-worker/interfaces-window.https.html.ini
--- a/dom/serviceworkers/ServiceWorkerContainer.cpp
+++ b/dom/serviceworkers/ServiceWorkerContainer.cpp
@@ -41,33 +41,66 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
 NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper)
 
 NS_IMPL_ADDREF_INHERITED(ServiceWorkerContainer, DOMEventTargetHelper)
 NS_IMPL_RELEASE_INHERITED(ServiceWorkerContainer, DOMEventTargetHelper)
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(ServiceWorkerContainer, DOMEventTargetHelper,
                                    mControllerWorker, mReadyPromise)
 
+namespace {
+
+bool
+IsInPrivateBrowsing(JSContext* const aCx)
+{
+  if (const nsCOMPtr<nsIGlobalObject> global = xpc::CurrentNativeGlobal(aCx)) {
+    if (const nsCOMPtr<nsIPrincipal> principal = global->PrincipalOrNull()) {
+      return principal->GetPrivateBrowsingId() > 0;
+    }
+  }
+  return false;
+}
+
+bool
+IsServiceWorkersTestingEnabledInWindow(JSObject* const aGlobal)
+{
+  if (const nsCOMPtr<nsPIDOMWindowInner> innerWindow = Navigator::GetWindowFromGlobal(aGlobal)) {
+    if (const nsCOMPtr<nsPIDOMWindowOuter> outerWindow = innerWindow->GetOuterWindow()) {
+      return outerWindow->GetServiceWorkersTestingEnabled();
+    }
+  }
+  return false;
+}
+
+}
+
 /* static */ bool
 ServiceWorkerContainer::IsEnabled(JSContext* aCx, JSObject* aGlobal)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   JS::Rooted<JSObject*> global(aCx, aGlobal);
-  nsCOMPtr<nsPIDOMWindowInner> window = Navigator::GetWindowFromGlobal(global);
-  if (!window) {
+
+  if (!DOMPrefs::ServiceWorkersEnabled()) {
+    return false;
+  }
+
+  if (IsInPrivateBrowsing(aCx)) {
     return false;
   }
 
-  nsIDocument* doc = window->GetExtantDoc();
-  if (!doc || nsContentUtils::IsInPrivateBrowsing(doc)) {
-    return false;
+  if (IsSecureContextOrObjectIsFromSecureContext(aCx, aGlobal)) {
+    return true;
   }
 
-  return DOMPrefs::ServiceWorkersEnabled();
+  const bool isTestingEnabledInWindow = IsServiceWorkersTestingEnabledInWindow(aGlobal);
+  const bool isTestingEnabledByPref = DOMPrefs::ServiceWorkersTestingEnabled();
+  const bool isTestingEnabled = isTestingEnabledByPref || isTestingEnabledInWindow;
+
+  return isTestingEnabled;
 }
 
 // static
 already_AddRefed<ServiceWorkerContainer>
 ServiceWorkerContainer::Create(nsIGlobalObject* aGlobal)
 {
   RefPtr<Inner> inner;
   if (ServiceWorkerParentInterceptEnabled()) {
@@ -149,49 +182,16 @@ GetBaseURIFromGlobal(nsIGlobalObject* aG
   if (!baseURI) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return nullptr;
   }
 
   return baseURI.forget();
 }
 
-// This function implements parts of the step 3 of the following algorithm:
-// https://w3c.github.io/webappsec/specs/powerfulfeatures/#settings-secure
-static bool
-IsFromAuthenticatedOrigin(nsIDocument* aDoc)
-{
-  MOZ_ASSERT(aDoc);
-  nsCOMPtr<nsIDocument> doc(aDoc);
-  nsCOMPtr<nsIContentSecurityManager> csm = do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID);
-  if (NS_WARN_IF(!csm)) {
-    return false;
-  }
-
-  while (doc && !nsContentUtils::IsChromeDoc(doc)) {
-    bool trustworthyOrigin = false;
-
-    // The origin of the document may be different from the document URI
-    // itself.  Check the principal, not the document URI itself.
-    nsCOMPtr<nsIPrincipal> documentPrincipal = doc->NodePrincipal();
-
-    // The check for IsChromeDoc() above should mean we never see a system
-    // principal inside the loop.
-    MOZ_ASSERT(!nsContentUtils::IsSystemPrincipal(documentPrincipal));
-
-    csm->IsOriginPotentiallyTrustworthy(documentPrincipal, &trustworthyOrigin);
-    if (!trustworthyOrigin) {
-      return false;
-    }
-
-    doc = doc->GetParentDocument();
-  }
-  return true;
-}
-
 } // anonymous namespace
 
 already_AddRefed<Promise>
 ServiceWorkerContainer::Register(const nsAString& aScriptURL,
                                  const RegistrationOptions& aOptions,
                                  ErrorResult& aRv)
 {
   // Note, we can't use GetGlobalIfValid() from the start here.  If we
@@ -280,37 +280,16 @@ ServiceWorkerContainer::Register(const n
   }
 
   nsIDocument* doc = window->GetExtantDoc();
   if (!doc) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return nullptr;
   }
 
-  // Next, implement a lame version of [SecureContext] with an
-  // exception based on a pref or devtools option.
-  // TODO: This logic should be moved to a webidl [Func]. See bug 1455078.
-  nsCOMPtr<nsPIDOMWindowOuter> outerWindow = window->GetOuterWindow();
-  bool serviceWorkersTestingEnabled =
-    outerWindow->GetServiceWorkersTestingEnabled();
-
-  bool authenticatedOrigin;
-  if (DOMPrefs::ServiceWorkersTestingEnabled() ||
-      serviceWorkersTestingEnabled) {
-    authenticatedOrigin = true;
-  } else {
-    authenticatedOrigin = IsFromAuthenticatedOrigin(doc);
-  }
-
-  if (!authenticatedOrigin) {
-    NS_WARNING("ServiceWorker registration from insecure websites is not allowed.");
-    aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
-    return nullptr;
-  }
-
   // The next section of code executes an NS_CheckContentLoadPolicy()
   // check.  This is necessary to enforce the CSP of the calling client.
   // Currently this requires an nsIDocument.  Once bug 965637 lands we
   // should try to move this into ServiceWorkerScopeAndScriptAreValid()
   // using the ClientInfo instead of doing a window-specific check here.
   // See bug 1455077 for further investigation.
   nsCOMPtr<nsILoadInfo> secCheckLoadInfo =
     new mozilla::net::LoadInfo(doc->NodePrincipal(), // loading principal
--- a/dom/serviceworkers/test/register_https.html
+++ b/dom/serviceworkers/test/register_https.html
@@ -4,18 +4,11 @@ function ok(condition, message) {
   parent.postMessage({type: "ok", status: condition, msg: message}, "*");
 }
 
 function done() {
   parent.postMessage({type: "done"}, "*");
 }
 
 ok(location.protocol == "https:", "We should be loaded from HTTPS");
-
-navigator.serviceWorker.register("empty.js", {scope: "register-https"})
-  .then(reg => {
-    ok(false, "Registration should fail");
-    done();
-  }).catch(err => {
-    ok(err.name === "SecurityError", "Registration should fail with SecurityError");
-    done();
-  });
+ok(!("serviceWorker" in navigator), "ServiceWorkerContainer not availalble in insecure context");
+done();
 </script>
--- a/dom/serviceworkers/test/test_installation_simple.html
+++ b/dom/serviceworkers/test/test_installation_simple.html
@@ -41,17 +41,17 @@
 
   function httpsOnly() {
     return SpecialPowers.pushPrefEnv({'set': [["dom.serviceWorkers.testing.enabled", false]] })
     .then(function() {
       return navigator.serviceWorker.register("/worker.js");
     }).then(function(w) {
       ok(false, "non-HTTPS pages cannot register ServiceWorkers");
     }, function(e) {
-      ok(e.name === "SecurityError", "Should fail with a SecurityError");
+      ok(e.name === "TypeError", "navigator.serviceWorker should be undefined");
     }).then(function() {
       return SpecialPowers.popPrefEnv();
     });
   }
 
   function realWorker() {
     var p = navigator.serviceWorker.register("worker.js", { scope: "realworker" });
     return p.then(function(wr) {
@@ -171,20 +171,20 @@
 
   function checkReadyPromise() {
     ok(readyPromiseResolved, "The ready promise has been resolved!");
     return Promise.resolve();
   }
 
   function runTest() {
     simpleRegister()
-      .then(readyPromise)
       .then(sameOriginWorker)
       .then(sameOriginScope)
       .then(httpsOnly)
+      .then(readyPromise)
       .then(realWorker)
       .then(networkError404)
       .then(redirectError)
       .then(parseError)
       .then(updatefound)
       .then(checkReadyPromise)
       // put more tests here.
       .then(function() {
--- a/dom/serviceworkers/test/test_register_base.html
+++ b/dom/serviceworkers/test/test_register_base.html
@@ -13,24 +13,18 @@
 <body>
 <p id="display"></p>
 <div id="content" style="display: none">
 </div>
 <pre id="test"></pre>
 <script class="testbody" type="text/javascript">
 
   function runTest() {
-    navigator.serviceWorker.register("http://mochi.test:8888/tests/dom/serviceworkers/test/empty.js")
-      .then(reg => {
-        ok(false, "Register should fail");
-        SimpleTest.finish();
-      }, err => {
-        is(err.name, "SecurityError", "Registration should fail with SecurityError");
-        SimpleTest.finish();
-      });
+    ok(!("serviceWorker" in navigator), "ServiceWorkerContainer shouldn't be defined");
+    SimpleTest.finish();
   }
 
   SimpleTest.waitForExplicitFinish();
   onload = function() {
     SpecialPowers.pushPrefEnv({"set": [
       ["dom.serviceWorkers.exemptFromPerDomainMax", true],
       ["dom.serviceWorkers.enabled", true],
     ]}, runTest);
--- a/dom/tests/mochitest/general/test_interfaces.js
+++ b/dom/tests/mochitest/general/test_interfaces.js
@@ -859,17 +859,17 @@ var interfaceNamesInGlobalScope =
     {name: "ScrollAreaEvent", insecureContext: true},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "SecurityPolicyViolationEvent", insecureContext: true},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "Selection", insecureContext: true},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "ServiceWorker", insecureContext: true},
 // IMPORTANT: Do not change this list without review from a DOM peer!
-    {name: "ServiceWorkerContainer", insecureContext: true},
+    {name: "ServiceWorkerContainer", insecureContext: false},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "ServiceWorkerRegistration", insecureContext: true},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "ScopedCredential", insecureContext: true, disabled: true},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "ScopedCredentialInfo", insecureContext: true, disabled: true},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "ShadowRoot", insecureContext: true},
deleted file mode 100644
--- a/testing/web-platform/meta/service-workers/service-worker/http-to-https-redirect-and-register.https.html.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[http-to-https-redirect-and-register.https.html]
-  [register on a non-secure page after redirect from an non-secure url]
-    expected: FAIL
-
--- a/testing/web-platform/meta/service-workers/service-worker/interfaces-window.https.html.ini
+++ b/testing/web-platform/meta/service-workers/service-worker/interfaces-window.https.html.ini
@@ -66,11 +66,8 @@
     expected: FAIL
 
   [ServiceWorkerRegistration interface: window.registrationInstance must inherit property "active" with the proper type]
     expected: FAIL
 
   [ServiceWorkerRegistration interface: window.registrationInstance must inherit property "navigationPreload" with the proper type]
     expected: FAIL
 
-  [navigator.serviceWorker is not available in a data: iframe]
-    expected: FAIL
-