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 From a06ce3f03b260d59199dba7e01ea8afb3de1ef59 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Fri, 25 Aug 2017 09:25:03 +0200 Subject: CSP: Upgrade SO navigational requests per spec. --- docshell/base/nsDocShell.cpp | 23 +++++ .../test/csp/file_upgrade_insecure_navigation.sjs | 79 ++++++++++++++++ dom/security/test/csp/mochitest.ini | 2 + .../test/csp/test_upgrade_insecure_navigation.html | 103 +++++++++++++++++++++ netwerk/base/LoadInfo.cpp | 6 ++ netwerk/base/LoadInfo.h | 1 + 6 files changed, 214 insertions(+) create mode 100644 dom/security/test/csp/file_upgrade_insecure_navigation.sjs create mode 100644 dom/security/test/csp/test_upgrade_insecure_navigation.html diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index ab119a016..2e08e6720 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -11025,6 +11025,29 @@ nsDocShell::DoURILoad(nsIURI* aURI, } } + // Navigational requests that are same origin need to be upgraded in case + // upgrade-insecure-requests is present. Please note that in that case + // the triggeringPrincipal is holding the CSP that potentially + // holds upgrade-insecure-requests. + nsCOMPtr csp; + aTriggeringPrincipal->GetCsp(getter_AddRefs(csp)); + if (csp) { + bool upgradeInsecureRequests = false; + csp->GetUpgradeInsecureRequests(&upgradeInsecureRequests); + if (upgradeInsecureRequests) { + // only upgrade if the navigation is same origin + nsCOMPtr resultPrincipal; + rv = nsContentUtils::GetSecurityManager()-> + GetChannelResultPrincipal(channel, + getter_AddRefs(resultPrincipal)); + NS_ENSURE_SUCCESS(rv, rv); + if (resultPrincipal->Equals(aTriggeringPrincipal)) { + static_cast(loadInfo.get())->SetUpgradeInsecureRequests(); + } + } + } + + nsCOMPtr appCacheChannel = do_QueryInterface(channel); if (appCacheChannel) { diff --git a/dom/security/test/csp/file_upgrade_insecure_navigation.sjs b/dom/security/test/csp/file_upgrade_insecure_navigation.sjs new file mode 100644 index 000000000..51afa39bf --- /dev/null +++ b/dom/security/test/csp/file_upgrade_insecure_navigation.sjs @@ -0,0 +1,79 @@ +// Custom *.sjs file specifically for the needs of +// https://bugzilla.mozilla.org/show_bug.cgi?id=1271173 + +"use strict"; +Components.utils.importGlobalProperties(["URLSearchParams"]); + +const TEST_NAVIGATIONAL_UPGRADE = ` + + + + + clickme + + + `; + +const FRAME_NAV = ` + + + + + + + `; + +const DOC_NAV = ` + + + + + + + `; + +function handleRequest(request, response) { + const query = new URLSearchParams(request.queryString); + + response.setHeader("Cache-Control", "no-cache", false); + response.setHeader("Content-Type", "text/html", false); + if (query.get("csp")) { + response.setHeader("Content-Security-Policy", query.get("csp"), false); + } + + if (query.get("action") === "perform_navigation") { + response.write(TEST_NAVIGATIONAL_UPGRADE); + return; + } + + if (query.get("action") === "framenav") { + response.write(FRAME_NAV); + return; + } + + if (query.get("action") === "docnav") { + response.write(DOC_NAV); + return; + } + + // we should never get here, but just in case + // return something unexpected + response.write("do'h"); +} diff --git a/dom/security/test/csp/mochitest.ini b/dom/security/test/csp/mochitest.ini index 535109752..04401b063 100644 --- a/dom/security/test/csp/mochitest.ini +++ b/dom/security/test/csp/mochitest.ini @@ -210,6 +210,7 @@ support-files = file_ignore_xfo.html^headers^ file_ro_ignore_xfo.html file_ro_ignore_xfo.html^headers^ + file_upgrade_insecure_navigation.sjs [test_base-uri.html] [test_blob_data_schemes.html] @@ -296,6 +297,7 @@ tags = mcb [test_strict_dynamic.html] [test_strict_dynamic_parser_inserted.html] [test_strict_dynamic_default_src.html] +[test_upgrade_insecure_navigation.html] [test_iframe_sandbox_srcdoc.html] [test_iframe_srcdoc.html] [test_sandbox_allow_scripts.html] diff --git a/dom/security/test/csp/test_upgrade_insecure_navigation.html b/dom/security/test/csp/test_upgrade_insecure_navigation.html new file mode 100644 index 000000000..db6a6a1be --- /dev/null +++ b/dom/security/test/csp/test_upgrade_insecure_navigation.html @@ -0,0 +1,103 @@ + + + + Bug 1271173 - Missing spec on Upgrade Insecure Requests(Navigational Upgrades) + + + + + + + + + + + diff --git a/netwerk/base/LoadInfo.cpp b/netwerk/base/LoadInfo.cpp index 216cf559c..42fdea4a1 100644 --- a/netwerk/base/LoadInfo.cpp +++ b/netwerk/base/LoadInfo.cpp @@ -859,6 +859,12 @@ LoadInfo::SetIsPreflight() mIsPreflight = true; } +void +LoadInfo::SetUpgradeInsecureRequests() +{ + mUpgradeInsecureRequests = true; +} + NS_IMETHODIMP LoadInfo::GetIsPreflight(bool* aIsPreflight) { diff --git a/netwerk/base/LoadInfo.h b/netwerk/base/LoadInfo.h index 261f85349..3e1b92ff4 100644 --- a/netwerk/base/LoadInfo.h +++ b/netwerk/base/LoadInfo.h @@ -78,6 +78,7 @@ public: already_AddRefed CloneForNewRequest() const; void SetIsPreflight(); + void SetUpgradeInsecureRequests(); private: // private constructor that is only allowed to be called from within -- cgit v1.2.3 From b4dac5093a75a024643b93aef88758770df73c55 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Fri, 25 Aug 2017 09:36:20 +0200 Subject: CSP: Ignore nonces on per spec --- dom/security/nsCSPContext.cpp | 11 ++-- dom/security/test/csp/file_image_nonce.html | 39 ++++++++++++++ .../test/csp/file_image_nonce.html^headers^ | 2 + dom/security/test/csp/mochitest.ini | 3 ++ dom/security/test/csp/test_image_nonce.html | 60 ++++++++++++++++++++++ 5 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 dom/security/test/csp/file_image_nonce.html create mode 100644 dom/security/test/csp/file_image_nonce.html^headers^ create mode 100644 dom/security/test/csp/test_image_nonce.html diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index 0a3e20305..5e435d4ca 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -156,10 +156,13 @@ nsCSPContext::ShouldLoad(nsContentPolicyType aContentType, nsAutoString nonce; bool parserCreated = false; if (!isPreload) { - nsCOMPtr htmlElement = do_QueryInterface(aRequestContext); - if (htmlElement) { - rv = htmlElement->GetAttribute(NS_LITERAL_STRING("nonce"), nonce); - NS_ENSURE_SUCCESS(rv, rv); + if (aContentType == nsIContentPolicy::TYPE_SCRIPT || + aContentType == nsIContentPolicy::TYPE_STYLESHEET) { + nsCOMPtr htmlElement = do_QueryInterface(aRequestContext); + if (htmlElement) { + rv = htmlElement->GetAttribute(NS_LITERAL_STRING("nonce"), nonce); + NS_ENSURE_SUCCESS(rv, rv); + } } nsCOMPtr script = do_QueryInterface(aRequestContext); diff --git a/dom/security/test/csp/file_image_nonce.html b/dom/security/test/csp/file_image_nonce.html new file mode 100644 index 000000000..5d57bb837 --- /dev/null +++ b/dom/security/test/csp/file_image_nonce.html @@ -0,0 +1,39 @@ + + + + + Bug 1355801: Nonce should not apply to images + + + + + + + + + + diff --git a/dom/security/test/csp/file_image_nonce.html^headers^ b/dom/security/test/csp/file_image_nonce.html^headers^ new file mode 100644 index 000000000..0d63558c4 --- /dev/null +++ b/dom/security/test/csp/file_image_nonce.html^headers^ @@ -0,0 +1,2 @@ +Content-Security-Policy: img-src 'nonce-abc'; +Cache-Control: no-cache diff --git a/dom/security/test/csp/mochitest.ini b/dom/security/test/csp/mochitest.ini index 04401b063..d3cabc16d 100644 --- a/dom/security/test/csp/mochitest.ini +++ b/dom/security/test/csp/mochitest.ini @@ -211,6 +211,8 @@ support-files = file_ro_ignore_xfo.html file_ro_ignore_xfo.html^headers^ file_upgrade_insecure_navigation.sjs + file_image_nonce.html + file_image_nonce.html^headers^ [test_base-uri.html] [test_blob_data_schemes.html] @@ -305,3 +307,4 @@ support-files = file_sandbox_allow_scripts.html file_sandbox_allow_scripts.html^headers^ [test_ignore_xfo.html] +[test_image_nonce.html] diff --git a/dom/security/test/csp/test_image_nonce.html b/dom/security/test/csp/test_image_nonce.html new file mode 100644 index 000000000..ff6d636b6 --- /dev/null +++ b/dom/security/test/csp/test_image_nonce.html @@ -0,0 +1,60 @@ + + + + + Bug 1139297 - Implement CSP upgrade-insecure-requests directive + + + + + + + + + + -- cgit v1.2.3 From cdcfbde10dbcf0fab0630d5ee0146be45d7a6572 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Fri, 25 Aug 2017 09:50:55 +0200 Subject: CSP: Support IDNs in connect-src --- dom/security/nsCSPUtils.cpp | 4 +- dom/security/test/csp/file_punycode_host_src.js | 2 + dom/security/test/csp/file_punycode_host_src.sjs | 45 +++++++++++++ dom/security/test/csp/mochitest.ini | 3 + dom/security/test/csp/test_punycode_host_src.html | 81 +++++++++++++++++++++++ dom/security/test/gtest/TestCSPParser.cpp | 2 + 6 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 dom/security/test/csp/file_punycode_host_src.js create mode 100644 dom/security/test/csp/file_punycode_host_src.sjs create mode 100644 dom/security/test/csp/test_punycode_host_src.html diff --git a/dom/security/nsCSPUtils.cpp b/dom/security/nsCSPUtils.cpp index b074a980c..0ca8f520e 100644 --- a/dom/security/nsCSPUtils.cpp +++ b/dom/security/nsCSPUtils.cpp @@ -270,7 +270,7 @@ CSP_CreateHostSrcFromURI(nsIURI* aURI) { // Create the host first nsCString host; - aURI->GetHost(host); + aURI->GetAsciiHost(host); nsCSPHostSrc *hostsrc = new nsCSPHostSrc(NS_ConvertUTF8toUTF16(host)); // Add the scheme. @@ -643,7 +643,7 @@ nsCSPHostSrc::permits(nsIURI* aUri, const nsAString& aNonce, bool aWasRedirected // Before we can check if the host matches, we have to // extract the host part from aUri. nsAutoCString uriHost; - nsresult rv = aUri->GetHost(uriHost); + nsresult rv = aUri->GetAsciiHost(uriHost); NS_ENSURE_SUCCESS(rv, false); nsString decodedUriHost; diff --git a/dom/security/test/csp/file_punycode_host_src.js b/dom/security/test/csp/file_punycode_host_src.js new file mode 100644 index 000000000..3505faf70 --- /dev/null +++ b/dom/security/test/csp/file_punycode_host_src.js @@ -0,0 +1,2 @@ +const LOADED = true; +parent.postMessage({result: 'script-allowed'}, "*"); \ No newline at end of file diff --git a/dom/security/test/csp/file_punycode_host_src.sjs b/dom/security/test/csp/file_punycode_host_src.sjs new file mode 100644 index 000000000..3189cc063 --- /dev/null +++ b/dom/security/test/csp/file_punycode_host_src.sjs @@ -0,0 +1,45 @@ +// custom *.sjs for Bug 1224225 +// Punycode in CSP host sources + +const HTML_PART1 = + "" + + "" + + "Bug 1224225 - CSP source matching should work for punycoded domain names" + + "" + + "" + + "" + + "" + + ""; + +function handleRequest(request, response) +{ + // avoid confusing cache behaviors + response.setHeader("Cache-Control", "no-cache", false); + response.setHeader("Content-Type", "text/html", false); + + Components.utils.importGlobalProperties(["URLSearchParams"]); + const query = new URLSearchParams(request.queryString); + + + if (query.get("csp")) { + response.setHeader("Content-Security-Policy", query.get("csp"), false); + } + if (query.get("action") == "script-unicode-csp-punycode") { + response.write(HTML_PART1 + TESTCASE1 + HTML_PART2); + return + } + if (query.get("action") == "script-punycode-csp-punycode") { + response.write(HTML_PART1 + TESTCASE2 + HTML_PART2); + return + } + + + // we should never get here, but just in case + // return something unexpected + response.write("do'h"); +} diff --git a/dom/security/test/csp/mochitest.ini b/dom/security/test/csp/mochitest.ini index d3cabc16d..8d44e9b0b 100644 --- a/dom/security/test/csp/mochitest.ini +++ b/dom/security/test/csp/mochitest.ini @@ -213,6 +213,8 @@ support-files = file_upgrade_insecure_navigation.sjs file_image_nonce.html file_image_nonce.html^headers^ + file_punycode_host_src.sjs + file_punycode_host_src.js [test_base-uri.html] [test_blob_data_schemes.html] @@ -308,3 +310,4 @@ support-files = file_sandbox_allow_scripts.html^headers^ [test_ignore_xfo.html] [test_image_nonce.html] +[test_punycode_host_src.html] diff --git a/dom/security/test/csp/test_punycode_host_src.html b/dom/security/test/csp/test_punycode_host_src.html new file mode 100644 index 000000000..8d891725c --- /dev/null +++ b/dom/security/test/csp/test_punycode_host_src.html @@ -0,0 +1,81 @@ + + + + + Bug 1224225 - CSP source matching should work for punycoded domain names + + + + + + + + + + diff --git a/dom/security/test/gtest/TestCSPParser.cpp b/dom/security/test/gtest/TestCSPParser.cpp index fafa7b5d9..8d168d81c 100644 --- a/dom/security/test/gtest/TestCSPParser.cpp +++ b/dom/security/test/gtest/TestCSPParser.cpp @@ -204,6 +204,8 @@ TEST(CSPParser, Directives) { static const PolicyTest policies[] = { + { "connect-src xn--mnchen-3ya.de", + "connect-src http://xn--mnchen-3ya.de"}, { "default-src http://www.example.com", "default-src http://www.example.com" }, { "script-src http://www.example.com", -- cgit v1.2.3 From 896e23c20eba71bffa77cb0874b9b341e1b6c264 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Fri, 25 Aug 2017 10:38:52 +0200 Subject: CSP: connect-src 'self' should always include https: and wss: schemes --- dom/security/nsCSPParser.cpp | 2 +- dom/security/nsCSPUtils.cpp | 36 +++++++++---- dom/security/nsCSPUtils.h | 6 ++- dom/security/test/csp/file_websocket_explicit.html | 31 +++++++++++ dom/security/test/csp/file_websocket_self.html | 31 +++++++++++ dom/security/test/csp/file_websocket_self_wsh.py | 7 +++ dom/security/test/csp/mochitest.ini | 5 ++ dom/security/test/csp/test_websocket_self.html | 61 ++++++++++++++++++++++ 8 files changed, 168 insertions(+), 11 deletions(-) create mode 100644 dom/security/test/csp/file_websocket_explicit.html create mode 100644 dom/security/test/csp/file_websocket_self.html create mode 100644 dom/security/test/csp/file_websocket_self_wsh.py create mode 100644 dom/security/test/csp/test_websocket_self.html diff --git a/dom/security/nsCSPParser.cpp b/dom/security/nsCSPParser.cpp index f1b5d8ba7..86aa4e001 100644 --- a/dom/security/nsCSPParser.cpp +++ b/dom/security/nsCSPParser.cpp @@ -532,7 +532,7 @@ nsCSPParser::keywordSource() // Special case handling for 'self' which is not stored internally as a keyword, // but rather creates a nsCSPHostSrc using the selfURI if (CSP_IsKeyword(mCurToken, CSP_SELF)) { - return CSP_CreateHostSrcFromURI(mSelfURI); + return CSP_CreateHostSrcFromSelfURI(mSelfURI); } if (CSP_IsKeyword(mCurToken, CSP_STRICT_DYNAMIC)) { diff --git a/dom/security/nsCSPUtils.cpp b/dom/security/nsCSPUtils.cpp index 0ca8f520e..a5f683b01 100644 --- a/dom/security/nsCSPUtils.cpp +++ b/dom/security/nsCSPUtils.cpp @@ -266,20 +266,21 @@ CSP_ContentTypeToDirective(nsContentPolicyType aType) } nsCSPHostSrc* -CSP_CreateHostSrcFromURI(nsIURI* aURI) +CSP_CreateHostSrcFromSelfURI(nsIURI* aSelfURI) { // Create the host first nsCString host; - aURI->GetAsciiHost(host); + aSelfURI->GetAsciiHost(host); nsCSPHostSrc *hostsrc = new nsCSPHostSrc(NS_ConvertUTF8toUTF16(host)); + hostsrc->setGeneratedFromSelfKeyword(); // Add the scheme. nsCString scheme; - aURI->GetScheme(scheme); + aSelfURI->GetScheme(scheme); hostsrc->setScheme(NS_ConvertUTF8toUTF16(scheme)); int32_t port; - aURI->GetPort(&port); + aSelfURI->GetPort(&port); // Only add port if it's not default port. if (port > 0) { nsAutoString portStr; @@ -348,13 +349,17 @@ CSP_IsQuotelessKeyword(const nsAString& aKey) * @param aUpgradeInsecure * Whether the policy makes use of the directive * 'upgrade-insecure-requests'. + * @param aFromSelfURI + * Whether a scheme was generated from the keyword 'self' + * which then allows schemeless sources to match ws and wss. */ bool permitsScheme(const nsAString& aEnforcementScheme, nsIURI* aUri, bool aReportOnly, - bool aUpgradeInsecure) + bool aUpgradeInsecure, + bool aFromSelfURI) { nsAutoCString scheme; nsresult rv = aUri->GetScheme(scheme); @@ -373,8 +378,20 @@ permitsScheme(const nsAString& aEnforcementScheme, // allow scheme-less sources where the protected resource is http // and the load is https, see: // http://www.w3.org/TR/CSP2/#match-source-expression - if (aEnforcementScheme.EqualsASCII("http") && - scheme.EqualsASCII("https")) { + if (aEnforcementScheme.EqualsASCII("http")) { + if (scheme.EqualsASCII("https")) { + return true; + } + if ((scheme.EqualsASCII("ws") || scheme.EqualsASCII("wss")) && aFromSelfURI) { + return true; + } + } + if (aEnforcementScheme.EqualsASCII("https")) { + if (scheme.EqualsLiteral("wss") && aFromSelfURI) { + return true; + } + } + if (aEnforcementScheme.EqualsASCII("ws") && scheme.EqualsASCII("wss")) { return true; } @@ -483,7 +500,7 @@ nsCSPSchemeSrc::permits(nsIURI* aUri, const nsAString& aNonce, bool aWasRedirect if (mInvalidated) { return false; } - return permitsScheme(mScheme, aUri, aReportOnly, aUpgradeInsecure); + return permitsScheme(mScheme, aUri, aReportOnly, aUpgradeInsecure, false); } bool @@ -503,6 +520,7 @@ nsCSPSchemeSrc::toString(nsAString& outStr) const nsCSPHostSrc::nsCSPHostSrc(const nsAString& aHost) : mHost(aHost) + , mGeneratedFromSelfKeyword(false) , mWithinFrameAncstorsDir(false) { ToLowerCase(mHost); @@ -612,7 +630,7 @@ nsCSPHostSrc::permits(nsIURI* aUri, const nsAString& aNonce, bool aWasRedirected // http://www.w3.org/TR/CSP11/#match-source-expression // 4.3) scheme matching: Check if the scheme matches. - if (!permitsScheme(mScheme, aUri, aReportOnly, aUpgradeInsecure)) { + if (!permitsScheme(mScheme, aUri, aReportOnly, aUpgradeInsecure, mGeneratedFromSelfKeyword)) { return false; } diff --git a/dom/security/nsCSPUtils.h b/dom/security/nsCSPUtils.h index 468c734a2..cfbe83256 100644 --- a/dom/security/nsCSPUtils.h +++ b/dom/security/nsCSPUtils.h @@ -186,7 +186,7 @@ nsresult CSP_AppendCSPFromHeader(nsIContentSecurityPolicy* aCsp, class nsCSPHostSrc; -nsCSPHostSrc* CSP_CreateHostSrcFromURI(nsIURI* aURI); +nsCSPHostSrc* CSP_CreateHostSrcFromSelfURI(nsIURI* aSelfURI); bool CSP_IsValidDirective(const nsAString& aDir); bool CSP_IsDirective(const nsAString& aValue, CSPDirective aDir); bool CSP_IsKeyword(const nsAString& aValue, enum CSPKeyword aKey); @@ -256,6 +256,9 @@ class nsCSPHostSrc : public nsCSPBaseSrc { void setPort(const nsAString& aPort); void appendPath(const nsAString &aPath); + inline void setGeneratedFromSelfKeyword() const + { mGeneratedFromSelfKeyword = true;} + inline void setWithinFrameAncestorsDir(bool aValue) const { mWithinFrameAncstorsDir = aValue; } @@ -276,6 +279,7 @@ class nsCSPHostSrc : public nsCSPBaseSrc { nsString mHost; nsString mPort; nsString mPath; + mutable bool mGeneratedFromSelfKeyword; mutable bool mWithinFrameAncstorsDir; }; diff --git a/dom/security/test/csp/file_websocket_explicit.html b/dom/security/test/csp/file_websocket_explicit.html new file mode 100644 index 000000000..51462ab74 --- /dev/null +++ b/dom/security/test/csp/file_websocket_explicit.html @@ -0,0 +1,31 @@ + + + + + Bug 1345615: Allow websocket schemes when using 'self' in CSP + + + + + + diff --git a/dom/security/test/csp/file_websocket_self.html b/dom/security/test/csp/file_websocket_self.html new file mode 100644 index 000000000..3ff5f0558 --- /dev/null +++ b/dom/security/test/csp/file_websocket_self.html @@ -0,0 +1,31 @@ + + + + + Bug 1345615: Allow websocket schemes when using 'self' in CSP + + + + + + diff --git a/dom/security/test/csp/file_websocket_self_wsh.py b/dom/security/test/csp/file_websocket_self_wsh.py new file mode 100644 index 000000000..5fe508a91 --- /dev/null +++ b/dom/security/test/csp/file_websocket_self_wsh.py @@ -0,0 +1,7 @@ +from mod_pywebsocket import msgutil + +def web_socket_do_extra_handshake(request): + pass + +def web_socket_transfer_data(request): + pass diff --git a/dom/security/test/csp/mochitest.ini b/dom/security/test/csp/mochitest.ini index 8d44e9b0b..2102cbe70 100644 --- a/dom/security/test/csp/mochitest.ini +++ b/dom/security/test/csp/mochitest.ini @@ -215,6 +215,9 @@ support-files = file_image_nonce.html^headers^ file_punycode_host_src.sjs file_punycode_host_src.js + file_websocket_self.html + file_websocket_explicit.html + file_websocket_self_wsh.py [test_base-uri.html] [test_blob_data_schemes.html] @@ -311,3 +314,5 @@ support-files = [test_ignore_xfo.html] [test_image_nonce.html] [test_punycode_host_src.html] +[test_websocket_self.html] +skip-if = toolkit == 'android' diff --git a/dom/security/test/csp/test_websocket_self.html b/dom/security/test/csp/test_websocket_self.html new file mode 100644 index 000000000..a03c32704 --- /dev/null +++ b/dom/security/test/csp/test_websocket_self.html @@ -0,0 +1,61 @@ + + + + + Bug 1345615: Allow websocket schemes when using 'self' in CSP + + + + + + + + + + + -- cgit v1.2.3 From 6bb68deb2a1de8e2a54ffb6cfefa2d6eebb883ef Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sun, 27 Aug 2017 10:17:36 +0200 Subject: Propagate plugin permissions from top-level to XO iframes. --- dom/base/nsPluginArray.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/dom/base/nsPluginArray.cpp b/dom/base/nsPluginArray.cpp index b9c946ca3..5b9378ae0 100644 --- a/dom/base/nsPluginArray.cpp +++ b/dom/base/nsPluginArray.cpp @@ -372,9 +372,21 @@ nsPluginArray::EnsurePlugins() nsCString permString; nsresult rv = pluginHost->GetPermissionStringForTag(pluginTag, 0, permString); if (rv == NS_OK) { - nsIPrincipal* principal = mWindow->GetExtantDoc()->NodePrincipal(); - nsCOMPtr permMgr = services::GetPermissionManager(); - permMgr->TestPermissionFromPrincipal(principal, permString.get(), &permission); + nsCOMPtr currentDoc = mWindow->GetExtantDoc(); + + // The top-level content document gets the final say on whether or not + // a plugin is going to be hidden or not, regardless of the origin + // that a subframe is hosted at. This is to avoid spamming the user + // with the hidden plugin notification bar when third-party iframes + // attempt to access navigator.plugins after the user has already + // expressed that the top-level document has this permission. + nsCOMPtr topDoc = currentDoc->GetTopLevelContentDocument(); + + if (topDoc) { + nsIPrincipal* principal = topDoc->NodePrincipal(); + nsCOMPtr permMgr = services::GetPermissionManager(); + permMgr->TestPermissionFromPrincipal(principal, permString.get(), &permission); + } } } } -- cgit v1.2.3 From cff4a7d0b65b9cfe272d72f7a26e3dcc33ea0a03 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sun, 27 Aug 2017 10:19:04 +0200 Subject: Bug 1217907: [RTL] Wrong direction in about:preferences#general homepage --- browser/themes/shared/incontentprefs/preferences.inc.css | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/browser/themes/shared/incontentprefs/preferences.inc.css b/browser/themes/shared/incontentprefs/preferences.inc.css index 577baa6ed..0e62660de 100644 --- a/browser/themes/shared/incontentprefs/preferences.inc.css +++ b/browser/themes/shared/incontentprefs/preferences.inc.css @@ -173,6 +173,11 @@ treecol { margin-inline-start: 0; } +#browserHomePage:-moz-locale-dir(rtl) input { + unicode-bidi: plaintext; + direction: rtl; +} + /* Content pane */ #playDRMContentLink { /* Line up with the buttons in the other grid bits: */ -- cgit v1.2.3