Commit 4d428fff authored by Charlie Hu's avatar Charlie Hu Committed by Chromium LUCI CQ

[Permissions Policy] Fix Permissions Policy lost issue in XSLT documents

This CL fixes the missing Permissions Policy issue for XSLT document.
Previously, the XSLT navigation will commit a new document without
necessary header information in |DocumentLoader::response_|, resulting
permissions policy header not effective for XSLT documents.

This CL lets document loader copy the state of Permissions Policy
(Feature Policy) from previous document to solve this problem.

Bug: 1151954
Change-Id: I4e1591af65d9e93ad49850fb23f69426bf116dc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2561144Reviewed-by: default avatarDmitry Gozman <dgozman@chromium.org>
Reviewed-by: default avatarIan Clelland <iclelland@chromium.org>
Reviewed-by: default avatarArthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Charlie Hu <chenleihu@google.com>
Cr-Commit-Position: refs/heads/master@{#832008}
parent 2f63e679
...@@ -16,17 +16,16 @@ namespace blink { ...@@ -16,17 +16,16 @@ namespace blink {
namespace { namespace {
// Extracts an Allowlist from a ParsedFeaturePolicyDeclaration. // Extracts an Allowlist from a ParsedFeaturePolicyDeclaration.
std::unique_ptr<FeaturePolicy::Allowlist> AllowlistFromDeclaration( FeaturePolicy::Allowlist AllowlistFromDeclaration(
const ParsedFeaturePolicyDeclaration& parsed_declaration, const ParsedFeaturePolicyDeclaration& parsed_declaration,
const FeaturePolicyFeatureList& feature_list) { const FeaturePolicyFeatureList& feature_list) {
std::unique_ptr<FeaturePolicy::Allowlist> result = auto result = FeaturePolicy::Allowlist();
base::WrapUnique(new FeaturePolicy::Allowlist());
if (parsed_declaration.matches_all_origins) if (parsed_declaration.matches_all_origins)
result->AddAll(); result.AddAll();
if (parsed_declaration.matches_opaque_src) if (parsed_declaration.matches_opaque_src)
result->AddOpaqueSrc(); result.AddOpaqueSrc();
for (const auto& value : parsed_declaration.allowed_origins) for (const auto& value : parsed_declaration.allowed_origins)
result->Add(value); result.Add(value);
return result; return result;
} }
...@@ -120,6 +119,21 @@ std::unique_ptr<FeaturePolicy> FeaturePolicy::CreateWithOpenerPolicy( ...@@ -120,6 +119,21 @@ std::unique_ptr<FeaturePolicy> FeaturePolicy::CreateWithOpenerPolicy(
return new_policy; return new_policy;
} }
// static
std::unique_ptr<FeaturePolicy> FeaturePolicy::CopyStateFrom(
const FeaturePolicy* source) {
if (!source)
return nullptr;
std::unique_ptr<FeaturePolicy> new_policy = base::WrapUnique(
new FeaturePolicy(source->origin_, GetFeaturePolicyFeatureList()));
new_policy->inherited_policies_ = source->inherited_policies_;
new_policy->allowlists_ = source->allowlists_;
return new_policy;
}
bool FeaturePolicy::IsFeatureEnabled( bool FeaturePolicy::IsFeatureEnabled(
mojom::FeaturePolicyFeature feature) const { mojom::FeaturePolicyFeature feature) const {
return IsFeatureEnabledForOrigin(feature, origin_); return IsFeatureEnabledForOrigin(feature, origin_);
...@@ -134,7 +148,7 @@ bool FeaturePolicy::IsFeatureEnabledForOrigin( ...@@ -134,7 +148,7 @@ bool FeaturePolicy::IsFeatureEnabledForOrigin(
auto inherited_value = inherited_policies_.at(feature); auto inherited_value = inherited_policies_.at(feature);
auto allowlist = allowlists_.find(feature); auto allowlist = allowlists_.find(feature);
if (allowlist != allowlists_.end()) { if (allowlist != allowlists_.end()) {
return inherited_value && allowlist->second->Contains(origin); return inherited_value && allowlist->second.Contains(origin);
} }
// If no "allowlist" is specified, return default feature value. // If no "allowlist" is specified, return default feature value.
...@@ -155,7 +169,7 @@ bool FeaturePolicy::GetFeatureValueForOrigin( ...@@ -155,7 +169,7 @@ bool FeaturePolicy::GetFeatureValueForOrigin(
auto inherited_value = inherited_policies_.at(feature); auto inherited_value = inherited_policies_.at(feature);
auto allowlist = allowlists_.find(feature); auto allowlist = allowlists_.find(feature);
if (allowlist != allowlists_.end()) { if (allowlist != allowlists_.end()) {
return inherited_value && allowlist->second->Contains(origin); return inherited_value && allowlist->second.Contains(origin);
} }
return inherited_value; return inherited_value;
...@@ -172,7 +186,7 @@ const FeaturePolicy::Allowlist FeaturePolicy::GetAllowlistForFeature( ...@@ -172,7 +186,7 @@ const FeaturePolicy::Allowlist FeaturePolicy::GetAllowlistForFeature(
// Return defined policy if exists; otherwise return default policy. // Return defined policy if exists; otherwise return default policy.
auto allowlist = allowlists_.find(feature); auto allowlist = allowlists_.find(feature);
if (allowlist != allowlists_.end()) if (allowlist != allowlists_.end())
return FeaturePolicy::Allowlist(*(allowlist->second)); return allowlist->second;
const FeaturePolicyFeatureDefault default_policy = feature_list_.at(feature); const FeaturePolicyFeatureDefault default_policy = feature_list_.at(feature);
FeaturePolicy::Allowlist default_allowlist; FeaturePolicy::Allowlist default_allowlist;
...@@ -192,8 +206,8 @@ void FeaturePolicy::SetHeaderPolicy(const ParsedFeaturePolicy& parsed_header) { ...@@ -192,8 +206,8 @@ void FeaturePolicy::SetHeaderPolicy(const ParsedFeaturePolicy& parsed_header) {
parsed_header) { parsed_header) {
mojom::FeaturePolicyFeature feature = parsed_declaration.feature; mojom::FeaturePolicyFeature feature = parsed_declaration.feature;
DCHECK(feature != mojom::FeaturePolicyFeature::kNotFound); DCHECK(feature != mojom::FeaturePolicyFeature::kNotFound);
allowlists_[feature] = allowlists_.emplace(
AllowlistFromDeclaration(parsed_declaration, feature_list_); feature, AllowlistFromDeclaration(parsed_declaration, feature_list_));
} }
} }
...@@ -259,7 +273,7 @@ bool FeaturePolicy::InheritedValueForFeature( ...@@ -259,7 +273,7 @@ bool FeaturePolicy::InheritedValueForFeature(
// 9.8 5.1: If the allowlist for feature in container policy matches // 9.8 5.1: If the allowlist for feature in container policy matches
// origin, return "Enabled". // origin, return "Enabled".
// 9.8 5.2: Otherwise return "Disabled". // 9.8 5.2: Otherwise return "Disabled".
return AllowlistFromDeclaration(decl, feature_list_)->Contains(origin_); return AllowlistFromDeclaration(decl, feature_list_).Contains(origin_);
} }
} }
// 9.8 6: If feature’s default allowlist is *, return "Enabled". // 9.8 6: If feature’s default allowlist is *, return "Enabled".
......
...@@ -173,6 +173,8 @@ class BLINK_COMMON_EXPORT FeaturePolicy { ...@@ -173,6 +173,8 @@ class BLINK_COMMON_EXPORT FeaturePolicy {
const FeaturePolicyFeatureState& inherited_policies, const FeaturePolicyFeatureState& inherited_policies,
const url::Origin& origin); const url::Origin& origin);
static std::unique_ptr<FeaturePolicy> CopyStateFrom(const FeaturePolicy*);
bool IsFeatureEnabled(mojom::FeaturePolicyFeature feature) const; bool IsFeatureEnabled(mojom::FeaturePolicyFeature feature) const;
// Returns whether or not the given feature is enabled by this policy for a // Returns whether or not the given feature is enabled by this policy for a
...@@ -226,7 +228,7 @@ class BLINK_COMMON_EXPORT FeaturePolicy { ...@@ -226,7 +228,7 @@ class BLINK_COMMON_EXPORT FeaturePolicy {
// Map of feature names to declared allowlists. Any feature which is missing // Map of feature names to declared allowlists. Any feature which is missing
// from this map should use the inherited policy. // from this map should use the inherited policy.
std::map<mojom::FeaturePolicyFeature, std::unique_ptr<Allowlist>> allowlists_; std::map<mojom::FeaturePolicyFeature, Allowlist> allowlists_;
// Records whether or not each feature was enabled for this frame by its // Records whether or not each feature was enabled for this frame by its
// parent frame. // parent frame.
......
...@@ -146,6 +146,9 @@ class CORE_EXPORT SecurityContext { ...@@ -146,6 +146,9 @@ class CORE_EXPORT SecurityContext {
const FeaturePolicy* GetFeaturePolicy() const { const FeaturePolicy* GetFeaturePolicy() const {
return feature_policy_.get(); return feature_policy_.get();
} }
const FeaturePolicy* GetReportOnlyFeaturePolicy() const {
return report_only_feature_policy_.get();
}
void SetFeaturePolicy(std::unique_ptr<FeaturePolicy>); void SetFeaturePolicy(std::unique_ptr<FeaturePolicy>);
void SetReportOnlyFeaturePolicy(std::unique_ptr<FeaturePolicy>); void SetReportOnlyFeaturePolicy(std::unique_ptr<FeaturePolicy>);
......
...@@ -266,4 +266,11 @@ void SecurityContextInit::ApplyFeaturePolicy( ...@@ -266,4 +266,11 @@ void SecurityContextInit::ApplyFeaturePolicy(
} }
} }
void SecurityContextInit::InitFeaturePolicyFrom(const SecurityContext& other) {
auto& security_context = execution_context_->GetSecurityContext();
security_context.SetFeaturePolicy(
FeaturePolicy::CopyStateFrom(other.GetFeaturePolicy()));
security_context.SetReportOnlyFeaturePolicy(
FeaturePolicy::CopyStateFrom(other.GetReportOnlyFeaturePolicy()));
}
} // namespace blink } // namespace blink
...@@ -29,6 +29,13 @@ class CORE_EXPORT SecurityContextInit { ...@@ -29,6 +29,13 @@ class CORE_EXPORT SecurityContextInit {
public: public:
explicit SecurityContextInit(ExecutionContext*); explicit SecurityContextInit(ExecutionContext*);
// Init |feature_policy_| and |report_only_feature_policy_| by copying
// state from another security context instance.
// Used to carry feature policy information from previous document
// to current document during XSLT navigation, because XSLT navigation
// does not have header information available.
void InitFeaturePolicyFrom(const SecurityContext& other);
void ApplyFeaturePolicy(LocalFrame* frame, void ApplyFeaturePolicy(LocalFrame* frame,
const ResourceResponse& response, const ResourceResponse& response,
const base::Optional<WebOriginPolicy>& origin_policy, const base::Optional<WebOriginPolicy>& origin_policy,
......
...@@ -1697,11 +1697,24 @@ void DocumentLoader::CommitNavigation() { ...@@ -1697,11 +1697,24 @@ void DocumentLoader::CommitNavigation() {
InitializeWindow(owner_document); InitializeWindow(owner_document);
SecurityContextInit security_init(frame_->DomWindow()); SecurityContextInit security_init(frame_->DomWindow());
// FeaturePolicy and DocumentPolicy require SecurityOrigin and origin trials
// to be initialized. // The document constructed by XSLTProcessor should inherit Feature Policy
// TODO(iclelland): Add Feature-Policy-Report-Only to Origin Policy. // from the previous Document. Note: In XSLT commit, |response_| no longer
security_init.ApplyFeaturePolicy(frame_.Get(), response_, origin_policy_, // holds header fields. Going through regular initialization will cause empty
frame_policy_); // policy even if there is header on xml document.
// TODO(crbug.com/1151954): Fix the problem for Document Policy as well.
if (commit_reason_ == CommitReason::kXSLT) {
DCHECK(response_.HttpHeaderField(http_names::kFeaturePolicy).IsEmpty());
DCHECK(response_.HttpHeaderField(http_names::kPermissionsPolicy).IsEmpty());
security_init.InitFeaturePolicyFrom(previous_window->GetSecurityContext());
} else {
// FeaturePolicy and DocumentPolicy require SecurityOrigin and origin trials
// to be initialized.
// TODO(iclelland): Add Feature-Policy-Report-Only to Origin Policy.
security_init.ApplyFeaturePolicy(frame_.Get(), response_, origin_policy_,
frame_policy_);
}
// |document_policy_| is parsed in document loader because it is // |document_policy_| is parsed in document loader because it is
// compared with |frame_policy.required_document_policy| to decide // compared with |frame_policy.required_document_policy| to decide
// whether to block the document load or not. // whether to block the document load or not.
......
...@@ -5277,8 +5277,6 @@ crbug.com/993790 external/wpt/document-policy/required-policy/separate-document- ...@@ -5277,8 +5277,6 @@ crbug.com/993790 external/wpt/document-policy/required-policy/separate-document-
crbug.com/1134464 http/tests/images/document-policy/document-policy-oversized-images-edge-cases.php [ Pass Timeout ] crbug.com/1134464 http/tests/images/document-policy/document-policy-oversized-images-edge-cases.php [ Pass Timeout ]
crbug.com/1151954 http/tests/permissions-policy/permissions-policy-in-xsl.php [ Failure ]
# Skipping because of unimplemented behaviour # Skipping because of unimplemented behaviour
crbug.com/965495 external/wpt/feature-policy/experimental-features/focus-without-user-activation-enabled-tentative.sub.html [ Skip ] crbug.com/965495 external/wpt/feature-policy/experimental-features/focus-without-user-activation-enabled-tentative.sub.html [ Skip ]
crbug.com/965495 external/wpt/permissions-policy/experimental-features/focus-without-user-activation-enabled-tentative.sub.html [ Skip ] crbug.com/965495 external/wpt/permissions-policy/experimental-features/focus-without-user-activation-enabled-tentative.sub.html [ Skip ]
......
<?php
header("Permissions-Policy-Report-Only: document-domain=()");
header("Content-Type: application/xml");
echo '<?xml version="1.0"?>
<?xml-stylesheet href="resources/permissions-policy-report-only-in-xsl.xslt" type="text/xsl"?>
<page>
</page>';
?>
\ No newline at end of file
<?xml version="1.0"?>
<xsl:stylesheet version="1.0"
xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
<xsl:template match="page">
<html>
<head>
<title> Test XSLT </title>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script>
// 'document-domain' is enabled in the page but is disabled
// in Permissions-Policy-Report-Only header.
// A permissions-policy-violation report is expected.
async_test(t => {
new ReportingObserver(t.step_func_done((reports, _) => {
assert_equals(reports.length, 1);
const report = reports[0];
assert_equals(report.type, 'permissions-policy-violation');
assert_equals(report.body.featureId, 'document-domain');
assert_equals(report.body.disposition, 'report');
}), {types: ['permissions-policy-violation']}).observe();
});
document.domain = document.domain;
</script>
</head>
<body bgcolor="#ffffff">
</body>
</html>
</xsl:template>
</xsl:stylesheet>
\ No newline at end of file
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