From 2bb448cbb2ec087b28a4fadd8e1bae849721d551 Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Tue, 26 Apr 2016 11:30:43 +0100 Subject: [PATCH] Bug 1267509 - Make nsContentSecurityManager::IsURIPotentiallyTrustworthy act on an nsIPrincipal. r=bz MozReview-Commit-ID: Zu1zU4Brkx --HG-- rename : dom/security/test/unit/test_isURIPotentiallyTrustworthy.js => dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js --- .../security/nsIContentSecurityManager.idl | 10 +++--- dom/media/MediaManager.cpp | 2 +- dom/security/nsContentSecurityManager.cpp | 34 ++++++++++++++++--- ...=> test_isOriginPotentiallyTrustworthy.js} | 9 +++-- dom/security/test/unit/xpcshell.ini | 2 +- dom/workers/ServiceWorkerManager.cpp | 15 +++----- .../passwordmgr/LoginManagerContent.jsm | 11 +++--- 7 files changed, 55 insertions(+), 28 deletions(-) rename dom/security/test/unit/{test_isURIPotentiallyTrustworthy.js => test_isOriginPotentiallyTrustworthy.js} (70%) diff --git a/dom/interfaces/security/nsIContentSecurityManager.idl b/dom/interfaces/security/nsIContentSecurityManager.idl index 8a54d1699183..1a1c91021751 100644 --- a/dom/interfaces/security/nsIContentSecurityManager.idl +++ b/dom/interfaces/security/nsIContentSecurityManager.idl @@ -5,6 +5,7 @@ #include "nsISupports.idl" interface nsIChannel; +interface nsIPrincipal; interface nsIStreamListener; interface nsIURI; @@ -13,7 +14,7 @@ interface nsIURI; * Describes an XPCOM component used to perform security checks. */ -[scriptable, uuid(ec955006-747d-4151-aeec-70bd0edc3341)] +[scriptable, uuid(3a9a1818-2ae8-4ec5-a340-8b29d31fca3b)] interface nsIContentSecurityManager : nsISupports { /** @@ -45,10 +46,11 @@ interface nsIContentSecurityManager : nsISupports * Implementation of * https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy * - * This method should only be used when the context of the URI isn't available - * since isSecureContext is preferred as it handles parent contexts. + * The value returned by this method feeds into the the Secure Context + * algorithm that determins the value of Window.isSecureContext and + * WorkerGlobalScope.isSecureContext. * * This method returns false instead of throwing upon errors. */ - boolean isURIPotentiallyTrustworthy(in nsIURI aURI); + boolean isOriginPotentiallyTrustworthy(in nsIPrincipal aPrincipal); }; diff --git a/dom/media/MediaManager.cpp b/dom/media/MediaManager.cpp index 8f89c869dedc..1c0bf179b3be 100644 --- a/dom/media/MediaManager.cpp +++ b/dom/media/MediaManager.cpp @@ -1792,7 +1792,7 @@ MediaManager::GetUserMedia(nsPIDOMWindowInner* aWindow, bool isApp; docURI->SchemeIs("app", &isApp); // Same localhost check as ServiceWorkers uses - // (see IsURIPotentiallyTrustworthy()) + // (see IsOriginPotentiallyTrustworthy()) bool isLocalhost = NS_SUCCEEDED(rv) && (host.LowerCaseEqualsLiteral("localhost") || host.LowerCaseEqualsLiteral("127.0.0.1") || diff --git a/dom/security/nsContentSecurityManager.cpp b/dom/security/nsContentSecurityManager.cpp index 8f0315337f22..53d57ec32c09 100644 --- a/dom/security/nsContentSecurityManager.cpp +++ b/dom/security/nsContentSecurityManager.cpp @@ -544,19 +544,45 @@ nsContentSecurityManager::PerformSecurityCheck(nsIChannel* aChannel, } NS_IMETHODIMP -nsContentSecurityManager::IsURIPotentiallyTrustworthy(nsIURI* aURI, bool* aIsTrustWorthy) +nsContentSecurityManager::IsOriginPotentiallyTrustworthy(nsIPrincipal* aPrincipal, + bool* aIsTrustWorthy) { MOZ_ASSERT(NS_IsMainThread()); - NS_ENSURE_ARG_POINTER(aURI); + NS_ENSURE_ARG_POINTER(aPrincipal); NS_ENSURE_ARG_POINTER(aIsTrustWorthy); + if (aPrincipal->GetIsSystemPrincipal()) { + *aIsTrustWorthy = true; + return NS_OK; + } + + // The following implements: + // https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy + *aIsTrustWorthy = false; + + if (aPrincipal->GetIsNullPrincipal()) { + return NS_OK; + } + + MOZ_ASSERT(aPrincipal->GetIsCodebasePrincipal(), + "Nobody is expected to call us with an nsIExpandedPrincipal"); + + nsCOMPtr uri; + aPrincipal->GetURI(getter_AddRefs(uri)); + nsAutoCString scheme; - nsresult rv = aURI->GetScheme(scheme); + nsresult rv = uri->GetScheme(scheme); if (NS_FAILED(rv)) { return NS_OK; } + // Blobs are expected to inherit their principal so we don't expect to have + // a codebase principal with scheme 'blob' here. We can't assert that though + // since someone could mess with a non-blob URI to give it that scheme. + NS_WARN_IF_FALSE(!scheme.EqualsLiteral("blob"), + "IsOriginPotentiallyTrustworthy ignoring blob scheme"); + // According to the specification, the user agent may choose to extend the // trust to other, vendor-specific URL schemes. We use this for "resource:", // which is technically a substituting protocol handler that is not limited to @@ -572,7 +598,7 @@ nsContentSecurityManager::IsURIPotentiallyTrustworthy(nsIURI* aURI, bool* aIsTru } nsAutoCString host; - rv = aURI->GetHost(host); + rv = uri->GetHost(host); if (NS_FAILED(rv)) { return NS_OK; } diff --git a/dom/security/test/unit/test_isURIPotentiallyTrustworthy.js b/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js similarity index 70% rename from dom/security/test/unit/test_isURIPotentiallyTrustworthy.js rename to dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js index a5946a42f335..de1834fbc881 100644 --- a/dom/security/test/unit/test_isURIPotentiallyTrustworthy.js +++ b/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js @@ -11,11 +11,15 @@ const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components; Cu.import("resource://gre/modules/NetUtil.jsm"); Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +XPCOMUtils.defineLazyServiceGetter(this, "gScriptSecurityManager", + "@mozilla.org/scriptsecuritymanager;1", + "nsIScriptSecurityManager"); + XPCOMUtils.defineLazyServiceGetter(this, "gContentSecurityManager", "@mozilla.org/contentsecuritymanager;1", "nsIContentSecurityManager"); -add_task(function* test_isURIPotentiallyTrustworthy() { +add_task(function* test_isOriginPotentiallyTrustworthy() { for (let [uriSpec, expectedResult] of [ ["http://example.com/", false], ["https://example.com/", true], @@ -27,7 +31,8 @@ add_task(function* test_isURIPotentiallyTrustworthy() { ["urn:generic", false], ]) { let uri = NetUtil.newURI(uriSpec); - Assert.equal(gContentSecurityManager.isURIPotentiallyTrustworthy(uri), + let principal = gScriptSecurityManager.getCodebasePrincipal(uri); + Assert.equal(gContentSecurityManager.isOriginPotentiallyTrustworthy(principal), expectedResult); } }); diff --git a/dom/security/test/unit/xpcshell.ini b/dom/security/test/unit/xpcshell.ini index 74612ff968d3..7610e9eec7cd 100644 --- a/dom/security/test/unit/xpcshell.ini +++ b/dom/security/test/unit/xpcshell.ini @@ -5,5 +5,5 @@ skip-if = toolkit == 'gonk' [test_csp_reports.js] skip-if = buildapp == 'mulet' -[test_isURIPotentiallyTrustworthy.js] +[test_isOriginPotentiallyTrustworthy.js] [test_csp_upgrade_insecure_request_header.js] diff --git a/dom/workers/ServiceWorkerManager.cpp b/dom/workers/ServiceWorkerManager.cpp index 72e2a1e98374..2e985e261037 100644 --- a/dom/workers/ServiceWorkerManager.cpp +++ b/dom/workers/ServiceWorkerManager.cpp @@ -555,7 +555,7 @@ IsFromAuthenticatedOrigin(nsIDocument* aDoc) } while (doc && !nsContentUtils::IsChromeDoc(doc)) { - bool trustworthyURI = false; + bool trustworthyOrigin = false; // The origin of the document may be different from the document URI // itself. Check the principal, not the document URI itself. @@ -565,15 +565,8 @@ IsFromAuthenticatedOrigin(nsIDocument* aDoc) // principal inside the loop. MOZ_ASSERT(!nsContentUtils::IsSystemPrincipal(documentPrincipal)); - // Pass the principal as a URI to the security manager - nsCOMPtr uri; - documentPrincipal->GetURI(getter_AddRefs(uri)); - if (NS_WARN_IF(!uri)) { - return false; - } - - csm->IsURIPotentiallyTrustworthy(uri, &trustworthyURI); - if (!trustworthyURI) { + csm->IsOriginPotentiallyTrustworthy(documentPrincipal, &trustworthyOrigin); + if (!trustworthyOrigin) { return false; } @@ -654,7 +647,7 @@ ServiceWorkerManager::Register(mozIDOMWindow* aWindow, return NS_ERROR_DOM_SECURITY_ERR; } - // The IsURIPotentiallyTrustworthy() check allows file:// and possibly other + // The IsOriginPotentiallyTrustworthy() check allows file:// and possibly other // URI schemes. We need to explicitly only allows http and https schemes. // Note, we just use the aScriptURI here for the check since its already // been verified as same origin with the document principal. This also diff --git a/toolkit/components/passwordmgr/LoginManagerContent.jsm b/toolkit/components/passwordmgr/LoginManagerContent.jsm index 4a078b353b6b..3058942f4f85 100644 --- a/toolkit/components/passwordmgr/LoginManagerContent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm @@ -1123,8 +1123,8 @@ var LoginManagerContent = { * The document whose principal and URI are to be considered. */ isDocumentSecure(document) { - let docPrincipal = document.nodePrincipal; - if (docPrincipal.isSystemPrincipal) { + let principal = document.nodePrincipal; + if (principal.isSystemPrincipal) { return true; } @@ -1134,11 +1134,12 @@ var LoginManagerContent = { // insecure while they are secure, for example sandboxed documents created // using a "javascript:" or "data:" URI from an HTTPS page. See bug 1162772 // for defining "window.isSecureContext", that may help in these cases. - let uri = docPrincipal.isCodebasePrincipal ? docPrincipal.URI - : document.documentURIObject; + if (!principal.isCodebasePrincipal) { + principal = getCodebasePrincipal(document.documentURIObject); + } // These checks include "file", "resource", HTTPS, and HTTP to "localhost". - return gContentSecurityManager.isURIPotentiallyTrustworthy(uri); + return gContentSecurityManager.isOriginPotentiallyTrustworthy(principal); }, };