From 62d535967977ea64884e4418d78f1dc245e682e1 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Fri, 25 Aug 2017 09:18:29 +0200 Subject: CSP 2 - ignore (x-)frame-options if CSP with frame-ancestors directive exists --- docshell/base/nsDSURIContentListener.cpp | 86 ++++++++++++++++------ docshell/base/nsDSURIContentListener.h | 21 +++--- dom/base/nsDocument.cpp | 10 +++ dom/base/nsDocument.h | 1 - .../security/nsIContentSecurityPolicy.idl | 5 ++ dom/locales/en-US/chrome/security/csp.properties | 4 + dom/security/nsCSPContext.cpp | 14 ++++ dom/security/test/csp/file_ignore_xfo.html | 10 +++ .../test/csp/file_ignore_xfo.html^headers^ | 3 + dom/security/test/csp/file_ro_ignore_xfo.html | 10 +++ .../test/csp/file_ro_ignore_xfo.html^headers^ | 3 + dom/security/test/csp/mochitest.ini | 5 ++ dom/security/test/csp/test_ignore_xfo.html | 59 +++++++++++++++ 13 files changed, 197 insertions(+), 34 deletions(-) create mode 100644 dom/security/test/csp/file_ignore_xfo.html create mode 100644 dom/security/test/csp/file_ignore_xfo.html^headers^ create mode 100644 dom/security/test/csp/file_ro_ignore_xfo.html create mode 100644 dom/security/test/csp/file_ro_ignore_xfo.html^headers^ create mode 100644 dom/security/test/csp/test_ignore_xfo.html diff --git a/docshell/base/nsDSURIContentListener.cpp b/docshell/base/nsDSURIContentListener.cpp index cfac54f7f..93ce3cb26 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 thisWindow = mDocShell->GetWindow(); + nsCOMPtr 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 thisDocShellItem( - do_QueryInterface(static_cast(mDocShell))); + do_QueryInterface(static_cast(aDocShell))); nsCOMPtr parentDocShellItem; nsCOMPtr curDocShellItem = thisDocShellItem; nsCOMPtr 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 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 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 chan = do_QueryInterface(aRequest); - if (!chan) { + if (!aChannel || !aDocShell) { return true; } - nsCOMPtr httpChannel = do_QueryInterface(chan); + if (ShouldIgnoreFrameOptions(aChannel, aPrincipal)) { + return true; + } + + nsresult rv; + nsCOMPtr 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 webNav(do_QueryObject(mDocShell)); + if (aDocShell) { + nsCOMPtr 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 f33d1c045..432813471 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 8e6920a0e..4926b6c0a 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 17d936055..fc6749c9f 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 ade5b1243..51ca46f2a 100644 --- a/dom/interfaces/security/nsIContentSecurityPolicy.idl +++ b/dom/interfaces/security/nsIContentSecurityPolicy.idl @@ -97,6 +97,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... diff --git a/dom/locales/en-US/chrome/security/csp.properties b/dom/locales/en-US/chrome/security/csp.properties index fc7fc04ba..4124ef8aa 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 815c7734d..0a3e20305 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -342,6 +342,20 @@ nsCSPContext::GetBlockAllMixedContent(bool *outBlockAllMixedContent) return NS_OK; } +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) { 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 000000000..6746a3adb --- /dev/null +++ b/dom/security/test/csp/file_ignore_xfo.html @@ -0,0 +1,10 @@ + + + + + Bug 1024557: Ignore x-frame-options if CSP with frame-ancestors exists + + +
Ignoring XFO because of CSP
+ + 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 000000000..e93f9e3ec --- /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 000000000..85e7f0092 --- /dev/null +++ b/dom/security/test/csp/file_ro_ignore_xfo.html @@ -0,0 +1,10 @@ + + + + + Bug 1024557: Ignore x-frame-options if CSP with frame-ancestors exists + + +
Ignoring XFO because of CSP_RO
+ + \ 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 000000000..ab8366f06 --- /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 8add999c3..535109752 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 000000000..fb3aadc6c --- /dev/null +++ b/dom/security/test/csp/test_ignore_xfo.html @@ -0,0 +1,59 @@ + + + + Bug 1024557: Ignore x-frame-options if CSP with frame-ancestors exists + + + + + + + + + + + -- cgit v1.2.3