Commit 16c4707f authored by Mike West's avatar Mike West Committed by Chromium LUCI CQ

Implement embedding opt-in requirement behind a flag.

https://github.com/mikewest/embedding-requires-opt-in suggests that we
shift the default behavior of <iframe>, et al. to require embedees to
opt-into embeddings. This would protect developers by default, and seems
like a reasonable state for us to aim towards.

This patch implements this behavior in AncestorThrottle behind a new
feature flag for experimentation.

Bug: 1153274
Change-Id: Ib164cfde00fef836b409e7a684734862141d030f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2563300Reviewed-by: default avatarCamille Lamy <clamy@chromium.org>
Reviewed-by: default avatarAntonio Sartori <antoniosartori@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832345}
parent 0a480e07
......@@ -4,6 +4,7 @@
#include "content/browser/renderer_host/ancestor_throttle.h"
#include "base/feature_list.h"
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/ranges/algorithm.h"
......@@ -22,6 +23,7 @@
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/navigation_throttle.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/common/content_features.h"
#include "net/http/http_response_headers.h"
#include "services/network/public/cpp/content_security_policy/csp_context.h"
#include "third_party/blink/public/mojom/devtools/console_message.mojom.h"
......@@ -242,6 +244,9 @@ NavigationThrottle::ThrottleCheckResult AncestorThrottle::ProcessResponseImpl(
if (EvaluateXFrameOptions(logging) == CheckResult::BLOCK)
return NavigationThrottle::BLOCK_RESPONSE;
if (EvaluateEmbeddingOptIn(logging) == CheckResult::BLOCK)
return NavigationThrottle::BLOCK_RESPONSE;
// CSPEE is checked only for the final response.
if (is_response_check &&
EvaluateCSPEmbeddedEnforcement() == CheckResult::BLOCK) {
......@@ -295,6 +300,35 @@ void AncestorThrottle::ParseXFrameOptionsError(const std::string& value,
blink::mojom::ConsoleMessageLevel::kError, message);
}
void AncestorThrottle::ConsoleErrorEmbeddingRequiresOptIn() {
DCHECK(base::FeatureList::IsEnabled(features::kEmbeddingRequiresOptIn));
if (!navigation_handle()->GetRenderFrameHost())
return; // Some responses won't have a RFH (i.e. 204/205s or downloads).
std::string message = base::StringPrintf(
"Refused to display '%s' in a frame: It did not opt-into cross-origin "
"embedding by setting either an 'X-Frame-Options' header, or a "
"'Content-Security-Policy' header containing a 'frame-ancestors' "
"directive.",
url::Origin::Create(navigation_handle()->GetURL())
.GetURL()
.spec()
.c_str());
// Log a console error in the parent of the current RenderFrameHost (as
// the current RenderFrameHost itself doesn't yet have a document).
//
// TODO(https://crbug.com/1146651): We should not leak any information at all
// to the parent frame. Send a message directly to Devtools instead (without
// passing through a renderer): that can also contain more information (like
// the full blocked url).
auto* frame = static_cast<RenderFrameHostImpl*>(
navigation_handle()->GetRenderFrameHost());
ParentOrOuterDelegate(frame)->AddMessageToConsole(
blink::mojom::ConsoleMessageLevel::kError, message);
}
void AncestorThrottle::ConsoleErrorXFrameOptions(
HeaderDisposition disposition) {
DCHECK(disposition == HeaderDisposition::DENY ||
......@@ -352,7 +386,9 @@ AncestorThrottle::CheckResult AncestorThrottle::EvaluateXFrameOptions(
if (logging == LoggingDisposition::LOG_TO_CONSOLE)
ParseXFrameOptionsError(header_value, disposition);
RecordXFrameOptionsUsage(XFrameOptionsHistogram::INVALID);
// TODO(mkwst): Consider failing here.
// TODO(mkwst): Consider failing here, especially if we end up shipping
// a new default behavior which requires embedees to explicitly opt-in
// to being embedded: https://crbug.com/1153274.
return CheckResult::PROCEED;
case HeaderDisposition::DENY:
......@@ -401,6 +437,38 @@ AncestorThrottle::CheckResult AncestorThrottle::EvaluateXFrameOptions(
}
}
AncestorThrottle::CheckResult AncestorThrottle::EvaluateEmbeddingOptIn(
LoggingDisposition logging) {
if (!base::FeatureList::IsEnabled(features::kEmbeddingRequiresOptIn))
return CheckResult::PROCEED;
NavigationRequest* request = NavigationRequest::From(navigation_handle());
std::string unused_header_value;
HeaderDisposition xfo_value = ParseXFrameOptionsHeader(
request->GetResponseHeaders(), &unused_header_value);
// If embedding requires opt-in, then we check whether the response opted-into
// embedding via either an 'X-Frame-Options' header or a 'frame-ancestors'
// directive. If neither is present, the response will be blocked unless it is
// same-origin with its ancestor chain.
if (xfo_value == HeaderDisposition::NONE &&
!HeadersContainFrameAncestorsCSP(request->response()->parsed_headers)) {
RenderFrameHostImpl* parent =
ParentOrOuterDelegate(request->frame_tree_node()->current_frame_host());
url::Origin current_origin =
url::Origin::Create(navigation_handle()->GetURL());
while (parent) {
if (!parent->GetLastCommittedOrigin().IsSameOriginWith(current_origin)) {
if (logging == LoggingDisposition::LOG_TO_CONSOLE)
ConsoleErrorEmbeddingRequiresOptIn();
return CheckResult::BLOCK;
}
parent = ParentOrOuterDelegate(parent);
}
}
return CheckResult::PROCEED;
}
AncestorThrottle::CheckResult AncestorThrottle::EvaluateFrameAncestors(
const std::vector<network::mojom::ContentSecurityPolicyPtr>&
content_security_policy) {
......
......@@ -68,10 +68,12 @@ class CONTENT_EXPORT AncestorThrottle : public NavigationThrottle {
void ParseXFrameOptionsError(const std::string& value,
HeaderDisposition disposition);
void ConsoleErrorXFrameOptions(HeaderDisposition disposition);
void ConsoleErrorEmbeddingRequiresOptIn();
CheckResult EvaluateXFrameOptions(LoggingDisposition logging);
CheckResult EvaluateFrameAncestors(
const std::vector<network::mojom::ContentSecurityPolicyPtr>&
content_security_policy);
CheckResult EvaluateEmbeddingOptIn(LoggingDisposition logging);
CheckResult EvaluateCSPEmbeddedEnforcement();
static bool AllowsBlanketEnforcementOfRequiredCSP(
const url::Origin& request_origin,
......
......@@ -7,10 +7,12 @@
#include "base/bind.h"
#include "base/callback_helpers.h"
#include "base/memory/ref_counted.h"
#include "base/test/scoped_feature_list.h"
#include "content/browser/renderer_host/render_frame_host_impl.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/navigation_throttle.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_features.h"
#include "content/public/test/test_renderer_host.h"
#include "content/test/navigation_simulator_impl.h"
#include "content/test/test_navigation_url_loader.h"
......@@ -472,4 +474,119 @@ TEST_F(AncestorThrottleNavigationTest, EvaluateCSPEmbeddedEnforcement) {
}
}
TEST_F(AncestorThrottleNavigationTest, EmbeddingOptInRequirement) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(features::kEmbeddingRequiresOptIn);
// Set up the main frame. We'll add nested frames to it in the tests below.
NavigateAndCommit(GURL("https://www.example.org"));
auto* main_frame = static_cast<TestRenderFrameHost*>(main_rfh());
struct TestCase {
const char* name;
const char* frame_url;
const char* xfo;
const char* csp;
NavigationThrottle::ThrottleAction expected_result;
} cases[] = {{
"Same-origin, no XFO, no CSP",
"https://www.example.org",
nullptr,
nullptr,
NavigationThrottle::PROCEED,
},
{
"Same-site, no XFO, no CSP",
"https://not.example.org",
nullptr,
nullptr,
NavigationThrottle::BLOCK_RESPONSE,
},
{
"Same-site, XFO: ALLOWALL, no CSP",
"https://not.example.org",
"ALLOWALL",
nullptr,
NavigationThrottle::PROCEED,
},
{
"Same-site, XFO: INVALID, no CSP",
"https://not.example.org",
"INVALID",
nullptr,
NavigationThrottle::PROCEED,
},
{
"Same-site, no XFO, CSP: frame-ancestors *",
"https://not.example.org",
nullptr,
"frame-ancestors *",
NavigationThrottle::PROCEED,
},
{
"Same-site, no XFO, CSP without frame-ancestors",
"https://not.example.org",
nullptr,
"img-src 'self'",
NavigationThrottle::BLOCK_RESPONSE,
},
{
"Cross-origin, no XFO, no CSP",
"https://www.not-example.org",
nullptr,
nullptr,
NavigationThrottle::BLOCK_RESPONSE,
},
{
"Cross-origin, XFO: ALLOWALL, no CSP",
"https://www.not-example.org",
"ALLOWALL",
nullptr,
NavigationThrottle::PROCEED,
},
{
"Cross-origin, XFO: INVALID, no CSP",
"https://www.not-example.org",
"INVALID",
nullptr,
NavigationThrottle::PROCEED,
},
{
"Cross-origin, no XFO, CSP: frame-ancestors *",
"https://www.not-example.org",
nullptr,
"frame-ancestors *",
NavigationThrottle::PROCEED,
},
{
"Cross-origin, no XFO, CSP without frame-ancestors",
"https://www.not-example.org",
nullptr,
"img-src 'self'",
NavigationThrottle::BLOCK_RESPONSE,
}};
for (auto test : cases) {
SCOPED_TRACE(test.name);
auto* frame = static_cast<TestRenderFrameHost*>(
content::RenderFrameHostTester::For(main_frame)
->AppendChild(test.name));
std::unique_ptr<NavigationSimulator> simulator =
content::NavigationSimulator::CreateRendererInitiated(
GURL(test.frame_url), frame);
auto response_headers =
base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 200 OK");
if (test.xfo)
response_headers->SetHeader("X-Frame-Options", test.xfo);
if (test.csp)
response_headers->SetHeader("Content-Security-Policy", test.csp);
simulator->SetResponseHeaders(response_headers);
simulator->ReadyToCommit();
EXPECT_EQ(test.expected_result, simulator->GetLastThrottleCheckResult());
}
}
} // namespace content
......@@ -191,6 +191,11 @@ const base::Feature kDocumentPolicy{"DocumentPolicy",
const base::Feature kDocumentPolicyNegotiation{
"DocumentPolicyNegotiation", base::FEATURE_DISABLED_BY_DEFAULT};
// Requires documents embedded via <iframe>, etc, to explicitly opt-into the
// embedding: https://github.com/mikewest/embedding-requires-opt-in.
const base::Feature kEmbeddingRequiresOptIn{"EmbeddingRequiresOptIn",
base::FEATURE_DISABLED_BY_DEFAULT};
// Enables new canvas 2d api features. Enabled either with either
// enable-experimental-canvas-features or new-canvas-2d-api runtime flags
const base::Feature kEnableNewCanvas2DAPI{"EnableNewCanvas2DAPI",
......
......@@ -50,6 +50,7 @@ CONTENT_EXPORT extern const base::Feature kDataSaverHoldback;
CONTENT_EXPORT extern const base::Feature kDesktopCaptureChangeSource;
CONTENT_EXPORT extern const base::Feature kDocumentPolicy;
CONTENT_EXPORT extern const base::Feature kDocumentPolicyNegotiation;
CONTENT_EXPORT extern const base::Feature kEmbeddingRequiresOptIn;
CONTENT_EXPORT extern const base::Feature kEnableNewCanvas2DAPI;
CONTENT_EXPORT extern const base::Feature kEnumerateDevicesHideDeviceIDs;
CONTENT_EXPORT extern const base::Feature kExperimentalAccessibilityLabels;
......
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