Add CanCommitOriginAndUrl() exception for 'no access' URLs.

Temporary fix that creates an exception for 'no access' URLs like
chrome-native://history. These URLs can cause a origin mismatch in the
commit code because of slight differences between how url::Origin and
blink::SecurityOrigin handle 'no access' URLs. This is intended as a
low risk change that can be easily merged to the M79 branch.

Bug: 1016711
Change-Id: I20734fe68b6919c06849a1ce1439624d6332898c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896863
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712415}

content/browser/child_process_security_policy_impl.cc

@@ -1306,6 +1306,19 @@ CanCommitStatus ChildProcessSecurityPolicyImpl::CanCommitOriginAndUrl(
       return CanCommitStatus::CAN_COMMIT_ORIGIN_AND_URL;
     }
 
+    // Allow "no access" schemes to commit even though |url_origin| and
+    // |origin| tuples don't match. We have to allow this because Blink's
+    // SecurityOrigin::CreateWithReferenceOrigin() and url::Origin::Resolve()
+    // handle "no access" URLs differently. CreateWithReferenceOrigin() treats
+    // "no access" like data: URLs and returns an opaque origin with |origin|
+    // as a precursor. Resolve() returns a non-opaque origin consisting of the
+    // scheme and host portions of the original URL.
+    //
+    // TODO(1020201): Make CreateWithReferenceOrigin() & Resolve() consistent
+    // with each other and then remove this exception.
+    if (base::Contains(url::GetNoAccessSchemes(), url.scheme()))
+      return CanCommitStatus::CAN_COMMIT_ORIGIN_AND_URL;
+
     return CanCommitStatus::CANNOT_COMMIT_ORIGIN;
   }
 

content/browser/navigation_browsertest.cc

@@ -17,6 +17,7 @@
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 #include "components/network_session_configurator/common/network_switches.h"
+#include "content/browser/browser_url_handler_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/browser/web_contents/web_contents_impl.h"
@@ -55,6 +56,7 @@
 #include "content/shell/browser/shell_download_manager_delegate.h"
 #include "content/test/content_browser_test_utils_internal.h"
 #include "content/test/did_commit_navigation_interceptor.h"
+#include "content/test/fake_network_url_loader_factory.h"
 #include "ipc/ipc_security_test_util.h"
 #include "mojo/public/cpp/bindings/pending_associated_remote.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
@@ -3069,4 +3071,107 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedDataUrl) {
   EXPECT_EQ(0u, response_2.http_request()->headers.count("Cookie"));
 }
 
+// Tests for validating URL rewriting behavior like chrome://history to
+// chrome-native://history.
+class NavigationUrlRewriteBrowserTest : public NavigationBaseBrowserTest {
+ protected:
+  static constexpr const char* kRewriteURL = "http://foo.com/rewrite";
+  static constexpr const char* kNoAccessScheme = "no-access";
+  static constexpr const char* kNoAccessURL = "no-access://testing/";
+
+  class BrowserClient : public ContentBrowserClient {
+   public:
+    void BrowserURLHandlerCreated(BrowserURLHandler* handler) override {
+      handler->AddHandlerPair(RewriteUrl,
+                              BrowserURLHandlerImpl::null_handler());
+    }
+
+    void RegisterNonNetworkNavigationURLLoaderFactories(
+        int frame_tree_node_id,
+        NonNetworkURLLoaderFactoryMap* factories) override {
+      auto url_loader_factory = std::make_unique<FakeNetworkURLLoaderFactory>(
+          "HTTP/1.1 200 OK\nContent-Type: text/html\n\n", "This is a test",
+          /* network_accessed */ true, net::OK);
+      factories->emplace(std::string(kNoAccessScheme),
+                         std::move(url_loader_factory));
+    }
+
+    bool ShouldAssignSiteForURL(const GURL& url) override {
+      return !url.SchemeIs(kNoAccessScheme);
+    }
+
+    static bool RewriteUrl(GURL* url, BrowserContext* browser_context) {
+      if (*url == GURL(kRewriteURL)) {
+        *url = GURL(kNoAccessURL);
+        return true;
+      }
+      return false;
+    }
+  };
+
+  void SetUp() override {
+    url::AddStandardScheme(kNoAccessScheme, url::SCHEME_WITH_HOST);
+    url::AddNoAccessScheme(kNoAccessScheme);
+
+    NavigationBaseBrowserTest::SetUp();
+  }
+
+  void SetUpOnMainThread() override {
+    NavigationBaseBrowserTest::SetUpOnMainThread();
+    ASSERT_TRUE(embedded_test_server()->Start());
+
+    browser_client_ = std::make_unique<BrowserClient>();
+    old_browser_client_ = SetBrowserClientForTesting(browser_client_.get());
+  }
+
+  void TearDownOnMainThread() override {
+    NavigationBaseBrowserTest::TearDownOnMainThread();
+    SetBrowserClientForTesting(old_browser_client_);
+  }
+
+  GURL GetRewriteToNoAccessURL() const { return GURL(kRewriteURL); }
+
+ private:
+  std::unique_ptr<BrowserClient> browser_client_;
+  ContentBrowserClient* old_browser_client_;
+};
+
+INSTANTIATE_TEST_SUITE_P(/* no prefix */,
+                         NavigationUrlRewriteBrowserTest,
+                         ::testing::Bool());
+
+// Tests navigating to a URL that gets rewritten to a "no access" URL. This
+// mimics the behavior of navigating to special URLs like chrome://newtab and
+// chrome://history which get rewritten to "no access" chrome-native:// URLs.
+IN_PROC_BROWSER_TEST_P(NavigationUrlRewriteBrowserTest, RewriteToNoAccess) {
+  // Perform an initial navigation.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL url = embedded_test_server()->GetURL("foo.com", "/title1.html");
+    EXPECT_TRUE(NavigateToURL(shell(), url));
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+    EXPECT_FALSE(observer.last_initiator_origin().has_value());
+  }
+
+  // Navigate to the URL that will get rewritten to a "no access" URL.
+  {
+    auto* web_contents = shell()->web_contents();
+    TestNavigationObserver observer(web_contents);
+
+    // Note: We are using LoadURLParams here because we need to have the
+    // initiator_origin set and NavigateToURL() does not do that.
+    NavigationController::LoadURLParams params(GetRewriteToNoAccessURL());
+    params.initiator_origin =
+        web_contents->GetMainFrame()->GetLastCommittedOrigin();
+    web_contents->GetController().LoadURLWithParams(params);
+    web_contents->Focus();
+    observer.Wait();
+
+    EXPECT_EQ(GURL(kNoAccessURL), observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+    EXPECT_TRUE(observer.last_initiator_origin().has_value());
+  }
+}
+
 }  // namespace content