Bug 1024557 - Ignore x-frame-options if CSP with frame-ancestors exists. r=smaug

docshell/base/nsDSURIContentListener.cpp

@@ -23,6 +23,7 @@
 #include "nsDocShellLoadTypes.h"
 #include "nsIMultiPartChannel.h"
 #include "nsContentUtils.h"
+#include "mozilla/dom/nsCSPUtils.h"
 
 using namespace mozilla;
 
@@ -85,14 +86,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...
@@ -266,9 +259,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;
@@ -286,7 +280,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;
   }
 
@@ -294,7 +288,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;
@@ -314,7 +308,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;
@@ -403,22 +397,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;
     }
@@ -443,11 +481,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) {
           nsCOMPtr<nsILoadInfo> loadInfo = httpChannel->GetLoadInfo();
           nsCOMPtr<nsIPrincipal> triggeringPrincipal = loadInfo
@@ -465,7 +503,7 @@ nsDSURIContentListener::CheckFrameOptions(nsIRequest* aRequest)
   return true;
 }
 
-void
+/* static */ void
 nsDSURIContentListener::ReportXFOViolation(nsIDocShellTreeItem* aTopDocShellItem,
                                            nsIURI* aThisURI,
                                            XFOHeader aHeader)

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;

dom/base/nsDocument.cpp

@@ -65,6 +65,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
@@ -2584,6 +2585,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;
 }
 

dom/base/nsDocument.h

@@ -1564,7 +1564,6 @@ private:
   void PostUnblockOnloadEvent();
   void DoUnblockOnload();
 
-  nsresult CheckFrameOptions();
   nsresult InitCSP(nsIChannel* aChannel);
 
   /**

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...

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):

dom/security/nsCSPContext.cpp

@@ -349,6 +349,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)
 {