Bug 1809151 - corporate web proxy no kerberos auth for iframe content by backout 1629307 r=necko-reviewers,valentin,jesup

backing out rev-id: 5f5c6f101a19

Differential Revision: https://phabricator.services.mozilla.com/D167691

dom/security/FramingChecker.cpp

@@ -141,6 +141,21 @@ static bool ShouldIgnoreFrameOptions(nsIChannel* aChannel,
     return false;
   }
 
+  // log warning to console that xfo is ignored because of CSP
+  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
+  uint64_t innerWindowID = loadInfo->GetInnerWindowID();
+  bool privateWindow = !!loadInfo->GetOriginAttributes().mPrivateBrowsingId;
+  AutoTArray<nsString, 2> params = {u"x-frame-options"_ns,
+                                    u"frame-ancestors"_ns};
+  CSP_LogLocalizedStr("IgnoringSrcBecauseOfDirective", params,
+                      u""_ns,  // no sourcefile
+                      u""_ns,  // no scriptsample
+                      0,       // no linenumber
+                      0,       // no columnnumber
+                      nsIScriptError::warningFlag,
+                      "IgnoringSrcBecauseOfDirective"_ns, innerWindowID,
+                      privateWindow);
+
   return true;
 }
 
@@ -150,8 +165,7 @@ static bool ShouldIgnoreFrameOptions(nsIChannel* aChannel,
 // multiple headers, etc).
 /* static */
 bool FramingChecker::CheckFrameOptions(nsIChannel* aChannel,
-                                       nsIContentSecurityPolicy* aCsp,
+                                       nsIContentSecurityPolicy* aCsp) {
-                                       bool& outIsFrameCheckingSkipped) {
   if (!aChannel) {
     return true;
   }
@@ -204,7 +218,6 @@ bool FramingChecker::CheckFrameOptions(nsIChannel* aChannel,
   // xfo checks are ignored in case CSP frame-ancestors is present,
   // if so, there is nothing to do here.
   if (ShouldIgnoreFrameOptions(aChannel, aCsp)) {
-    outIsFrameCheckingSkipped = true;
     return true;
   }
 

dom/security/FramingChecker.h

@@ -25,8 +25,7 @@ class FramingChecker {
   // Determine if X-Frame-Options allows content to be framed
   // as a subdocument
   static bool CheckFrameOptions(nsIChannel* aChannel,
-                                nsIContentSecurityPolicy* aCSP,
+                                nsIContentSecurityPolicy* aCSP);
-                                bool& outIsFrameCheckingSkipped);
 
  protected:
   enum XFOHeader { eDENY, eSAMEORIGIN };

dom/security/nsContentSecurityUtils.cpp

@@ -952,8 +952,8 @@ nsresult nsContentSecurityUtils::GetHttpChannelFromPotentialMultiPart(
   return NS_OK;
 }
 
-nsresult CheckCSPFrameAncestorPolicy(nsIChannel* aChannel,
+nsresult ParseCSPAndEnforceFrameAncestorCheck(
-                                     nsIContentSecurityPolicy** aOutCSP) {
+    nsIChannel* aChannel, nsIContentSecurityPolicy** aOutCSP) {
   MOZ_ASSERT(aChannel);
 
   nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
@@ -1059,6 +1059,7 @@ nsresult CheckCSPFrameAncestorPolicy(nsIChannel* aChannel,
 
   if (NS_FAILED(rv) || !safeAncestry) {
     // stop!  ERROR page!
+    aChannel->Cancel(NS_ERROR_CSP_FRAME_ANCESTOR_VIOLATION);
     return NS_ERROR_CSP_FRAME_ANCESTOR_VIOLATION;
   }
 
@@ -1068,51 +1069,22 @@ nsresult CheckCSPFrameAncestorPolicy(nsIChannel* aChannel,
   return NS_OK;
 }
 
-void EnforceCSPFrameAncestorPolicy(nsIChannel* aChannel,
-                                   const nsresult& aError) {
-  if (aError == NS_ERROR_CSP_FRAME_ANCESTOR_VIOLATION) {
-    aChannel->Cancel(NS_ERROR_CSP_FRAME_ANCESTOR_VIOLATION);
-  }
-}
-
 void EnforceXFrameOptionsCheck(nsIChannel* aChannel,
                                nsIContentSecurityPolicy* aCsp) {
   MOZ_ASSERT(aChannel);
-  bool isFrameOptionsIgnored = false;
+  if (!FramingChecker::CheckFrameOptions(aChannel, aCsp)) {
-  // check for XFO options
-  // XFO checks can be skipped if there are frame ancestors
-  if (!FramingChecker::CheckFrameOptions(aChannel, aCsp,
-                                         isFrameOptionsIgnored)) {
     // stop!  ERROR page!
     aChannel->Cancel(NS_ERROR_XFO_VIOLATION);
   }
-
-  if (isFrameOptionsIgnored) {
-    // log warning to console that xfo is ignored because of CSP
-    nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
-    uint64_t innerWindowID = loadInfo->GetInnerWindowID();
-    bool privateWindow = !!loadInfo->GetOriginAttributes().mPrivateBrowsingId;
-    AutoTArray<nsString, 2> params = {u"x-frame-options"_ns,
-                                      u"frame-ancestors"_ns};
-    CSP_LogLocalizedStr("IgnoringSrcBecauseOfDirective", params,
-                        u""_ns,  // no sourcefile
-                        u""_ns,  // no scriptsample
-                        0,       // no linenumber
-                        0,       // no columnnumber
-                        nsIScriptError::warningFlag,
-                        "IgnoringSrcBecauseOfDirective"_ns, innerWindowID,
-                        privateWindow);
-  }
 }
 
 /* static */
 void nsContentSecurityUtils::PerformCSPFrameAncestorAndXFOCheck(
     nsIChannel* aChannel) {
   nsCOMPtr<nsIContentSecurityPolicy> csp;
-  nsresult rv = CheckCSPFrameAncestorPolicy(aChannel, getter_AddRefs(csp));
+  nsresult rv =
-
+      ParseCSPAndEnforceFrameAncestorCheck(aChannel, getter_AddRefs(csp));
   if (NS_FAILED(rv)) {
-    EnforceCSPFrameAncestorPolicy(aChannel, rv);
     return;
   }
 
@@ -1121,21 +1093,6 @@ void nsContentSecurityUtils::PerformCSPFrameAncestorAndXFOCheck(
   // will be discarded
   EnforceXFrameOptionsCheck(aChannel, csp);
 }
-/* static */
-bool nsContentSecurityUtils::CheckCSPFrameAncestorAndXFO(nsIChannel* aChannel) {
-  nsCOMPtr<nsIContentSecurityPolicy> csp;
-  nsresult rv = CheckCSPFrameAncestorPolicy(aChannel, getter_AddRefs(csp));
-
-  if (NS_FAILED(rv)) {
-    EnforceCSPFrameAncestorPolicy(aChannel, rv);
-    return false;
-  }
-
-  bool isFrameOptionsIgnored = false;
-
-  return FramingChecker::CheckFrameOptions(aChannel, csp,
-                                           isFrameOptionsIgnored);
-}
 
 #if defined(DEBUG)
 /* static */

dom/security/nsContentSecurityUtils.h

@@ -61,11 +61,6 @@ class nsContentSecurityUtils {
   // If any of the two disallows framing, the channel will be cancelled.
   static void PerformCSPFrameAncestorAndXFOCheck(nsIChannel* aChannel);
 
-  // Helper function which just checks if the channel violates any:
-  // 1. CSP frame-ancestors properties
-  // 2. x-frame-options
-  static bool CheckCSPFrameAncestorAndXFO(nsIChannel* aChannel);
-
   // Helper function to Check if a Download is allowed;
   static long ClassifyDownload(nsIChannel* aChannel,
                                const nsAutoCString& aMimeTypeGuess);

netwerk/protocol/http/nsHttpChannel.cpp

@@ -17,7 +17,6 @@
 #include "mozilla/dom/nsCSPService.h"
 #include "mozilla/StoragePrincipalHelper.h"
 
-#include "nsContentSecurityUtils.h"
 #include "nsHttp.h"
 #include "nsHttpChannel.h"
 #include "nsHttpChannelAuthProvider.h"
@@ -2376,9 +2375,6 @@ nsresult nsHttpChannel::ContinueProcessResponse3(nsresult rv) {
         // It's up to the consumer to re-try w/o setting a custom
         // auth header if cached credentials should be attempted.
         rv = NS_ERROR_FAILURE;
-      } else if (!nsContentSecurityUtils::CheckCSPFrameAncestorAndXFO(this)) {
-        // CSP Frame Ancestor and X-Frame-Options check has failed
-        rv = NS_ERROR_FAILURE;
       } else {
         rv = mAuthProvider->ProcessAuthentication(
             httpStatus, mConnectionInfo->EndToEndSSL() && mTransaction &&

netwerk/test/browser/browser.ini

@@ -65,9 +65,6 @@ support-files =
   cookie_filtering_secure_resource_org.html^headers^
   cookie_filtering_square.png
   cookie_filtering_square.png^headers^
-  x_frame_options.html
-  x_frame_options.html^headers^
-  test_1629307.html
 
 [browser_about_cache.js]
 [browser_bug1535877.js]
@@ -125,7 +122,6 @@ support-files =
 [browser_103_user_load.js]
 support-files =
   early_hint_preload_test_helper.jsm
-[browser_bug1629307.js]
 [browser_103_referrer_policy.js]
 support-files =
   early_hint_referrer_policy_html.sjs

netwerk/test/browser/browser_bug1629307.js

@@ -1,43 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-"use strict";
-
-// Load a web page containing an iframe that requires authentication but includes the X-Frame-Options: SAMEORIGIN header.
-// Make sure that we don't needlessly show an authentication prompt for it.
-
-const { PromptTestUtils } = ChromeUtils.importESModule(
-  "resource://testing-common/PromptTestUtils.sys.mjs"
-);
-
-add_task(async function() {
-  let URL =
-    "https://example.com/browser/netwerk/test/browser/test_1629307.html";
-
-  let hasPrompt = false;
-
-  PromptTestUtils.handleNextPrompt(
-    window,
-    {
-      modalType: Services.prefs.getIntPref("prompts.modalType.httpAuth"),
-      promptType: "promptUserAndPass",
-    },
-    { buttonNumClick: 1 }
-  )
-    .then(function() {
-      hasPrompt = true;
-    })
-    .catch(function() {});
-
-  BrowserTestUtils.loadURIString(gBrowser.selectedBrowser, URL);
-
-  // wait until the page and its iframe page is loaded
-  await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, true, URL);
-
-  Assert.equal(
-    hasPrompt,
-    false,
-    "no prompt when loading page via iframe with x-auth options"
-  );
-});

netwerk/test/browser/test_1629307.html

@@ -1,9 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-</head>
-<body>
-  <iframe
-    src="https://example.org/browser/netwerk/test/browser/x_frame_options.html"></iframe>
-</body>
-</html>

netwerk/test/browser/x_frame_options.html^headers^

@@ -1,3 +0,0 @@
-HTTP 401 UNAUTHORIZED
-X-Frame-Options: SAMEORIGIN
-WWW-Authenticate: basic realm="login required"