Skip to content

Commit

Permalink
Make a fenced main frame's certificate error handled like a subframe.
Browse files Browse the repository at this point in the history
SSLManager handles certificate errors for the primary main frame
request, we do not need to handle it in
MixedContentNavigationThrottle::MaybeHandleCertificateError.

At the same time, to propagate certificate errors in fenced frames to
the primary main frame, we need to call outer frame's
OnDidRunContentWithCertificateErrors.  Otherwise, the errors cannot
be seen in the primary main frame's SSL content_status because fenced
frames use isolated NavigationController.

Bug: 1241387
Change-Id: Ic8e2df2c4f56172dda87eab60b49e286ea439298
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3508985
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#987311}
  • Loading branch information
yoshisatoyanagisawa authored and Chromium LUCI CQ committed Mar 31, 2022
1 parent dd0962d commit 876d584
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 2 deletions.
140 changes: 140 additions & 0 deletions content/browser/renderer_host/frame_tree_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "base/synchronization/lock.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "components/network_session_configurator/common/network_switches.h"
#include "content/browser/fenced_frame/fenced_frame.h"
#include "content/browser/fenced_frame/fenced_frame_url_mapping.h"
#include "content/browser/renderer_host/frame_tree.h"
Expand All @@ -27,6 +28,7 @@
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "content/public/test/fenced_frame_test_util.h"
#include "content/public/test/test_browser_context.h"
#include "content/public/test/test_frame_navigation_observer.h"
#include "content/public/test/test_navigation_observer.h"
#include "content/public/test/test_utils.h"
Expand All @@ -35,6 +37,7 @@
#include "content/test/content_browser_test_utils_internal.h"
#include "content/test/fenced_frame_test_utils.h"
#include "content/test/resource_load_observer.h"
#include "content/test/test_content_browser_client.h"
#include "net/base/features.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
Expand Down Expand Up @@ -1819,6 +1822,135 @@ IN_PROC_BROWSER_TEST_P(FencedFrameTreeBrowserTest, CheckSecFetchDestHeader) {
EXPECT_TRUE(CheckAndClearSecFetchDestHeader(iframe_url, "fencedframe"));
}

namespace {
class ScopedInsecureContentTestContentBrowserClient
: public TestContentBrowserClient {
public:
ScopedInsecureContentTestContentBrowserClient()
: old_client(SetBrowserClientForTesting(this)) {}
~ScopedInsecureContentTestContentBrowserClient() override {
SetBrowserClientForTesting(old_client);
}

void OverrideWebkitPrefs(WebContents* web_contents,
blink::web_pref::WebPreferences* prefs) override {
// Browser will both run and display insecure content.
prefs->allow_running_insecure_content = true;
}

private:
raw_ptr<ContentBrowserClient> old_client;
};
} // namespace

class FencedFrameIgnoreCertErrors : public FencedFrameTreeBrowserTest {
public:
FencedFrameIgnoreCertErrors()
: https_server_mismatched_(net::EmbeddedTestServer::TYPE_HTTPS) {}

protected:
void SetUpOnMainThread() override {
https_server_mismatched_.ServeFilesFromSourceDirectory(
GetTestDataFilePath());
https_server_mismatched_.SetSSLConfig(
net::EmbeddedTestServer::CERT_MISMATCHED_NAME);
ASSERT_TRUE(https_server_mismatched_.Start());

// We need to have a dedicated browser context for the tests.
// Or, SSLManager::UpdateEntry() doesn't update the entry if
// |ssl_host_state_delegate_| is nullptr.
browser_context_ = std::make_unique<TestBrowserContext>();

FencedFrameTreeBrowserTest::SetUpOnMainThread();
}

// Tests should call CreateWebContents() to use web_contents() in the test.
void CreateWebContents() {
ASSERT_FALSE(web_contents_.get());
web_contents_ =
WebContents::Create(WebContents::CreateParams(browser_context_.get()));
}

void SetUpCommandLine(base::CommandLine* command_line) override {
FencedFrameTreeBrowserTest::SetUpCommandLine(command_line);
// Browser will ignore certificate errors.
command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
}

void TearDownOnMainThread() override {
web_contents_.reset();
FencedFrameTreeBrowserTest::TearDownOnMainThread();
}

void TearDown() override {
GetUIThreadTaskRunner({})->DeleteSoon(FROM_HERE,
browser_context_.release());
FencedFrameTreeBrowserTest::TearDown();
}

net::EmbeddedTestServer* https_server_mismatched() {
return &https_server_mismatched_;
}

WebContents* web_contents() {
// web_contents_ should be initialized before calling this method.
EXPECT_TRUE(web_contents_.get());
return web_contents_.get();
}

private:
net::EmbeddedTestServer https_server_mismatched_;
std::unique_ptr<BrowserContext> browser_context_;
std::unique_ptr<WebContents> web_contents_;
};

IN_PROC_BROWSER_TEST_P(FencedFrameIgnoreCertErrors, FencedframeHasCertError) {
CreateWebContents();
// Allow insecure content.
ScopedInsecureContentTestContentBrowserClient scoped_content_browser_client;

GURL main_frame_url =
https_server_mismatched()->GetURL("a.test", "/hello.html");
EXPECT_TRUE(NavigateToURL(web_contents(), main_frame_url));
EXPECT_FALSE(web_contents()
->GetController()
.GetLastCommittedEntry()
->GetSSL()
.content_status &
SSLStatus::RAN_CONTENT_WITH_CERT_ERRORS);

// It is safe to obtain the root frame tree node here, as it doesn't change.
FrameTreeNode* root = static_cast<WebContentsImpl*>(web_contents())
->GetPrimaryFrameTree()
.root();

// Create a fenced frame element.
EXPECT_TRUE(ExecJs(root,
"var f = document.createElement('fencedframe');"
"document.body.appendChild(f);"));
ASSERT_EQ(1U, root->child_count());
FrameTreeNode* fenced_frame_root_node =
GetFencedFrameRootNode(root->child_at(0));
EXPECT_TRUE(fenced_frame_root_node->IsInFencedFrameTree());

// Navigate the fenced frame.
GURL fenced_frame_url(https_server_mismatched()->GetURL(
"b.test", "/fenced_frames/title1.html"));
TestFrameNavigationObserver observer(fenced_frame_root_node);
EXPECT_TRUE(ExecJs(root, JsReplace("f.src = $1;", fenced_frame_url.spec())));
observer.WaitForCommit();
EXPECT_EQ(
fenced_frame_url,
fenced_frame_root_node->current_frame_host()->GetLastCommittedURL());

EXPECT_TRUE(web_contents()
->GetController()
.GetLastCommittedEntry()
->GetSSL()
.content_status &
SSLStatus::RAN_CONTENT_WITH_CERT_ERRORS);
}

namespace {
class TestJavaScriptDialogManager : public JavaScriptDialogManager,
public WebContentsDelegate {
Expand Down Expand Up @@ -2485,6 +2617,14 @@ INSTANTIATE_TEST_SUITE_P(
blink::features::FencedFramesImplementationType::kMPArch),
&FencedFrameTreeBrowserTest::DescribeParams);

INSTANTIATE_TEST_SUITE_P(
All,
FencedFrameIgnoreCertErrors,
::testing::Values(
blink::features::FencedFramesImplementationType::kShadowDOM,
blink::features::FencedFramesImplementationType::kMPArch),
&FencedFrameTreeBrowserTest::DescribeParams);

// Parameterized on whether the feature is enabled or not.
class UUIDFrameTreeBrowserTest : public FrameTreeBrowserTest,
public ::testing::WithParamInterface<bool> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,9 @@ void MixedContentNavigationThrottle::ReportBasicMixedContentFeatures(
}

void MixedContentNavigationThrottle::MaybeHandleCertificateError() {
// Main frame certificate errors are handled separately in SSLManager.
if (navigation_handle()->IsInMainFrame()) {
// The outermost main frame certificate errors are handled separately in
// SSLManager.
if (!navigation_handle()->GetParentFrameOrOuterDocument()) {
return;
}

Expand Down
8 changes: 8 additions & 0 deletions content/browser/renderer_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13343,6 +13343,14 @@ void RenderFrameHostImpl::OnDidRunContentWithCertificateErrors() {
DisallowActivationReasonId::kCertificateErrors)) {
return;
}
// To update mixed content status in a fenced frame, we should call
// an outer frame's OnDidRunContentWithCertificateErrors.
// Otherwise, no update can be processed from fenced frames since they have
// their own NavigationController"
if (IsNestedWithinFencedFrame()) {
GetParentOrOuterDocument()->OnDidRunContentWithCertificateErrors();
return;
}
frame_tree_->controller().ssl_manager()->DidRunContentWithCertErrors(
GetMainFrame()->GetLastCommittedOrigin().GetURL());
}
Expand Down

0 comments on commit 876d584

Please sign in to comment.