diff --git a/devtools/server/actors/errordocs.js b/devtools/server/actors/errordocs.js index 912dcb864df9..0bcbae6f87b0 100644 --- a/devtools/server/actors/errordocs.js +++ b/devtools/server/actors/errordocs.js @@ -112,7 +112,10 @@ const SOURCE_MAP_LEARN_MORE = "https://developer.mozilla.org/en-US/docs/Tools/Debugger/Source_map_errors"; const TLS_LEARN_MORE = "https://blog.mozilla.org/security/2018/10/15/removing-old-versions-of-tls/"; +const X_FRAME_OPTIONS_LEARN_MORE = + "https://developer.mozilla.org/docs/Web/HTTP/Headers/X-Frame-Options"; const ErrorCategories = { + "X-Frame-Options": X_FRAME_OPTIONS_LEARN_MORE, "Insecure Password Field": INSECURE_PASSWORDS_LEARN_MORE, "Mixed Content Message": MIXED_CONTENT_LEARN_MORE, "Mixed Content Blocker": MIXED_CONTENT_LEARN_MORE, diff --git a/dom/base/test/test_x-frame-options.html b/dom/base/test/test_x-frame-options.html index 000672292fcc..852cc34e1ee6 100644 --- a/dom/base/test/test_x-frame-options.html +++ b/dom/base/test/test_x-frame-options.html @@ -75,10 +75,10 @@ var testFramesLoaded = function() { var test11 = frame.contentDocument.getElementById("test").textContent; is(test11, "allow-from-allow", "test allow-from-allow"); - // iframe from different origin, with allow-from: other - should not load + // iframe from different origin, with allow-from: other - should load as we no longer support allow-from (Bug 1301529) frame = harness.contentDocument.getElementById("allow-from-deny"); var test12 = frame.contentDocument.getElementById("test"); - is(test12, null, "test allow-from-deny"); + isnot(test12, null, "test allow-from-deny"); // iframe from different origin, X-F-O: SAMEORIGIN, multipart - should not load frame = harness.contentDocument.getElementById("sameorigin-multipart"); @@ -91,7 +91,7 @@ var testFramesLoaded = function() { is(test14, "sameorigin-multipart2", "test sameorigin-multipart2"); - // frames from bug 836132 tests + // frames from bug 836132 tests, no longer supported allow-from { frame = harness.contentDocument.getElementById("allow-from-allow-1"); var theTestResult = frame.contentDocument.getElementById("test"); @@ -100,10 +100,11 @@ var testFramesLoaded = function() { is(theTestResult.textContent, "allow-from-allow-1", "test allow-from-allow-1"); } } + // Verify allow-from no longer works for (var i = 1; i<=14; i++) { frame = harness.contentDocument.getElementById("allow-from-deny-" + i); var theTestResult = frame.contentDocument.getElementById("test"); - is(theTestResult, null, "test allow-from-deny-" + i); + isnot(theTestResult, null, "test allow-from-deny-" + i); } // call tests to check principal comparison, e.g. a document can open a window diff --git a/dom/browser-element/mochitest/browserElement_XFrameOptionsAllowFrom.js b/dom/browser-element/mochitest/browserElement_XFrameOptionsAllowFrom.js deleted file mode 100644 index d7fb6373dc1c..000000000000 --- a/dom/browser-element/mochitest/browserElement_XFrameOptionsAllowFrom.js +++ /dev/null @@ -1,64 +0,0 @@ -/* Any copyright is dedicated to the public domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -// Bug 690168 - Support Allow-From notation for X-Frame-Options header. -"use strict"; - -/* global browserElementTestHelpers */ - -SimpleTest.waitForExplicitFinish(); -browserElementTestHelpers.setEnabledPref(true); -browserElementTestHelpers.addPermission(); - -var initialScreenshotArrayBuffer = null; - -function arrayBuffersEqual(a, b) { - var x = new Int8Array(a); - var y = new Int8Array(b); - if (x.length != y.length) { - return false; - } - - for (var i = 0; i < x.length; i++) { - if (x[i] != y[i]) { - return false; - } - } - - return true; -} - -function runTest() { - var iframe = document.createElement("iframe"); - iframe.setAttribute("mozbrowser", "true"); - iframe.height = "1000px"; - - var step1, stepfinish; - // The innermost page we load will fire an alert when it successfully loads. - iframe.addEventListener("mozbrowsershowmodalprompt", function(e) { - switch (e.detail.message) { - case "step 1": - step1 = SpecialPowers.snapshotWindow(iframe.contentWindow); - break; - case "step 2": - ok(false, "cross origin page loaded"); - break; - case "finish": - // The page has now attempted to load the X-Frame-Options page; take - // another screenshot. - stepfinish = SpecialPowers.snapshotWindow(iframe.contentWindow); - ok( - step1.toDataURL() == stepfinish.toDataURL(), - "Screenshots should be identical" - ); - SimpleTest.finish(); - } - }); - - document.body.appendChild(iframe); - - iframe.src = - "http://example.com/tests/dom/browser-element/mochitest/file_browserElement_XFrameOptionsAllowFrom.html"; -} - -addEventListener("testready", runTest); diff --git a/dom/browser-element/mochitest/mochitest-oop.ini b/dom/browser-element/mochitest/mochitest-oop.ini index 8d4943f17bcc..4172fbc4ca04 100644 --- a/dom/browser-element/mochitest/mochitest-oop.ini +++ b/dom/browser-element/mochitest/mochitest-oop.ini @@ -59,9 +59,6 @@ disabled = Disabling some OOP tests for WebIDL scope changes (bug 1310706 for re [test_browserElement_oop_Titlechange.html] [test_browserElement_oop_TopBarrier.html] [test_browserElement_oop_XFrameOptions.html] -[test_browserElement_oop_XFrameOptionsAllowFrom.html] -#skip-if = asan # bug 1189592 - should be OK when ASAN mochitests are on Ubuntu 16.04 -disabled = Disabling some OOP tests for WebIDL scope changes (bug 1310706 for re-enabling) [test_browserElement_oop_XFrameOptionsDeny.html] disabled = Disabling some OOP tests for WebIDL scope changes (bug 1310706 for re-enabling) [test_browserElement_oop_XFrameOptionsSameOrigin.html] diff --git a/dom/browser-element/mochitest/mochitest.ini b/dom/browser-element/mochitest/mochitest.ini index 04e97e4e6614..f1ea56228b81 100644 --- a/dom/browser-element/mochitest/mochitest.ini +++ b/dom/browser-element/mochitest/mochitest.ini @@ -48,7 +48,6 @@ support-files = browserElement_Titlechange.js browserElement_TopBarrier.js browserElement_XFrameOptions.js - browserElement_XFrameOptionsAllowFrom.js browserElement_XFrameOptionsDeny.js browserElement_XFrameOptionsSameOrigin.js file_browserElement_AlertInFrame.html @@ -75,8 +74,6 @@ support-files = file_browserElement_TargetBlank.html file_browserElement_TargetTop.html file_browserElement_XFrameOptions.sjs - file_browserElement_XFrameOptionsAllowFrom.html - file_browserElement_XFrameOptionsAllowFrom.sjs file_browserElement_XFrameOptionsDeny.html file_browserElement_XFrameOptionsSameOrigin.html file_bug741717.sjs @@ -135,7 +132,6 @@ skip-if = android_version == '22' # bug 1358876, bug 1322607 [test_browserElement_inproc_Titlechange.html] [test_browserElement_inproc_TopBarrier.html] [test_browserElement_inproc_XFrameOptions.html] -[test_browserElement_inproc_XFrameOptionsAllowFrom.html] [test_browserElement_inproc_XFrameOptionsDeny.html] [test_browserElement_inproc_XFrameOptionsSameOrigin.html] [test_browserElement_inproc_Reload.html] diff --git a/dom/browser-element/mochitest/test_browserElement_inproc_XFrameOptionsAllowFrom.html b/dom/browser-element/mochitest/test_browserElement_inproc_XFrameOptionsAllowFrom.html deleted file mode 100644 index 151e3e9809ec..000000000000 --- a/dom/browser-element/mochitest/test_browserElement_inproc_XFrameOptionsAllowFrom.html +++ /dev/null @@ -1,13 +0,0 @@ - - - - Test for Bug 690168 - - - - - - - - diff --git a/dom/browser-element/mochitest/test_browserElement_oop_XFrameOptionsAllowFrom.html b/dom/browser-element/mochitest/test_browserElement_oop_XFrameOptionsAllowFrom.html deleted file mode 100644 index 151e3e9809ec..000000000000 --- a/dom/browser-element/mochitest/test_browserElement_oop_XFrameOptionsAllowFrom.html +++ /dev/null @@ -1,13 +0,0 @@ - - - - Test for Bug 690168 - - - - - - - - diff --git a/dom/locales/en-US/chrome/security/security.properties b/dom/locales/en-US/chrome/security/security.properties index b9fb03e01123..18b592f8b7e1 100644 --- a/dom/locales/en-US/chrome/security/security.properties +++ b/dom/locales/en-US/chrome/security/security.properties @@ -126,3 +126,11 @@ FeaturePolicyInvalidAllowValue=Feature Policy: Skipping unsupported allow value ReferrerLengthOverLimitation=HTTP Referrer header: Length is over “%1$S” bytes limit - stripping referrer header down to origin: “%2$S” # LOCALIZATION NOTE: "%1$S" is the limitation length (bytes) of referrer URI, "%2$S" is the origin of the referrer URI. ReferrerOriginLengthOverLimitation=HTTP Referrer header: Length of origin within referrer is over “%1$S” bytes limit - removing referrer with origin “%2$S”. + +# X-Frame-Options +# LOCALIZATION NOTE: %1$S is the header value, %2$S is frame URI and %3$S is the parent document URI. +XFOInvalid = Invalid X-Frame-Options: “%1$S” header from “%2$S” loaded into “%3$S”. +# LOCALIZATION NOTE: %1$S is the header value, %2$S is frame URI and %3$S is the parent document URI. +XFODeny = Load denied by X-Frame-Options: “%1$S” from “%2$S”, site does not permit any framing. Attempted to load into “%3$S”. +# LOCALIZATION NOTE: %1$S is the header value, %2$S is frame URI and %3$S is the parent document URI. +XFOSameOrigin = Load denied by X-Frame-Options: “%1$S” from “%2$S”, site does not permit cross-origin framing from “%3$S”. diff --git a/dom/security/FramingChecker.cpp b/dom/security/FramingChecker.cpp index bdcfdd1b5a69..d9d9bc6e0b98 100644 --- a/dom/security/FramingChecker.cpp +++ b/dom/security/FramingChecker.cpp @@ -17,27 +17,112 @@ #include "mozilla/dom/nsCSPUtils.h" #include "mozilla/dom/LoadURIOptionsBinding.h" #include "mozilla/NullPrincipal.h" +#include "nsIStringBundle.h" using namespace mozilla; +void FramingChecker::ReportError(const char* aMessageTag, + nsIDocShellTreeItem* aParentDocShellItem, + nsIURI* aChildURI, const nsAString& aPolicy) { + MOZ_ASSERT(aParentDocShellItem, "Need a parent docshell"); + if (!aChildURI || !aParentDocShellItem) { + return; + } + + Document* parentDocument = aParentDocShellItem->GetDocument(); + nsCOMPtr parentURI; + parentDocument->NodePrincipal()->GetURI(getter_AddRefs(parentURI)); + MOZ_ASSERT(!parentDocument->NodePrincipal()->IsSystemPrincipal(), + "Should not get system principal here."); + + // Get the parent URL spec + nsAutoCString parentSpec; + nsresult rv; + rv = parentURI->GetAsciiSpec(parentSpec); + if (NS_FAILED(rv)) { + return; + } + + // Get the child URL spec + nsAutoCString childSpec; + rv = aChildURI->GetAsciiSpec(childSpec); + if (NS_FAILED(rv)) { + return; + } + + nsCOMPtr bundleService = + mozilla::services::GetStringBundleService(); + nsCOMPtr bundle; + rv = bundleService->CreateBundle( + "chrome://global/locale/security/security.properties", + getter_AddRefs(bundle)); + if (NS_FAILED(rv)) { + return; + } + + if (NS_WARN_IF(!bundle)) { + return; + } + + nsCOMPtr console( + do_GetService(NS_CONSOLESERVICE_CONTRACTID)); + nsCOMPtr error(do_CreateInstance(NS_SCRIPTERROR_CONTRACTID)); + if (!console || !error) { + return; + } + + // Localize the error message + nsAutoString message; + AutoTArray formatStrings; + formatStrings.AppendElement(aPolicy); + CopyASCIItoUTF16(childSpec, *formatStrings.AppendElement()); + CopyASCIItoUTF16(parentSpec, *formatStrings.AppendElement()); + rv = bundle->FormatStringFromName(aMessageTag, formatStrings, message); + if (NS_FAILED(rv)) { + return; + } + + rv = error->InitWithWindowID(message, EmptyString(), EmptyString(), 0, 0, + nsIScriptError::errorFlag, "X-Frame-Options", + parentDocument->InnerWindowID()); + if (NS_FAILED(rv)) { + return; + } + console->LogMessage(error); +} + /* static */ bool FramingChecker::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, const nsAString& aPolicy, nsIDocShell* aDocShell) { - static const char allowFrom[] = "allow-from"; - const uint32_t allowFromLen = ArrayLength(allowFrom) - 1; - bool isAllowFrom = - StringHead(aPolicy, allowFromLen).LowerCaseEqualsLiteral(allowFrom); + nsresult rv; + // Find the top docshell in our parent chain that doesn't have the system + // principal and use it for the principal comparison. Finding the top + // content-type docshell doesn't work because some chrome documents are + // loaded in content docshells (see bug 593387). + nsCOMPtr thisDocShellItem(aDocShell); + nsCOMPtr parentDocShellItem; + nsCOMPtr curDocShellItem = thisDocShellItem; + nsCOMPtr topDoc; + nsCOMPtr ssm = + do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv); - // return early if header does not have one of the values with meaning - if (!aPolicy.LowerCaseEqualsLiteral("deny") && - !aPolicy.LowerCaseEqualsLiteral("sameorigin") && !isAllowFrom) { - return true; + if (!ssm) { + MOZ_CRASH(); } nsCOMPtr uri; aHttpChannel->GetURI(getter_AddRefs(uri)); + // return early if header does not have one of the values with meaning + if (!aPolicy.LowerCaseEqualsLiteral("deny") && + !aPolicy.LowerCaseEqualsLiteral("sameorigin")) { + nsCOMPtr root; + curDocShellItem->GetSameTypeRootTreeItem(getter_AddRefs(root)); + ReportError("XFOInvalid", root, uri, aPolicy); + return true; + } + // XXXkhuey when does this happen? Is returning true safe here? if (!aDocShell) { return true; @@ -62,21 +147,6 @@ bool FramingChecker::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, return true; } - // Find the top docshell in our parent chain that doesn't have the system - // principal and use it for the principal comparison. Finding the top - // content-type docshell doesn't work because some chrome documents are - // loaded in content docshells (see bug 593387). - nsCOMPtr thisDocShellItem(aDocShell); - nsCOMPtr parentDocShellItem; - nsCOMPtr curDocShellItem = thisDocShellItem; - nsCOMPtr topDoc; - nsresult rv; - nsCOMPtr ssm = - do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv); - if (!ssm) { - MOZ_CRASH(); - } - // If the X-Frame-Options value is SAMEORIGIN, then the top frame in the // parent chain must be from the same origin as this document. bool checkSameOrigin = aPolicy.LowerCaseEqualsLiteral("sameorigin"); @@ -109,7 +179,7 @@ bool FramingChecker::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, // one of the ancestors is not same origin as this document if (NS_FAILED(rv)) { - ReportXFOViolation(curDocShellItem, uri, eSAMEORIGIN); + ReportError("XFOSameOrigin", curDocShellItem, uri, aPolicy); return false; } } @@ -129,34 +199,10 @@ bool FramingChecker::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, // not met (current docshell is not the top docshell), prohibit the // load. if (aPolicy.LowerCaseEqualsLiteral("deny")) { - ReportXFOViolation(curDocShellItem, uri, eDENY); + ReportError("XFODeny", curDocShellItem, uri, aPolicy); return false; } - topDoc = curDocShellItem->GetDocument(); - topDoc->NodePrincipal()->GetURI(getter_AddRefs(topUri)); - - // If the X-Frame-Options value is "allow-from [uri]", then the top - // frame in the parent chain must be from that origin - if (isAllowFrom) { - if (aPolicy.Length() == allowFromLen || - (aPolicy[allowFromLen] != ' ' && aPolicy[allowFromLen] != '\t')) { - ReportXFOViolation(curDocShellItem, uri, eALLOWFROM); - return false; - } - rv = NS_NewURI(getter_AddRefs(uri), Substring(aPolicy, allowFromLen)); - if (NS_FAILED(rv)) { - return false; - } - bool isPrivateWin = - topDoc->NodePrincipal()->OriginAttributesRef().mPrivateBrowsingId > 0; - rv = ssm->CheckSameOriginURI(uri, topUri, true, isPrivateWin); - if (NS_FAILED(rv)) { - ReportXFOViolation(curDocShellItem, uri, eALLOWFROM); - return false; - } - } - return true; } @@ -259,81 +305,3 @@ bool FramingChecker::CheckFrameOptions(nsIChannel* aChannel, return true; } - -/* static */ -void FramingChecker::ReportXFOViolation(nsIDocShellTreeItem* aTopDocShellItem, - nsIURI* aThisURI, XFOHeader aHeader) { - MOZ_ASSERT(aTopDocShellItem, "Need a top docshell"); - - nsCOMPtr topOuterWindow = aTopDocShellItem->GetWindow(); - if (!topOuterWindow) { - return; - } - - nsPIDOMWindowInner* topInnerWindow = topOuterWindow->GetCurrentInnerWindow(); - if (!topInnerWindow) { - return; - } - - nsCOMPtr topURI; - - nsCOMPtr document = aTopDocShellItem->GetDocument(); - nsresult rv = document->NodePrincipal()->GetURI(getter_AddRefs(topURI)); - if (NS_FAILED(rv)) { - return; - } - - if (!topURI) { - return; - } - - nsCString topURIString; - nsCString thisURIString; - - rv = topURI->GetSpec(topURIString); - if (NS_FAILED(rv)) { - return; - } - - rv = aThisURI->GetSpec(thisURIString); - if (NS_FAILED(rv)) { - return; - } - - nsCOMPtr consoleService = - do_GetService(NS_CONSOLESERVICE_CONTRACTID); - nsCOMPtr errorObject = - do_CreateInstance(NS_SCRIPTERROR_CONTRACTID); - - if (!consoleService || !errorObject) { - return; - } - - nsString msg = NS_LITERAL_STRING("Load denied by X-Frame-Options: "); - msg.Append(NS_ConvertUTF8toUTF16(thisURIString)); - - switch (aHeader) { - case eDENY: - msg.AppendLiteral(" does not permit framing."); - break; - case eSAMEORIGIN: - msg.AppendLiteral(" does not permit cross-origin framing."); - break; - case eALLOWFROM: - msg.AppendLiteral(" does not permit framing by "); - msg.Append(NS_ConvertUTF8toUTF16(topURIString)); - msg.Append('.'); - break; - } - - // It is ok to use InitWithSanitizedSource, because the source string is - // empty. - rv = errorObject->InitWithSanitizedSource( - msg, EmptyString(), EmptyString(), 0, 0, nsIScriptError::errorFlag, - "X-Frame-Options", topInnerWindow->WindowID()); - if (NS_FAILED(rv)) { - return; - } - - consoleService->LogMessage(errorObject); -} diff --git a/dom/security/FramingChecker.h b/dom/security/FramingChecker.h index 2d557cb4949b..572f3ebd0374 100644 --- a/dom/security/FramingChecker.h +++ b/dom/security/FramingChecker.h @@ -22,14 +22,24 @@ class FramingChecker { nsIContentSecurityPolicy* aCSP); protected: - enum XFOHeader { eDENY, eSAMEORIGIN, eALLOWFROM }; + enum XFOHeader { eDENY, eSAMEORIGIN }; + + /** + * Logs to the window about a X-Frame-Options error. + * + * @param aMessageTag the error message identifier to log + * @param aParentDocShellItem the containing docshell that the frame is + * loading into + * @param aChildURI the URI of the frame attempting to load + * @param aPolicy the header value string from the frame + */ + static void ReportError(const char* aMessageTag, + nsIDocShellTreeItem* aParentDocShellItem, + nsIURI* aChildURI, const nsAString& aPolicy); static bool CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, const nsAString& aPolicy, nsIDocShell* aDocShell); - - static void ReportXFOViolation(nsIDocShellTreeItem* aTopDocShellItem, - nsIURI* aThisURI, XFOHeader aHeader); }; #endif /* mozilla_dom_FramingChecker_h */