Commit d7de94fb authored by Antonio Sartori's avatar Antonio Sartori Committed by Chromium LUCI CQ

Ensure every LocalFrame has a fully functional PolicyContainer

A LocalFrame's PolicyContainer is the counterpart of the
RenderFrameHost's PolicyContainerHost. There are LocalFrames without a
corresponding RenderFrameHost, and there are RenderFrameHost (the
speculative ones) without a PolicyContainerHost. Previously, in those
cases, we were setting a LocalFrame's PolicyContainer to be
nullptr.

We were trying to handle accesses to a non-defined LocalFrame's
PolicyContainer with single nullptr checks. However, a LocalFrame's
PolicyContainer is needed every time a LocalFrame needs to handle
security policies. It turn out this can happen in a myriad of
cases. For example, SVG images included via the img tag (which are
rendered using a fake LocalFrame) allow meta tags delivering security
policies (although those policies effectively do nothing). Using
nullptr checks to handle those cases everywhere is tedious and error
prone.

With this CL we adopt a similar approach as for the
EmptyFrameClients. We always define a PolicyContainer for a
LocalFrame, even if it has no RenderFrameHost. In this case, the
PolicyContainer is an "empty" client: it is fully functional for
Blink's purposes, but it treats all mojo messaging to its
(non-existent) Browser counterpart as no-ops.

Bug: 1158034,1130587
Change-Id: I18347b9a16c6c7d71931c1f34abda793e66a9324
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2594768Reviewed-by: default avatarArthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarPâris Meuleman <pmeuleman@chromium.org>
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843501}
parent 531ec468
...@@ -1481,7 +1481,8 @@ LocalFrame::LocalFrame(LocalFrameClient* client, ...@@ -1481,7 +1481,8 @@ LocalFrame::LocalFrame(LocalFrameClient* client,
: InterfaceRegistry::GetEmptyInterfaceRegistry()), : InterfaceRegistry::GetEmptyInterfaceRegistry()),
is_save_data_enabled_(GetNetworkStateNotifier().SaveDataEnabled()), is_save_data_enabled_(GetNetworkStateNotifier().SaveDataEnabled()),
lifecycle_state_(mojom::FrameLifecycleState::kRunning), lifecycle_state_(mojom::FrameLifecycleState::kRunning),
policy_container_(std::move(policy_container)) { policy_container_(policy_container ? std::move(policy_container)
: PolicyContainer::CreateEmpty()) {
auto frame_tracking_result = GetLocalFramesMap().insert( auto frame_tracking_result = GetLocalFramesMap().insert(
base::UnguessableTokenHash()(frame_token), this); base::UnguessableTokenHash()(frame_token), this);
CHECK(frame_tracking_result.stored_value) << "Inserting a duplicate item."; CHECK(frame_tracking_result.stored_value) << "Inserting a duplicate item.";
......
...@@ -12,6 +12,18 @@ PolicyContainer::PolicyContainer( ...@@ -12,6 +12,18 @@ PolicyContainer::PolicyContainer(
: policies_(std::move(policies)), : policies_(std::move(policies)),
policy_container_host_remote_(std::move(remote)) {} policy_container_host_remote_(std::move(remote)) {}
// static
std::unique_ptr<PolicyContainer> PolicyContainer::CreateEmpty() {
// Create a dummy PolicyContainerHost remote. All the messages will be
// ignored.
mojo::AssociatedRemote<mojom::blink::PolicyContainerHost> dummy_host;
ignore_result(dummy_host.BindNewEndpointAndPassDedicatedReceiver());
return std::make_unique<PolicyContainer>(
dummy_host.Unbind(),
mojom::blink::PolicyContainerDocumentPolicies::New());
}
// static // static
std::unique_ptr<PolicyContainer> PolicyContainer::CreateFromWebPolicyContainer( std::unique_ptr<PolicyContainer> PolicyContainer::CreateFromWebPolicyContainer(
std::unique_ptr<WebPolicyContainer> container) { std::unique_ptr<WebPolicyContainer> container) {
......
...@@ -32,6 +32,7 @@ class CORE_EXPORT PolicyContainer { ...@@ -32,6 +32,7 @@ class CORE_EXPORT PolicyContainer {
PolicyContainer& operator=(const PolicyContainer&) = delete; PolicyContainer& operator=(const PolicyContainer&) = delete;
~PolicyContainer() = default; ~PolicyContainer() = default;
static std::unique_ptr<PolicyContainer> CreateEmpty();
static std::unique_ptr<PolicyContainer> CreateFromWebPolicyContainer( static std::unique_ptr<PolicyContainer> CreateFromWebPolicyContainer(
std::unique_ptr<WebPolicyContainer> container); std::unique_ptr<WebPolicyContainer> container);
......
...@@ -599,11 +599,7 @@ void HTMLMetaElement::ProcessContent() { ...@@ -599,11 +599,7 @@ void HTMLMetaElement::ProcessContent() {
// has no PolicyContainer, so we ignore the next step. This function will // has no PolicyContainer, so we ignore the next step. This function will
// run again anyway, should this document or this element be attached to a // run again anyway, should this document or this element be attached to a
// frame. // frame.
// if (frame) {
// SVG images (loaded via the img tag) create a fake LocalFrame which has
// no PolicyContainer. Ignore policy updates from meta tags inside SVG
// images.
if (frame && frame->GetPolicyContainer()) {
frame->GetPolicyContainer()->UpdateReferrerPolicy( frame->GetPolicyContainer()->UpdateReferrerPolicy(
GetExecutionContext()->GetReferrerPolicy()); GetExecutionContext()->GetReferrerPolicy());
} }
......
...@@ -1835,12 +1835,8 @@ void DocumentLoader::InitializeWindow(Document* owner_document) { ...@@ -1835,12 +1835,8 @@ void DocumentLoader::InitializeWindow(Document* owner_document) {
frame_->DomWindow()->SetAddressSpace(ip_address_space_); frame_->DomWindow()->SetAddressSpace(ip_address_space_);
if (base::FeatureList::IsEnabled(blink::features::kPolicyContainer)) { if (base::FeatureList::IsEnabled(blink::features::kPolicyContainer)) {
// SVG image documents go throught this but don't have a PolicyContainer, so frame_->DomWindow()->SetReferrerPolicy(
// ignore them. frame_->GetPolicyContainer()->GetReferrerPolicy());
if (frame_->GetPolicyContainer()) {
frame_->DomWindow()->SetReferrerPolicy(
frame_->GetPolicyContainer()->GetReferrerPolicy());
}
} }
String referrer_policy_header = String referrer_policy_header =
response_.HttpHeaderField(http_names::kReferrerPolicy); response_.HttpHeaderField(http_names::kReferrerPolicy);
......
...@@ -84,11 +84,6 @@ DummyPageHolder::DummyPageHolder( ...@@ -84,11 +84,6 @@ DummyPageHolder::DummyPageHolder(
if (!local_frame_client_) if (!local_frame_client_)
local_frame_client_ = MakeGarbageCollected<DummyLocalFrameClient>(); local_frame_client_ = MakeGarbageCollected<DummyLocalFrameClient>();
mojo::PendingAssociatedRemote<mojom::blink::PolicyContainerHost>
stub_policy_container_remote;
ignore_result(
stub_policy_container_remote.InitWithNewEndpointAndPassReceiver());
// Create new WindowAgentFactory as this page will be isolated from others. // Create new WindowAgentFactory as this page will be isolated from others.
frame_ = MakeGarbageCollected<LocalFrame>( frame_ = MakeGarbageCollected<LocalFrame>(
local_frame_client_.Get(), *page_, local_frame_client_.Get(), *page_,
...@@ -96,11 +91,7 @@ DummyPageHolder::DummyPageHolder( ...@@ -96,11 +91,7 @@ DummyPageHolder::DummyPageHolder(
/* Frame* previous_sibling */ nullptr, /* Frame* previous_sibling */ nullptr,
FrameInsertType::kInsertInConstructor, base::UnguessableToken::Create(), FrameInsertType::kInsertInConstructor, base::UnguessableToken::Create(),
/* WindowAgentFactory* */ nullptr, /* WindowAgentFactory* */ nullptr,
/* InterfaceRegistry* */ nullptr, /* InterfaceRegistry* */ nullptr, /* policy_container */ nullptr, clock);
std::make_unique<PolicyContainer>(
std::move(stub_policy_container_remote),
mojom::blink::PolicyContainerDocumentPolicies::New()),
clock);
frame_->SetView( frame_->SetView(
MakeGarbageCollected<LocalFrameView>(*frame_, initial_view_size)); MakeGarbageCollected<LocalFrameView>(*frame_, initial_view_size));
frame_->View()->GetPage()->GetVisualViewport().SetSize(initial_view_size); frame_->View()->GetPage()->GetVisualViewport().SetSize(initial_view_size);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment