Skip to content

Commit

Permalink
Bug 1597704 - Move is OriginPotentially Trustworthy into Principal r=…
Browse files Browse the repository at this point in the history
…ckerschb

Differential Revision: https://phabricator.services.mozilla.com/D53830
  • Loading branch information
strseb committed Nov 28, 2019
1 parent 4a06357 commit 3d4f0e2
Show file tree
Hide file tree
Showing 16 changed files with 61 additions and 107 deletions.
16 changes: 16 additions & 0 deletions caps/BasePrincipal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "mozilla/dom/BlobURLProtocolHandler.h"
#include "mozilla/dom/ChromeUtils.h"
#include "mozilla/dom/ToJSValue.h"
#include "mozilla/dom/nsMixedContentBlocker.h"

#include "json/json.h"
#include "nsSerializationHelper.h"
Expand Down Expand Up @@ -498,6 +499,21 @@ BasePrincipal::IsURIInPrefList(const char* aPref, bool* aResult) {
return NS_OK;
}

NS_IMETHODIMP
BasePrincipal::GetIsOriginPotentiallyTrustworthy(bool* aResult) {
MOZ_ASSERT(NS_IsMainThread());
*aResult = false;

nsCOMPtr<nsIURI> uri;
nsresult rv = GetURI(getter_AddRefs(uri));
if (NS_FAILED(rv) || !uri) {
return NS_OK;
}

*aResult = nsMixedContentBlocker::IsPotentiallyTrustworthyOrigin(uri);
return NS_OK;
}

NS_IMETHODIMP
BasePrincipal::GetAboutModuleFlags(uint32_t* flags) {
*flags = 0;
Expand Down
1 change: 1 addition & 0 deletions caps/BasePrincipal.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ class BasePrincipal : public nsJSPrincipals {
NS_IMETHOD GetSiteOrigin(nsACString& aOrigin) override;
NS_IMETHOD IsThirdPartyURI(nsIURI* uri, bool* aRes) override;
NS_IMETHOD IsThirdPartyPrincipal(nsIPrincipal* uri, bool* aRes) override;
NS_IMETHOD GetIsOriginPotentiallyTrustworthy(bool* aResult) override;

nsresult ToJSON(nsACString& aJSON);
static already_AddRefed<BasePrincipal> FromJSON(const nsACString& aJSON);
Expand Down
5 changes: 5 additions & 0 deletions caps/NullPrincipal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ NullPrincipal::GetURI(nsIURI** aURI) {
uri.forget(aURI);
return NS_OK;
}
NS_IMETHODIMP
NullPrincipal::GetIsOriginPotentiallyTrustworthy(bool* aResult) {
*aResult = false;
return NS_OK;
}

NS_IMETHODIMP
NullPrincipal::GetDomain(nsIURI** aDomain) {
Expand Down
1 change: 1 addition & 0 deletions caps/NullPrincipal.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class NullPrincipal final : public BasePrincipal {
NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
uint32_t GetHashValue() override;
NS_IMETHOD GetURI(nsIURI** aURI) override;
NS_IMETHOD GetIsOriginPotentiallyTrustworthy(bool* aResult) override;
NS_IMETHOD GetDomain(nsIURI** aDomain) override;
NS_IMETHOD SetDomain(nsIURI* aDomain) override;
NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
Expand Down
6 changes: 6 additions & 0 deletions caps/SystemPrincipal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ SystemPrincipal::GetURI(nsIURI** aURI) {
return NS_OK;
}

NS_IMETHODIMP
SystemPrincipal::GetIsOriginPotentiallyTrustworthy(bool* aResult) {
*aResult = true;
return NS_OK;
}

NS_IMETHODIMP
SystemPrincipal::GetDomain(nsIURI** aDomain) {
*aDomain = nullptr;
Expand Down
1 change: 1 addition & 0 deletions caps/SystemPrincipal.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class SystemPrincipal final : public BasePrincipal {
NS_IMETHOD SetDomain(nsIURI* aDomain) override;
NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
NS_IMETHOD GetAddonId(nsAString& aAddonId) override;
NS_IMETHOD GetIsOriginPotentiallyTrustworthy(bool* aResult) override;

virtual nsresult GetScriptLocation(nsACString& aStr) override;

Expand Down
12 changes: 12 additions & 0 deletions caps/nsIPrincipal.idl
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,18 @@ interface nsIPrincipal : nsISerializable
*/
bool IsURIInPrefList(in string pref);

/**
* Implementation of
* https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy
*
* 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.
*/
readonly attribute bool IsOriginPotentiallyTrustworthy;

/**
* Returns the Flags of the Principals
* associated AboutModule, in case there is one.
Expand Down
27 changes: 6 additions & 21 deletions dom/base/nsContentUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8891,18 +8891,9 @@ bool nsContentUtils::HttpsStateIsModern(Document* aDocument) {

MOZ_ASSERT(principal->GetIsContentPrincipal());

nsCOMPtr<nsIContentSecurityManager> csm =
do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID);
NS_WARNING_ASSERTION(csm, "csm is null");
if (csm) {
bool isTrustworthyOrigin = false;
csm->IsOriginPotentiallyTrustworthy(principal, &isTrustworthyOrigin);
if (isTrustworthyOrigin) {
return true;
}
}

return false;
bool isTrustworthyOrigin = false;
principal->GetIsOriginPotentiallyTrustworthy(&isTrustworthyOrigin);
return isTrustworthyOrigin;
}

/* static */
Expand Down Expand Up @@ -8932,15 +8923,9 @@ bool nsContentUtils::ComputeIsSecureContext(nsIChannel* aChannel) {
return false;
}

nsCOMPtr<nsIContentSecurityManager> csm =
do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID);
NS_WARNING_ASSERTION(csm, "csm is null");
if (csm) {
bool isTrustworthyOrigin = false;
csm->IsOriginPotentiallyTrustworthy(principal, &isTrustworthyOrigin);
return isTrustworthyOrigin;
}
return true;
bool isTrustworthyOrigin = false;
principal->GetIsOriginPotentiallyTrustworthy(&isTrustworthyOrigin);
return isTrustworthyOrigin;
}

/* static */
Expand Down
15 changes: 3 additions & 12 deletions dom/base/nsGlobalWindowOuter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1693,18 +1693,9 @@ bool nsGlobalWindowOuter::ComputeIsSecureContext(Document* aDocument,
}
}

nsCOMPtr<nsIContentSecurityManager> csm =
do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID);
NS_WARNING_ASSERTION(csm, "csm is null");
if (csm) {
bool isTrustworthyOrigin = false;
csm->IsOriginPotentiallyTrustworthy(principal, &isTrustworthyOrigin);
if (isTrustworthyOrigin) {
return true;
}
}

return false;
bool isTrustworthyOrigin = false;
principal->GetIsOriginPotentiallyTrustworthy(&isTrustworthyOrigin);
return isTrustworthyOrigin;
}

// We need certain special behavior for remote XUL whitelisted domains, but we
Expand Down
11 changes: 0 additions & 11 deletions dom/interfaces/security/nsIContentSecurityManager.idl
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,4 @@ interface nsIContentSecurityManager : nsISupports
nsIStreamListener performSecurityCheck(in nsIChannel aChannel,
in nsIStreamListener aStreamListener);

/**
* Implementation of
* https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy
*
* 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 isOriginPotentiallyTrustworthy(in nsIPrincipal aPrincipal);
};
8 changes: 1 addition & 7 deletions dom/presentation/PresentationRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -509,14 +509,8 @@ bool PresentationRequest::IsPrioriAuthenticatedURL(const nsAString& aUrl) {
return false;
}

nsCOMPtr<nsIContentSecurityManager> csm =
do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID);
if (NS_WARN_IF(!csm)) {
return false;
}

bool isTrustworthyOrigin = false;
csm->IsOriginPotentiallyTrustworthy(principal, &isTrustworthyOrigin);
principal->GetIsOriginPotentiallyTrustworthy(&isTrustworthyOrigin);
return isTrustworthyOrigin;
}

Expand Down
27 changes: 0 additions & 27 deletions dom/security/nsContentSecurityManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1051,30 +1051,3 @@ nsContentSecurityManager::PerformSecurityCheck(
inAndOutListener.forget(outStreamListener);
return NS_OK;
}

NS_IMETHODIMP
nsContentSecurityManager::IsOriginPotentiallyTrustworthy(
nsIPrincipal* aPrincipal, bool* aIsTrustWorthy) {
MOZ_ASSERT(NS_IsMainThread());
NS_ENSURE_ARG_POINTER(aPrincipal);
NS_ENSURE_ARG_POINTER(aIsTrustWorthy);

if (aPrincipal->IsSystemPrincipal()) {
*aIsTrustWorthy = true;
return NS_OK;
}
*aIsTrustWorthy = false;
if (aPrincipal->GetIsNullPrincipal()) {
return NS_OK;
}

MOZ_ASSERT(aPrincipal->GetIsContentPrincipal(),
"Nobody is expected to call us with an nsIExpandedPrincipal");

nsCOMPtr<nsIURI> uri;
nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
NS_ENSURE_SUCCESS(rv, rv);
*aIsTrustWorthy = nsMixedContentBlocker::IsPotentiallyTrustworthyOrigin(uri);

return NS_OK;
}
18 changes: 5 additions & 13 deletions dom/security/test/gtest/TestSecureContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithContentPrincipal)
rv = nsScriptSecurityManager::GetScriptSecurityManager()
->CreateContentPrincipalFromOrigin(uri, getter_AddRefs(prin));
bool isPotentiallyTrustworthy = false;
rv = csManager->IsOriginPotentiallyTrustworthy(prin,
&isPotentiallyTrustworthy);
rv = prin->GetIsOriginPotentiallyTrustworthy(&isPotentiallyTrustworthy);
ASSERT_EQ(NS_OK, rv);
ASSERT_EQ(isPotentiallyTrustworthy, uris[i].expectedResult);
}
Expand All @@ -82,14 +81,10 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithSystemPrincipal)
RefPtr<nsScriptSecurityManager> ssManager =
nsScriptSecurityManager::GetScriptSecurityManager();
ASSERT_TRUE(!!ssManager);
nsCOMPtr<nsIContentSecurityManager> csManager =
do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID);
ASSERT_TRUE(!!csManager);

nsCOMPtr<nsIPrincipal> sysPrin = nsContentUtils::GetSystemPrincipal();
bool isPotentiallyTrustworthy;
nsresult rv = csManager->IsOriginPotentiallyTrustworthy(
sysPrin, &isPotentiallyTrustworthy);
nsresult rv =
sysPrin->GetIsOriginPotentiallyTrustworthy(&isPotentiallyTrustworthy);
ASSERT_EQ(rv, NS_OK);
ASSERT_TRUE(isPotentiallyTrustworthy);
}
Expand All @@ -99,15 +94,12 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithNullPrincipal)
RefPtr<nsScriptSecurityManager> ssManager =
nsScriptSecurityManager::GetScriptSecurityManager();
ASSERT_TRUE(!!ssManager);
nsCOMPtr<nsIContentSecurityManager> csManager =
do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID);
ASSERT_TRUE(!!csManager);

RefPtr<NullPrincipal> nullPrin =
NullPrincipal::CreateWithoutOriginAttributes();
bool isPotentiallyTrustworthy;
nsresult rv = csManager->IsOriginPotentiallyTrustworthy(
nullPrin, &isPotentiallyTrustworthy);
nsresult rv =
nullPrin->GetIsOriginPotentiallyTrustworthy(&isPotentiallyTrustworthy);
ASSERT_EQ(rv, NS_OK);
ASSERT_TRUE(!isPotentiallyTrustworthy);
}
10 changes: 2 additions & 8 deletions dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,12 @@ add_task(async function test_isOriginPotentiallyTrustworthy() {
]) {
let uri = NetUtil.newURI(uriSpec);
let principal = gScriptSecurityManager.createContentPrincipal(uri, {});
Assert.equal(
gContentSecurityManager.isOriginPotentiallyTrustworthy(principal),
expectedResult
);
Assert.equal(principal.IsOriginPotentiallyTrustworthy, expectedResult);
}
// And now let's test whether .onion sites are properly treated when
// whitelisted, see bug 1382359.
Services.prefs.setBoolPref("dom.securecontext.whitelist_onions", true);
let uri = NetUtil.newURI("http://1234567890abcdef.onion/");
let principal = gScriptSecurityManager.createContentPrincipal(uri, {});
Assert.equal(
gContentSecurityManager.isOriginPotentiallyTrustworthy(principal),
true
);
Assert.equal(principal.IsOriginPotentiallyTrustworthy, true);
});
5 changes: 1 addition & 4 deletions toolkit/components/clearsitedata/ClearSiteData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,8 @@ void ClearSiteData::ClearDataFromChannel(nsIHttpChannel* aChannel) {
return;
}

nsCOMPtr<nsIContentSecurityManager> csm =
do_GetService(NS_CONTENTSECURITYMANAGER_CONTRACTID);

bool secure;
rv = csm->IsOriginPotentiallyTrustworthy(principal, &secure);
rv = principal->GetIsOriginPotentiallyTrustworthy(&secure);
if (NS_WARN_IF(NS_FAILED(rv)) || !secure) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,13 @@

add_task(
function test_isOriginPotentiallyTrustworthnsIContentSecurityManagery() {
let contentSecManager = Cc[
"@mozilla.org/contentsecuritymanager;1"
].getService(Ci.nsIContentSecurityManager);
let uri = NetUtil.newURI("moz-extension://foobar/something.html");
let principal = Services.scriptSecurityManager.createContentPrincipal(
uri,
{}
);
Assert.equal(
contentSecManager.isOriginPotentiallyTrustworthy(principal),
principal.IsOriginPotentiallyTrustworthy(),
true,
"it is potentially trustworthy"
);
Expand Down

0 comments on commit 3d4f0e2

Please sign in to comment.