From 3e9801630f7a3697936eb90d31185542abd8f60c Mon Sep 17 00:00:00 2001
From: Carlos IL <carlosil@chromium.org>
Date: Wed, 07 May 2025 18:32:19 -0700
Subject: [PATCH] Mixed Content: Use the same check for ShouldAutoUpgrade and IsMixedContent

Currently IsMixedContent determines whether mixed content is restricted
in a particular context by checking if its security origin or precursor
if opaque is https (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/loader/mixed_content_checker.cc;drc=8d201f296ea4efda4529e69fd9509be8abd63156;l=293), whereas ShouldAutoupgrade only checks the
security origin, which is null if opaque (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/loader/mixed_content_checker.cc;drc=8d201f296ea4efda4529e69fd9509be8abd63156;l=867).

This can lead to some requests not being autoupgraded when they should be.

Bug: 40062462
Change-Id: I10c381e407a0693ae262533027fad9e2c37fa365
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6388538
Auto-Submit: Carlos IL <carlosil@chromium.org>
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1457364}
---

diff --git a/third_party/blink/renderer/core/frame/remote_frame.cc b/third_party/blink/renderer/core/frame/remote_frame.cc
index b5b166c..96b0b13d 100644
--- a/third_party/blink/renderer/core/frame/remote_frame.cc
+++ b/third_party/blink/renderer/core/frame/remote_frame.cc
@@ -212,7 +212,8 @@
       frame_request.GetResourceRequest(), fetch_client_settings_object, window,
       frame_request.GetFrameType(),
       window->GetFrame() ? window->GetFrame()->GetContentSettingsClient()
-                         : nullptr);
+                         : nullptr,
+      window->GetFrame());
 
   if (NavigationShouldReplaceCurrentHistoryEntry(frame_load_type))
     frame_load_type = WebFrameLoadType::kReplaceCurrentItem;
diff --git a/third_party/blink/renderer/core/loader/frame_loader.cc b/third_party/blink/renderer/core/loader/frame_loader.cc
index bb0848d..eb4422f9 100644
--- a/third_party/blink/renderer/core/loader/frame_loader.cc
+++ b/third_party/blink/renderer/core/loader/frame_loader.cc
@@ -1797,7 +1797,7 @@
 
   MixedContentChecker::UpgradeInsecureRequest(
       resource_request, fetch_client_settings_object, window_for_logging,
-      frame_type, frame_->GetContentSettingsClient());
+      frame_type, frame_->GetContentSettingsClient(), frame_);
 }
 
 void FrameLoader::WriteIntoTrace(perfetto::TracedValue context) const {
diff --git a/third_party/blink/renderer/core/loader/mixed_content_checker.cc b/third_party/blink/renderer/core/loader/mixed_content_checker.cc
index c3a04f6..92fbcfcef6 100644
--- a/third_party/blink/renderer/core/loader/mixed_content_checker.cc
+++ b/third_party/blink/renderer/core/loader/mixed_content_checker.cc
@@ -318,6 +318,31 @@
 }
 
 // static
+bool MixedContentChecker::IsMixedContentRestrictedInFrameContext(
+    LocalFrame* frame) {
+  if (!frame) {
+    return false;
+  }
+  // Check the top frame first.
+  Frame& top = frame->Tree().Top();
+  if (SchemeRegistry::ShouldTreatURLSchemeAsRestrictingMixedContent(
+          top.GetSecurityContext()
+              ->GetSecurityOrigin()
+              ->GetOriginOrPrecursorOriginIfOpaque()
+              ->Protocol())) {
+    return true;
+  }
+  if (SchemeRegistry::ShouldTreatURLSchemeAsRestrictingMixedContent(
+          frame->GetSecurityContext()
+              ->GetSecurityOrigin()
+              ->GetOriginOrPrecursorOriginIfOpaque()
+              ->Protocol())) {
+    return true;
+  }
+  return false;
+}
+
+// static
 Frame* MixedContentChecker::InWhichFrameIsContentMixed(LocalFrame* frame,
                                                        const KURL& url) {
   // Frameless requests cannot be mixed content.
@@ -858,14 +883,22 @@
     mojom::blink::RequestContextType type,
     WebContentSettingsClient* settings_client,
     const ResourceRequest& resource_request,
-    ExecutionContext* execution_context_for_logging) {
-  const HttpsState https_state = fetch_client_settings_object->GetHttpsState();
+    ExecutionContext* execution_context_for_logging,
+    LocalFrame* frame) {
   const KURL& request_url = resource_request.Url();
   // We are currently not autoupgrading plugin loaded content, which is why
   // check_mode_for_plugin is hardcoded to kStrict.
+  bool settings_restricts_mixed_content;
+  if (frame) {
+    settings_restricts_mixed_content =
+        IsMixedContentRestrictedInFrameContext(frame);
+  } else {
+    settings_restricts_mixed_content =
+        fetch_client_settings_object->GetHttpsState() == HttpsState::kModern;
+  }
   if (!base::FeatureList::IsEnabled(
           blink::features::kMixedContentAutoupgrade) ||
-      https_state == HttpsState::kNone ||
+      !settings_restricts_mixed_content ||
       MixedContent::ContextTypeFromRequestContext(
           type, MixedContent::CheckModeForPlugin::kStrict) !=
           mojom::blink::MixedContentContextType::kOptionallyBlockable) {
@@ -1000,7 +1033,8 @@
     const FetchClientSettingsObject* fetch_client_settings_object,
     ExecutionContext* execution_context_for_logging,
     mojom::RequestContextFrameType frame_type,
-    WebContentSettingsClient* settings_client) {
+    WebContentSettingsClient* settings_client,
+    LocalFrame* frame) {
   // We always upgrade requests that meet any of the following criteria:
   //  1. Are for subresources.
   //  2. Are for nested frames.
@@ -1025,7 +1059,7 @@
     if (context == mojom::blink::RequestContextType::UNSPECIFIED ||
         !MixedContentChecker::ShouldAutoupgrade(
             fetch_client_settings_object, context, settings_client,
-            resource_request, execution_context_for_logging)) {
+            resource_request, execution_context_for_logging, frame)) {
       return;
     }
     // We set the upgrade if insecure flag regardless of whether we autoupgrade
diff --git a/third_party/blink/renderer/core/loader/mixed_content_checker.h b/third_party/blink/renderer/core/loader/mixed_content_checker.h
index 1c8149eb1..39e3c66 100644
--- a/third_party/blink/renderer/core/loader/mixed_content_checker.h
+++ b/third_party/blink/renderer/core/loader/mixed_content_checker.h
@@ -112,7 +112,8 @@
       mojom::blink::RequestContextType type,
       WebContentSettingsClient* settings_client,
       const ResourceRequest& resource_request,
-      ExecutionContext* execution_context_for_logging);
+      ExecutionContext* execution_context_for_logging,
+      LocalFrame* frame);
 
   static mojom::blink::MixedContentContextType ContextTypeForInspector(
       LocalFrame*,
@@ -152,7 +153,8 @@
       const FetchClientSettingsObject* fetch_client_settings_object,
       ExecutionContext* execution_context_for_logging,
       mojom::RequestContextFrameType,
-      WebContentSettingsClient* settings_client);
+      WebContentSettingsClient* settings_client,
+      LocalFrame* frame);
 
   static MixedContent::CheckModeForPlugin DecideCheckModeForPlugin(Settings*);
 
@@ -162,6 +164,8 @@
  private:
   FRIEND_TEST_ALL_PREFIXES(MixedContentCheckerTest, HandleCertificateError);
 
+  static bool IsMixedContentRestrictedInFrameContext(LocalFrame* frame);
+
   static Frame* InWhichFrameIsContentMixed(LocalFrame*, const KURL&);
 
   static ConsoleMessage* CreateConsoleMessageAboutFetch(
diff --git a/third_party/blink/renderer/core/loader/mixed_content_checker_test.cc b/third_party/blink/renderer/core/loader/mixed_content_checker_test.cc
index 56a4545e..c95b504 100644
--- a/third_party/blink/renderer/core/loader/mixed_content_checker_test.cc
+++ b/third_party/blink/renderer/core/loader/mixed_content_checker_test.cc
@@ -16,6 +16,8 @@
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink.h"
 #include "third_party/blink/public/mojom/loader/mixed_content.mojom-blink.h"
 #include "third_party/blink/public/mojom/loader/request_context_frame_type.mojom-blink.h"
+#include "third_party/blink/renderer/core/execution_context/security_context.h"
+#include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/frame/settings.h"
 #include "third_party/blink/renderer/core/loader/empty_clients.h"
@@ -349,7 +351,9 @@
   // These are not used in test, but need to be implemented since they are pure
   // virtual.
   const KURL& BaseUrl() const override { return url; }
-  const SecurityOrigin* GetSecurityOrigin() const override { return nullptr; }
+  const SecurityOrigin* GetSecurityOrigin() const override {
+    return origin_.get();
+  }
   network::mojom::ReferrerPolicy GetReferrerPolicy() const override {
     return network::mojom::ReferrerPolicy::kAlways;
   }
@@ -362,10 +366,19 @@
       const override {
     return set;
   }
+  void SetSecurityOrigin(String origin_url, String reference_origin) {
+    KURL origin_kurl(origin_url);
+    scoped_refptr<SecurityOrigin> reference =
+        SecurityOrigin::CreateFromString(reference_origin);
+    origin_ = SecurityOrigin::CreateWithReferenceOrigin(KURL(origin_url),
+                                                        reference.get());
+  }
+  scoped_refptr<SecurityOrigin> GetSecurityOrigin() { return origin_; }
 

diff --git a/third_party/blink/renderer/core/loader/mixed_content_checker_test.cc b/third_party/blink/renderer/core/loader/mixed_content_checker_test.cc
index 56a4545e..c95b504 100644
--- a/third_party/blink/renderer/core/loader/mixed_content_checker_test.cc
+++ b/third_party/blink/renderer/core/loader/mixed_content_checker_test.cc
@@ -16,6 +16,8 @@
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink.h"
 #include "third_party/blink/public/mojom/loader/mixed_content.mojom-blink.h"
 #include "third_party/blink/public/mojom/loader/request_context_frame_type.mojom-blink.h"
+#include "third_party/blink/renderer/core/execution_context/security_context.h"
+#include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/frame/settings.h"
 #include "third_party/blink/renderer/core/loader/empty_clients.h"
@@ -349,7 +351,9 @@
   // These are not used in test, but need to be implemented since they are pure
   // virtual.
   const KURL& BaseUrl() const override { return url; }
-  const SecurityOrigin* GetSecurityOrigin() const override { return nullptr; }
+  const SecurityOrigin* GetSecurityOrigin() const override {
+    return origin_.get();
+  }
   network::mojom::ReferrerPolicy GetReferrerPolicy() const override {
     return network::mojom::ReferrerPolicy::kAlways;
   }
@@ -362,10 +366,19 @@
       const override {
     return set;
   }
+  void SetSecurityOrigin(String origin_url, String reference_origin) {
+    KURL origin_kurl(origin_url);
+    scoped_refptr<SecurityOrigin> reference =
+        SecurityOrigin::CreateFromString(reference_origin);
+    origin_ = SecurityOrigin::CreateWithReferenceOrigin(KURL(origin_url),
+                                                        reference.get());
+  }
+  scoped_refptr<SecurityOrigin> GetSecurityOrigin() { return origin_; }
 
  private:
   const KURL url = KURL("https://example.test");
   const InsecureNavigationsSet set;
+  scoped_refptr<SecurityOrigin> origin_;
 };
 
 TEST(MixedContentCheckerTest,
@@ -376,12 +389,17 @@
   request.SetRequestContext(mojom::blink::RequestContextType::AUDIO);
   TestFetchClientSettingsObject* settings =
       MakeGarbageCollected<TestFetchClientSettingsObject>();
+  settings->SetSecurityOrigin("https://example.test", "");
   // Used to get a non-null document.
   DummyPageHolder holder;
+  holder.GetFrame()
+      .DomWindow()
+      ->GetSecurityContext()
+      .SetSecurityOriginForTesting(settings->GetSecurityOrigin());
 
   MixedContentChecker::UpgradeInsecureRequest(
       request, settings, holder.GetDocument().GetExecutionContext(),
-      mojom::RequestContextFrameType::kTopLevel, nullptr);
+      mojom::RequestContextFrameType::kTopLevel, nullptr, &holder.GetFrame());
 
   EXPECT_FALSE(request.IsAutomaticUpgrade());
   EXPECT_TRUE(request.UpgradeIfInsecure());
@@ -394,12 +412,18 @@
   request.SetRequestContext(mojom::blink::RequestContextType::AUDIO);
   TestFetchClientSettingsObject* settings =
       MakeGarbageCollected<TestFetchClientSettingsObject>();
+  settings->SetSecurityOrigin("https://example.test", "");
+
   // Used to get a non-null document.
   DummyPageHolder holder;
+  holder.GetFrame()
+      .DomWindow()
+      ->GetSecurityContext()
+      .SetSecurityOriginForTesting(settings->GetSecurityOrigin());
 
   MixedContentChecker::UpgradeInsecureRequest(
       request, settings, holder.GetDocument().GetExecutionContext(),
-      mojom::RequestContextFrameType::kTopLevel, nullptr);
+      mojom::RequestContextFrameType::kTopLevel, nullptr, &holder.GetFrame());
 
   EXPECT_TRUE(request.IsAutomaticUpgrade());
   EXPECT_TRUE(request.UpgradeIfInsecure());
@@ -413,12 +437,18 @@
   request.SetRequestContext(mojom::blink::RequestContextType::AUDIO);
   TestFetchClientSettingsObject* settings =
       MakeGarbageCollected<TestFetchClientSettingsObject>();
+  settings->SetSecurityOrigin("https://example.test", "");
+
   // Used to get a non-null document.
   DummyPageHolder holder;
+  holder.GetFrame()
+      .DomWindow()
+      ->GetSecurityContext()
+      .SetSecurityOriginForTesting(settings->GetSecurityOrigin());
 
   MixedContentChecker::UpgradeInsecureRequest(
       request, settings, holder.GetDocument().GetExecutionContext(),
-      mojom::RequestContextFrameType::kTopLevel, nullptr);
+      mojom::RequestContextFrameType::kTopLevel, nullptr, &holder.GetFrame());
 
   EXPECT_FALSE(request.IsAutomaticUpgrade());
   EXPECT_FALSE(request.UpgradeIfInsecure());
@@ -432,15 +462,76 @@
   request.SetRequestContext(mojom::blink::RequestContextType::AUDIO);
   TestFetchClientSettingsObject* settings =
       MakeGarbageCollected<TestFetchClientSettingsObject>();
+  settings->SetSecurityOrigin("https://example.test", "");
+
   // Used to get a non-null document.
   DummyPageHolder holder;
+  holder.GetFrame()
+      .DomWindow()
+      ->GetSecurityContext()
+      .SetSecurityOriginForTesting(settings->GetSecurityOrigin());
 
   MixedContentChecker::UpgradeInsecureRequest(
       request, settings, holder.GetDocument().GetExecutionContext(),
-      mojom::RequestContextFrameType::kTopLevel, nullptr);
+      mojom::RequestContextFrameType::kTopLevel, nullptr, &holder.GetFrame());
 
   EXPECT_FALSE(request.IsAutomaticUpgrade());
   EXPECT_FALSE(request.UpgradeIfInsecure());
 }
 
+TEST(MixedContentCheckerTest,
+     AutoupgradeMixedContentInOpaqueOriginIfPrecursorIsSecure) {
+  test::TaskEnvironment task_environment;
+  ResourceRequest request;
+  request.SetUrl(KURL("http://example.test"));
+  request.SetRequestContext(mojom::blink::RequestContextType::IMAGE);
+  TestFetchClientSettingsObject* settings =
+      MakeGarbageCollected<TestFetchClientSettingsObject>();
+  // Set the security origin to an opaque one, with a secure precursor.
+  settings->SetSecurityOrigin(
+      "data:text/html,<img src=http://example.test/insecureimage.jpg>",
+      "https://example.test");
+
+  // Used to get a non-null document.
+  DummyPageHolder holder;
+  holder.GetFrame()
+      .DomWindow()
+      ->GetSecurityContext()
+      .SetSecurityOriginForTesting(settings->GetSecurityOrigin());
+
+  MixedContentChecker::UpgradeInsecureRequest(
+      request, settings, holder.GetDocument().GetExecutionContext(),
+      mojom::RequestContextFrameType::kTopLevel, nullptr, &holder.GetFrame());
+
+  EXPECT_TRUE(request.IsAutomaticUpgrade());
+  EXPECT_TRUE(request.UpgradeIfInsecure());
+}
+
+TEST(MixedContentCheckerTest,
+     DontAutoupgradeMixedContentInOpaqueOriginIfPrecursorIsNotSecure) {
+  test::TaskEnvironment task_environment;
+  ResourceRequest request;
+  request.SetUrl(KURL("http://example.test"));
+  request.SetRequestContext(mojom::blink::RequestContextType::IMAGE);
+  TestFetchClientSettingsObject* settings =
+      MakeGarbageCollected<TestFetchClientSettingsObject>();
+  // Set the security origin to an opaque one, with a not secure precursor.
+  settings->SetSecurityOrigin(
+      "data:text/html,<img src=http://example.test/insecureimage.jpg>",
+      "http://example.test");
+
+  // Used to get a non-null document.
+  DummyPageHolder holder;
+  holder.GetFrame()
+      .DomWindow()
+      ->GetSecurityContext()
+      .SetSecurityOriginForTesting(settings->GetSecurityOrigin());
+
+  MixedContentChecker::UpgradeInsecureRequest(
+      request, settings, holder.GetDocument().GetExecutionContext(),
+      mojom::RequestContextFrameType::kTopLevel, nullptr, &holder.GetFrame());
+
+  EXPECT_FALSE(request.IsAutomaticUpgrade());
+}
+
 }  // namespace blink