From b6387305a44c28b9faf8412eefd81296a251ce71 Mon Sep 17 00:00:00 2001 From: Christoph Kerschbaumer Date: Fri, 7 Aug 2020 11:27:56 +0000 Subject: [PATCH] Bug 1657583: Simplify TestSitePermission within nsHTTPSOnlyUtils r=necko-reviewers,JulianWels,dragana Differential Revision: https://phabricator.services.mozilla.com/D86178 --- docshell/base/nsDocShell.cpp | 22 +++-------- dom/security/nsHTTPSOnlyUtils.cpp | 55 +++++++++++++++++++++++----- dom/security/nsHTTPSOnlyUtils.h | 9 +++-- netwerk/ipc/DocumentLoadListener.cpp | 29 +-------------- 4 files changed, 56 insertions(+), 59 deletions(-) diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 2daa843a215b..ffe890e8ce28 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -9183,23 +9183,6 @@ nsIPrincipal* nsDocShell::GetInheritedPrincipal( aLoadInfo->SetIsFormSubmission(true); } - // If the HTTPS-Only mode is enabled, every insecure request gets upgraded to - // HTTPS by default. This behavior can be disabled through the loadinfo flag - // HTTPS_ONLY_EXEMPT. - bool isPrivateWin = attrs.mPrivateBrowsingId > 0; - if (nsHTTPSOnlyUtils::IsHttpsOnlyModeEnabled(isPrivateWin)) { - // Let's create a new content principal based on the URI for the - // PermissionManager - nsCOMPtr permissionPrincipal = - BasePrincipal::CreateContentPrincipal(aLoadState->URI(), attrs); - - if (nsHTTPSOnlyUtils::TestHttpsOnlySitePermission(permissionPrincipal)) { - uint32_t httpsOnlyStatus = aLoadInfo->GetHttpsOnlyStatus(); - httpsOnlyStatus |= nsILoadInfo::HTTPS_ONLY_EXEMPT; - aLoadInfo->SetHttpsOnlyStatus(httpsOnlyStatus); - } - } - nsCOMPtr channel; aRv = CreateRealChannelForDocument(getter_AddRefs(channel), aLoadState->URI(), aLoadInfo, aCallbacks, aLoadFlags, srcdoc, @@ -9210,6 +9193,11 @@ nsIPrincipal* nsDocShell::GetInheritedPrincipal( return false; } + // If the HTTPS-Only mode is enabled, every insecure request gets upgraded to + // HTTPS by default. This behavior can be disabled through the loadinfo flag + // HTTPS_ONLY_EXEMPT. + nsHTTPSOnlyUtils::TestSitePermissionAndPotentiallyAddExemption(channel); + if (nsCOMPtr appCacheChannel = do_QueryInterface(channel)) { // Any document load should not inherit application cache. diff --git a/dom/security/nsHTTPSOnlyUtils.cpp b/dom/security/nsHTTPSOnlyUtils.cpp index 39be06f78ef5..0dc2cddc50f1 100644 --- a/dom/security/nsHTTPSOnlyUtils.cpp +++ b/dom/security/nsHTTPSOnlyUtils.cpp @@ -9,6 +9,7 @@ #include "nsContentUtils.h" #include "nsHTTPSOnlyUtils.h" #include "nsIConsoleService.h" +#include "nsIHttpChannel.h" #include "nsIHttpsOnlyModePermission.h" #include "nsIPermissionManager.h" #include "nsIScriptError.h" @@ -164,23 +165,57 @@ bool nsHTTPSOnlyUtils::CouldBeHttpsOnlyError(nsIChannel* aChannel, } /* static */ -bool nsHTTPSOnlyUtils::TestHttpsOnlySitePermission(nsIPrincipal* aPrincipal) { - if (!aPrincipal) { - // We always deny the permission if we don't have a principal. - return false; +void nsHTTPSOnlyUtils::TestSitePermissionAndPotentiallyAddExemption( + nsIChannel* aChannel) { + NS_ENSURE_TRUE_VOID(aChannel); + + // if https-only mode is not enabled, then there is nothing to do here. + nsCOMPtr loadInfo = aChannel->LoadInfo(); + bool isPrivateWin = loadInfo->GetOriginAttributes().mPrivateBrowsingId > 0; + if (!IsHttpsOnlyModeEnabled(isPrivateWin)) { + return; } + // if it's not a top-level or iframe load then there is nothing to here. + nsContentPolicyType type = loadInfo->GetExternalContentPolicyType(); + if (type != nsIContentPolicy::TYPE_DOCUMENT && + type != nsIContentPolicy::TYPE_SUBDOCUMENT) { + return; + } + + // it it's not an http channel, then there is nothing to do here. + nsCOMPtr httpChannel = do_QueryInterface(aChannel); + if (!httpChannel) { + return; + } + + nsCOMPtr principal; + nsresult rv = nsContentUtils::GetSecurityManager()->GetChannelResultPrincipal( + aChannel, getter_AddRefs(principal)); + NS_ENSURE_SUCCESS_VOID(rv); + nsCOMPtr permMgr = mozilla::services::GetPermissionManager(); - NS_ENSURE_TRUE(permMgr, false); + NS_ENSURE_TRUE_VOID(permMgr); uint32_t perm; - nsresult rv = permMgr->TestExactPermissionFromPrincipal( - aPrincipal, "https-only-load-insecure"_ns, &perm); - NS_ENSURE_SUCCESS(rv, false); + rv = permMgr->TestExactPermissionFromPrincipal( + principal, "https-only-load-insecure"_ns, &perm); + NS_ENSURE_SUCCESS_VOID(rv); - return perm == nsIHttpsOnlyModePermission::LOAD_INSECURE_ALLOW || - perm == nsIHttpsOnlyModePermission::LOAD_INSECURE_ALLOW_SESSION; + bool isHttpsOnlyExempt = + perm == nsIHttpsOnlyModePermission::LOAD_INSECURE_ALLOW || + perm == nsIHttpsOnlyModePermission::LOAD_INSECURE_ALLOW_SESSION; + + // We explicitly add or also remove the exemption flag, because this + // function is also consulted after redirects. + uint32_t httpsOnlyStatus = loadInfo->GetHttpsOnlyStatus(); + if (isHttpsOnlyExempt) { + httpsOnlyStatus |= nsILoadInfo::HTTPS_ONLY_EXEMPT; + } else { + httpsOnlyStatus &= ~nsILoadInfo::HTTPS_ONLY_EXEMPT; + } + loadInfo->SetHttpsOnlyStatus(httpsOnlyStatus); } /* ------ Logging ------ */ diff --git a/dom/security/nsHTTPSOnlyUtils.h b/dom/security/nsHTTPSOnlyUtils.h index 85f361988e28..058d6839f0a7 100644 --- a/dom/security/nsHTTPSOnlyUtils.h +++ b/dom/security/nsHTTPSOnlyUtils.h @@ -66,11 +66,12 @@ class nsHTTPSOnlyUtils { nsIURI* aURI = nullptr); /** - * Tests is the HTTPS-Only Mode upgrade exception is set for a given principal - * @param aPrincipal Principal to check permission for - * @return true if exempt from upgrade + * Tests if the HTTPS-Only Mode upgrade exception is set for a given channel. + * Note: This function only adds an exemption for loads of TYPE_DOCUMENT. + * @param aChannel The channel to be checked */ - static bool TestHttpsOnlySitePermission(nsIPrincipal* aPrincipal); + static void TestSitePermissionAndPotentiallyAddExemption( + nsIChannel* aChannel); private: /** diff --git a/netwerk/ipc/DocumentLoadListener.cpp b/netwerk/ipc/DocumentLoadListener.cpp index 99adf2573e34..b1983caef932 100644 --- a/netwerk/ipc/DocumentLoadListener.cpp +++ b/netwerk/ipc/DocumentLoadListener.cpp @@ -21,7 +21,6 @@ #include "mozilla/dom/ClientChannelHelper.h" #include "mozilla/dom/ContentParent.h" #include "mozilla/dom/ContentProcessManager.h" -#include "mozilla/dom/nsHTTPSOnlyUtils.h" #include "mozilla/dom/SessionHistoryEntry.h" #include "mozilla/dom/WindowGlobalParent.h" #include "mozilla/dom/ipc/IdType.h" @@ -2275,33 +2274,7 @@ DocumentLoadListener::AsyncOnChannelRedirect( // If HTTPS-Only mode is enabled, we need to check whether the exception-flag // needs to be removed or set, by asking the PermissionManager. - RefPtr bc = - mParentChannelListener->GetBrowsingContext(); - nsCOMPtr channelLoadInfo = mChannel->LoadInfo(); - bool isPrivateWin = - channelLoadInfo->GetOriginAttributes().mPrivateBrowsingId > 0; - if (nsHTTPSOnlyUtils::IsHttpsOnlyModeEnabled(isPrivateWin) && bc && - bc->IsTop()) { - bool isHttpsOnlyExempt = false; - if (httpChannel) { - nsCOMPtr resultPrincipal; - nsresult rv = - nsContentUtils::GetSecurityManager()->GetChannelResultPrincipal( - mChannel, getter_AddRefs(resultPrincipal)); - if (NS_SUCCEEDED(rv)) { - isHttpsOnlyExempt = - nsHTTPSOnlyUtils::TestHttpsOnlySitePermission(resultPrincipal); - } - } - - uint32_t httpsOnlyStatus = channelLoadInfo->GetHttpsOnlyStatus(); - if (isHttpsOnlyExempt) { - httpsOnlyStatus |= nsILoadInfo::HTTPS_ONLY_EXEMPT; - } else { - httpsOnlyStatus &= ~nsILoadInfo::HTTPS_ONLY_EXEMPT; - } - channelLoadInfo->SetHttpsOnlyStatus(httpsOnlyStatus); - } + nsHTTPSOnlyUtils::TestSitePermissionAndPotentiallyAddExemption(mChannel); // We don't need to confirm internal redirects or record any // history for them, so just immediately verify and return.