diff options
| -rw-r--r-- | docshell/base/nsDSURIContentListener.cpp | 86 | ||||
| -rw-r--r-- | docshell/base/nsDSURIContentListener.h | 21 | ||||
| -rw-r--r-- | dom/base/nsDocument.cpp | 10 | ||||
| -rw-r--r-- | dom/base/nsDocument.h | 1 | ||||
| -rw-r--r-- | dom/interfaces/security/nsIContentSecurityPolicy.idl | 5 | ||||
| -rw-r--r-- | dom/locales/en-US/chrome/security/csp.properties | 4 | ||||
| -rw-r--r-- | dom/security/nsCSPContext.cpp | 14 | ||||
| -rw-r--r-- | dom/security/test/csp/file_ignore_xfo.html | 10 | ||||
| -rw-r--r-- | dom/security/test/csp/file_ignore_xfo.html^headers^ | 3 | ||||
| -rw-r--r-- | dom/security/test/csp/file_ro_ignore_xfo.html | 10 | ||||
| -rw-r--r-- | dom/security/test/csp/file_ro_ignore_xfo.html^headers^ | 3 | ||||
| -rw-r--r-- | dom/security/test/csp/mochitest.ini | 5 | ||||
| -rw-r--r-- | dom/security/test/csp/test_ignore_xfo.html | 59 |
13 files changed, 197 insertions, 34 deletions
diff --git a/docshell/base/nsDSURIContentListener.cpp b/docshell/base/nsDSURIContentListener.cpp index cfac54f7f4..93ce3cb264 100644 --- a/docshell/base/nsDSURIContentListener.cpp +++ b/docshell/base/nsDSURIContentListener.cpp @@ -22,6 +22,7 @@ #include "nsIScriptError.h" #include "nsDocShellLoadTypes.h" #include "nsIMultiPartChannel.h" +#include "mozilla/dom/nsCSPUtils.h" using namespace mozilla; @@ -84,14 +85,6 @@ nsDSURIContentListener::DoContent(const nsACString& aContentType, NS_ENSURE_ARG_POINTER(aContentHandler); NS_ENSURE_TRUE(mDocShell, NS_ERROR_FAILURE); - // Check whether X-Frame-Options permits us to load this content in an - // iframe and abort the load (unless we've disabled x-frame-options - // checking). - if (!CheckFrameOptions(aRequest)) { - *aAbortProcess = true; - return NS_OK; - } - *aAbortProcess = false; // determine if the channel has just been retargeted to us... @@ -265,9 +258,10 @@ nsDSURIContentListener::SetParentContentListener( return NS_OK; } -bool +/* static */ bool nsDSURIContentListener::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, - const nsAString& aPolicy) + const nsAString& aPolicy, + nsIDocShell* aDocShell) { static const char allowFrom[] = "allow-from"; const uint32_t allowFromLen = ArrayLength(allowFrom) - 1; @@ -285,7 +279,7 @@ nsDSURIContentListener::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, aHttpChannel->GetURI(getter_AddRefs(uri)); // XXXkhuey when does this happen? Is returning true safe here? - if (!mDocShell) { + if (!aDocShell) { return true; } @@ -293,7 +287,7 @@ nsDSURIContentListener::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, // window, if we're not the top. X-F-O: SAMEORIGIN requires that the // document must be same-origin with top window. X-F-O: DENY requires that // the document must never be framed. - nsCOMPtr<nsPIDOMWindowOuter> thisWindow = mDocShell->GetWindow(); + nsCOMPtr<nsPIDOMWindowOuter> thisWindow = aDocShell->GetWindow(); // If we don't have DOMWindow there is no risk of clickjacking if (!thisWindow) { return true; @@ -313,7 +307,7 @@ nsDSURIContentListener::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, // content-type docshell doesn't work because some chrome documents are // loaded in content docshells (see bug 593387). nsCOMPtr<nsIDocShellTreeItem> thisDocShellItem( - do_QueryInterface(static_cast<nsIDocShell*>(mDocShell))); + do_QueryInterface(static_cast<nsIDocShell*>(aDocShell))); nsCOMPtr<nsIDocShellTreeItem> parentDocShellItem; nsCOMPtr<nsIDocShellTreeItem> curDocShellItem = thisDocShellItem; nsCOMPtr<nsIDocument> topDoc; @@ -402,22 +396,66 @@ nsDSURIContentListener::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, return true; } +// Ignore x-frame-options if CSP with frame-ancestors exists +static bool +ShouldIgnoreFrameOptions(nsIChannel* aChannel, nsIPrincipal* aPrincipal) +{ + NS_ENSURE_TRUE(aChannel, false); + NS_ENSURE_TRUE(aPrincipal, false); + + nsCOMPtr<nsIContentSecurityPolicy> csp; + aPrincipal->GetCsp(getter_AddRefs(csp)); + if (!csp) { + // if there is no CSP, then there is nothing to do here + return false; + } + + bool enforcesFrameAncestors = false; + csp->GetEnforcesFrameAncestors(&enforcesFrameAncestors); + if (!enforcesFrameAncestors) { + // if CSP does not contain frame-ancestors, then there + // is nothing to do here. + return false; + } + + // log warning to console that xfo is ignored because of CSP + nsCOMPtr<nsILoadInfo> loadInfo = aChannel->GetLoadInfo(); + uint64_t innerWindowID = loadInfo ? loadInfo->GetInnerWindowID() : 0; + const char16_t* params[] = { u"x-frame-options", + u"frame-ancestors" }; + CSP_LogLocalizedStr(u"IgnoringSrcBecauseOfDirective", + params, ArrayLength(params), + EmptyString(), // no sourcefile + EmptyString(), // no scriptsample + 0, // no linenumber + 0, // no columnnumber + nsIScriptError::warningFlag, + "CSP", innerWindowID); + + return true; +} + // Check if X-Frame-Options permits this document to be loaded as a subdocument. // This will iterate through and check any number of X-Frame-Options policies // in the request (comma-separated in a header, multiple headers, etc). -bool -nsDSURIContentListener::CheckFrameOptions(nsIRequest* aRequest) +/* static */ bool +nsDSURIContentListener::CheckFrameOptions(nsIChannel* aChannel, + nsIDocShell* aDocShell, + nsIPrincipal* aPrincipal) { - nsresult rv; - nsCOMPtr<nsIChannel> chan = do_QueryInterface(aRequest); - if (!chan) { + if (!aChannel || !aDocShell) { return true; } - nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(chan); + if (ShouldIgnoreFrameOptions(aChannel, aPrincipal)) { + return true; + } + + nsresult rv; + nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel); if (!httpChannel) { // check if it is hiding in a multipart channel - rv = mDocShell->GetHttpChannel(chan, getter_AddRefs(httpChannel)); + rv = nsDocShell::Cast(aDocShell)->GetHttpChannel(aChannel, getter_AddRefs(httpChannel)); if (NS_FAILED(rv)) { return false; } @@ -442,11 +480,11 @@ nsDSURIContentListener::CheckFrameOptions(nsIRequest* aRequest) nsCharSeparatedTokenizer tokenizer(xfoHeaderValue, ','); while (tokenizer.hasMoreTokens()) { const nsSubstring& tok = tokenizer.nextToken(); - if (!CheckOneFrameOptionsPolicy(httpChannel, tok)) { + if (!CheckOneFrameOptionsPolicy(httpChannel, tok, aDocShell)) { // cancel the load and display about:blank httpChannel->Cancel(NS_BINDING_ABORTED); - if (mDocShell) { - nsCOMPtr<nsIWebNavigation> webNav(do_QueryObject(mDocShell)); + if (aDocShell) { + nsCOMPtr<nsIWebNavigation> webNav(do_QueryObject(aDocShell)); if (webNav) { webNav->LoadURI(u"about:blank", 0, nullptr, nullptr, nullptr); @@ -459,7 +497,7 @@ nsDSURIContentListener::CheckFrameOptions(nsIRequest* aRequest) return true; } -void +/* static */ void nsDSURIContentListener::ReportXFOViolation(nsIDocShellTreeItem* aTopDocShellItem, nsIURI* aThisURI, XFOHeader aHeader) diff --git a/docshell/base/nsDSURIContentListener.h b/docshell/base/nsDSURIContentListener.h index f33d1c045e..4328134710 100644 --- a/docshell/base/nsDSURIContentListener.h +++ b/docshell/base/nsDSURIContentListener.h @@ -28,6 +28,12 @@ public: nsresult Init(); + // Determine if X-Frame-Options allows content to be framed + // as a subdocument + static bool CheckFrameOptions(nsIChannel* aChannel, + nsIDocShell* aDocShell, + nsIPrincipal* aPrincipal); + protected: explicit nsDSURIContentListener(nsDocShell* aDocShell); virtual ~nsDSURIContentListener(); @@ -39,12 +45,9 @@ protected: mExistingJPEGStreamListener = nullptr; } - // Determine if X-Frame-Options allows content to be framed - // as a subdocument - bool CheckFrameOptions(nsIRequest* aRequest); - bool CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, - const nsAString& aPolicy); - + static bool CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, + const nsAString& aPolicy, + nsIDocShell* aDocShell); enum XFOHeader { eDENY, @@ -52,9 +55,9 @@ protected: eALLOWFROM }; - void ReportXFOViolation(nsIDocShellTreeItem* aTopDocShellItem, - nsIURI* aThisURI, - XFOHeader aHeader); + static void ReportXFOViolation(nsIDocShellTreeItem* aTopDocShellItem, + nsIURI* aThisURI, + XFOHeader aHeader); protected: nsDocShell* mDocShell; diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 8e6920a0e9..4926b6c0a3 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -61,6 +61,7 @@ #include "nsGenericHTMLElement.h" #include "mozilla/dom/CDATASection.h" #include "mozilla/dom/ProcessingInstruction.h" +#include "nsDSURIContentListener.h" #include "nsDOMString.h" #include "nsNodeUtils.h" #include "nsLayoutUtils.h" // for GetFrameForPoint @@ -2456,6 +2457,15 @@ nsDocument::StartDocumentLoad(const char* aCommand, nsIChannel* aChannel, NS_ENSURE_SUCCESS(rv, rv); } + // XFO needs to be checked after CSP because it is ignored if + // the CSP defines frame-ancestors. + if (!nsDSURIContentListener::CheckFrameOptions(aChannel, docShell, NodePrincipal())) { + MOZ_LOG(gCspPRLog, LogLevel::Debug, + ("XFO doesn't like frame's ancestry, not loading.")); + // stop! ERROR page! + aChannel->Cancel(NS_ERROR_CSP_FRAME_ANCESTOR_VIOLATION); + } + return NS_OK; } diff --git a/dom/base/nsDocument.h b/dom/base/nsDocument.h index 17d936055c..fc6749c9f0 100644 --- a/dom/base/nsDocument.h +++ b/dom/base/nsDocument.h @@ -1491,7 +1491,6 @@ private: void PostUnblockOnloadEvent(); void DoUnblockOnload(); - nsresult CheckFrameOptions(); nsresult InitCSP(nsIChannel* aChannel); /** diff --git a/dom/interfaces/security/nsIContentSecurityPolicy.idl b/dom/interfaces/security/nsIContentSecurityPolicy.idl index ade5b12439..51ca46f2a8 100644 --- a/dom/interfaces/security/nsIContentSecurityPolicy.idl +++ b/dom/interfaces/security/nsIContentSecurityPolicy.idl @@ -98,6 +98,11 @@ interface nsIContentSecurityPolicy : nsISerializable readonly attribute bool blockAllMixedContent; /** + * Returns whether this policy enforces the frame-ancestors directive. + */ + readonly attribute bool enforcesFrameAncestors; + + /** * Obtains the referrer policy (as integer) for this browsing context as * specified in CSP. If there are multiple policies and... * - only one sets a referrer policy: that policy is returned diff --git a/dom/locales/en-US/chrome/security/csp.properties b/dom/locales/en-US/chrome/security/csp.properties index fc7fc04ba2..4124ef8aac 100644 --- a/dom/locales/en-US/chrome/security/csp.properties +++ b/dom/locales/en-US/chrome/security/csp.properties @@ -91,6 +91,10 @@ ignoringReportOnlyDirective = Ignoring sandbox directive when delivered in a rep # LOCALIZATION NOTE (deprecatedReferrerDirective): # %1$S is the value of the deprecated Referrer Directive. deprecatedReferrerDirective = Referrer Directive ‘%1$S’ has been deprecated. Please use the Referrer-Policy header instead. +# LOCALIZATION NOTE (IgnoringSrcBecauseOfDirective): +# %1$S is the name of the src that is ignored. +# %2$S is the name of the directive that causes the src to be ignored. +IgnoringSrcBecauseOfDirective=Ignoring ‘%1$S’ because of ‘%2$S’ directive. # CSP Errors: # LOCALIZATION NOTE (couldntParseInvalidSource): diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index 815c7734d0..0a3e20305d 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -343,6 +343,20 @@ nsCSPContext::GetBlockAllMixedContent(bool *outBlockAllMixedContent) } NS_IMETHODIMP +nsCSPContext::GetEnforcesFrameAncestors(bool *outEnforcesFrameAncestors) +{ + *outEnforcesFrameAncestors = false; + for (uint32_t i = 0; i < mPolicies.Length(); i++) { + if (!mPolicies[i]->getReportOnlyFlag() && + mPolicies[i]->hasDirective(nsIContentSecurityPolicy::FRAME_ANCESTORS_DIRECTIVE)) { + *outEnforcesFrameAncestors = true; + return NS_OK; + } + } + return NS_OK; +} + +NS_IMETHODIMP nsCSPContext::GetReferrerPolicy(uint32_t* outPolicy, bool* outIsSet) { *outIsSet = false; diff --git a/dom/security/test/csp/file_ignore_xfo.html b/dom/security/test/csp/file_ignore_xfo.html new file mode 100644 index 0000000000..6746a3adba --- /dev/null +++ b/dom/security/test/csp/file_ignore_xfo.html @@ -0,0 +1,10 @@ +<!DOCTYPE HTML> +<html> +<head> + <meta charset="utf-8"> + <title>Bug 1024557: Ignore x-frame-options if CSP with frame-ancestors exists</title> +</head> +<body> +<div id="cspmessage">Ignoring XFO because of CSP</div> +</body> +</html> diff --git a/dom/security/test/csp/file_ignore_xfo.html^headers^ b/dom/security/test/csp/file_ignore_xfo.html^headers^ new file mode 100644 index 0000000000..e93f9e3ecb --- /dev/null +++ b/dom/security/test/csp/file_ignore_xfo.html^headers^ @@ -0,0 +1,3 @@ +Content-Security-Policy: frame-ancestors http://mochi.test:8888 +X-Frame-Options: deny +Cache-Control: no-cache diff --git a/dom/security/test/csp/file_ro_ignore_xfo.html b/dom/security/test/csp/file_ro_ignore_xfo.html new file mode 100644 index 0000000000..85e7f0092c --- /dev/null +++ b/dom/security/test/csp/file_ro_ignore_xfo.html @@ -0,0 +1,10 @@ +<!DOCTYPE HTML> +<html> +<head> + <meta charset="utf-8"> + <title>Bug 1024557: Ignore x-frame-options if CSP with frame-ancestors exists</title> +</head> +<body> +<div id="cspmessage">Ignoring XFO because of CSP_RO</div> +</body> +</html>
\ No newline at end of file diff --git a/dom/security/test/csp/file_ro_ignore_xfo.html^headers^ b/dom/security/test/csp/file_ro_ignore_xfo.html^headers^ new file mode 100644 index 0000000000..ab8366f061 --- /dev/null +++ b/dom/security/test/csp/file_ro_ignore_xfo.html^headers^ @@ -0,0 +1,3 @@ +Content-Security-Policy-Report-Only: frame-ancestors http://mochi.test:8888 +X-Frame-Options: deny +Cache-Control: no-cache diff --git a/dom/security/test/csp/mochitest.ini b/dom/security/test/csp/mochitest.ini index 8add999c30..5351097521 100644 --- a/dom/security/test/csp/mochitest.ini +++ b/dom/security/test/csp/mochitest.ini @@ -206,6 +206,10 @@ support-files = file_iframe_srcdoc.sjs file_iframe_sandbox_srcdoc.html file_iframe_sandbox_srcdoc.html^headers^ + file_ignore_xfo.html + file_ignore_xfo.html^headers^ + file_ro_ignore_xfo.html + file_ro_ignore_xfo.html^headers^ [test_base-uri.html] [test_blob_data_schemes.html] @@ -298,3 +302,4 @@ tags = mcb support-files = file_sandbox_allow_scripts.html file_sandbox_allow_scripts.html^headers^ +[test_ignore_xfo.html] diff --git a/dom/security/test/csp/test_ignore_xfo.html b/dom/security/test/csp/test_ignore_xfo.html new file mode 100644 index 0000000000..fb3aadc6c2 --- /dev/null +++ b/dom/security/test/csp/test_ignore_xfo.html @@ -0,0 +1,59 @@ +<!DOCTYPE HTML> +<html> +<head> + <title>Bug 1024557: Ignore x-frame-options if CSP with frame-ancestors exists</title> + <!-- Including SimpleTest.js so we can use waitForExplicitFinish !--> + <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script> + <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" /> +</head> +<body> +<iframe style="width:100%;" id="csp_testframe"></iframe> +<iframe style="width:100%;" id="csp_ro_testframe"></iframe> + +<script class="testbody" type="text/javascript"> + +/* + * We load two frames using: + * x-frame-options: deny + * where the first frame uses a csp and the second a csp_ro including frame-ancestors. + * We make sure that xfo is ignored for regular csp but not for csp_ro. + */ + +SimpleTest.waitForExplicitFinish(); + +var testcounter = 0; +function checkFinished() { + testcounter++; + if (testcounter < 2) { + return; + } + SimpleTest.finish(); +} + +// 1) test XFO with CSP +var csp_testframe = document.getElementById("csp_testframe"); +csp_testframe.onload = function() { + var msg = csp_testframe.contentWindow.document.getElementById("cspmessage"); + is(msg.innerHTML, "Ignoring XFO because of CSP", "Loading frame with with XFO and CSP"); + checkFinished(); +} +csp_testframe.onerror = function() { + ok(false, "sanity: should not fire onerror for csp_testframe"); +} +csp_testframe.src = "file_ignore_xfo.html"; + +// 2) test XFO with CSP_RO +var csp_ro_testframe = document.getElementById("csp_ro_testframe"); +csp_ro_testframe.onload = function() { + var msg = csp_ro_testframe.contentWindow.document.getElementById("cspmessage"); + is(msg, null, "Blocking frame with with XFO and CSP_RO"); + checkFinished(); +} +csp_ro_testframe.onerror = function() { + ok(false, "sanity: should not fire onerror for csp_ro_testframe"); +} +csp_ro_testframe.src = "file_ro_ignore_xfo.html"; + +</script> +</body> +</html> |