Skip to content

Commit

Permalink
Fenced frames: Add reporting to custom destination URLs [5/N]
Browse files Browse the repository at this point in the history 
Add extra checks on the destination URL for macro substitution. These
checks should be covered by existing checks, but the new versions should
be more robust to future changes and give more helpful error messages.

WICG/turtledove#477

Change-Id: I406c7da718cb7a757b3fffc8af0795490e64f166
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4818548
Reviewed-by: Shivani Sharma <[email protected]>
Commit-Queue: Garrett Tanzer <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1195573}
  • Loading branch information
Garrett Tanzer authored and Chromium LUCI CQ committed Sep 12, 2023
1 parent b2be5fa commit 10340ff
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 8 deletions.
42 changes: 34 additions & 8 deletions content/browser/fenced_frame/fenced_frame_reporter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ bool FencedFrameReporter::SendReportInternal(
DCHECK(reporting_destination_info.reporting_url_map);

// Compute the destination url for the report.
GURL url;
GURL destination_url;
if (absl::holds_alternative<DestinationEnumEvent>(event_variant)) {
std::string event_type =
absl::get<DestinationEnumEvent>(event_variant).type;
Expand All @@ -393,8 +393,8 @@ bool FencedFrameReporter::SendReportInternal(
}

// Validate the reporting URL.
url = url_iter->second;
if (!url.is_valid() || !url.SchemeIsHTTPOrHTTPS()) {
destination_url = url_iter->second;
if (!destination_url.is_valid() || !destination_url.SchemeIsHTTPOrHTTPS()) {
error_message = base::StrCat(
{"This frame registered invalid reporting url for destination '",
ReportingDestinationAsString(reporting_destination),
Expand Down Expand Up @@ -448,13 +448,38 @@ bool FencedFrameReporter::SendReportInternal(
return false;
}

const GURL& original_url =
absl::get<DestinationURLEvent>(event_variant).url;
if (!original_url.is_valid() || !original_url.SchemeIs(url::kHttpsScheme)) {
attempted_custom_url_report_to_disallowed_origin_ = true;
error_message =
"This frame attempted to send a report to an invalid custom "
"destination URL. No further reports to custom destination URLs will "
"be allowed for this fenced frame config.";
console_message_level = blink::mojom::ConsoleMessageLevel::kError;
NotifyFencedFrameReportingBeaconFailed(attribution_reporting_data);
return false;
}

// Substitute macros in the specified URL using the macros.
url = GURL(SubstituteMappedStrings(
absl::get<DestinationURLEvent>(event_variant).url.spec(),
destination_url = GURL(SubstituteMappedStrings(
original_url.spec(),
reporting_destination_info.reporting_ad_macros.value()));
url::Origin destination_origin = url::Origin::Create(url);
if (!destination_url.is_valid() ||
!destination_url.SchemeIs(url::kHttpsScheme)) {
attempted_custom_url_report_to_disallowed_origin_ = true;
error_message =
"This frame attempted to send a report to a custom destination URL "
"that is invalid after macro substitution. No further reports to "
"custom destination URLs will be allowed for this fenced frame "
"config.";
console_message_level = blink::mojom::ConsoleMessageLevel::kError;
NotifyFencedFrameReportingBeaconFailed(attribution_reporting_data);
return false;
}

// Check whether the destination URL has an allowed origin.
url::Origin destination_origin = url::Origin::Create(destination_url);
bool is_allowed_origin = false;
for (auto& origin : allowed_reporting_origins_.value()) {
if (origin.IsSameOriginWith(destination_origin)) {
Expand All @@ -481,7 +506,8 @@ bool FencedFrameReporter::SendReportInternal(
if (!GetContentClient()
->browser()
->IsPrivacySandboxReportingDestinationAttested(
browser_context_, url::Origin::Create(url), invoking_api_)) {
browser_context_, url::Origin::Create(destination_url),
invoking_api_)) {
error_message = base::StrCat({
"The reporting destination '",
ReportingDestinationAsString(reporting_destination),
Expand All @@ -497,7 +523,7 @@ bool FencedFrameReporter::SendReportInternal(
// Construct the resource request.
auto request = std::make_unique<network::ResourceRequest>();

request->url = url;
request->url = destination_url;
request->mode = network::mojom::RequestMode::kCors;
request->request_initiator = request_initiator;
request->credentials_mode = network::mojom::CredentialsMode::kOmit;
Expand Down
115 changes: 115 additions & 0 deletions content/browser/fenced_frame/fenced_frame_reporter_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,121 @@ TEST_F(FencedFrameReporterTest, CustomDestinationURLNestedMacro) {
report_destination_substituted, absl::nullopt);
}

// Test that reports to HTTP custom destination URLs fail.
TEST_F(FencedFrameReporterTest, CustomDestinationHTTPURL) {
scoped_refptr<FencedFrameReporter> reporter =
FencedFrameReporter::CreateForFledge(
shared_url_loader_factory(), browser_context(),
/*direct_seller_is_seller=*/false, &private_aggregation_manager_,
main_frame_origin_,
/*winner_origin=*/report_destination_origin_,
/*allowed_reporting_origins=*/
{{report_destination_origin_}});

// Receive buyer mapping.
reporter->OnUrlMappingReady(blink::FencedFrame::ReportingDestination::kBuyer,
/*reporting_url_map=*/{{}},
/*reporting_ad_macros=*/
FencedFrameReporter::ReportingMacros({}));
EXPECT_EQ(test_url_loader_factory_.NumPending(), 0);

GURL custom_report_destination{"http://report_destination.test"};

std::string error_message;
blink::mojom::ConsoleMessageLevel console_message_level =
blink::mojom::ConsoleMessageLevel::kError;
EXPECT_FALSE(reporter->SendReport(
DestinationURLEvent(custom_report_destination),
blink::FencedFrame::ReportingDestination::kBuyer, main_rfh_impl(),
network::AttributionReportingRuntimeFeatures(), error_message,
console_message_level));
EXPECT_EQ(
error_message,
"This frame attempted to send a report to an invalid custom "
"destination URL. No further reports to custom destination URLs will "
"be allowed for this fenced frame config.");
EXPECT_EQ(console_message_level, blink::mojom::ConsoleMessageLevel::kError);
EXPECT_EQ(test_url_loader_factory_.NumPending(), 0);
}

// Test that reports to invalid custom destination URLs fail.
TEST_F(FencedFrameReporterTest, CustomDestinationInvalidURL) {
scoped_refptr<FencedFrameReporter> reporter =
FencedFrameReporter::CreateForFledge(
shared_url_loader_factory(), browser_context(),
/*direct_seller_is_seller=*/false, &private_aggregation_manager_,
main_frame_origin_,
/*winner_origin=*/report_destination_origin_,
/*allowed_reporting_origins=*/
{{report_destination_origin_}});

// Receive buyer mapping.
reporter->OnUrlMappingReady(blink::FencedFrame::ReportingDestination::kBuyer,
/*reporting_url_map=*/{{}},
/*reporting_ad_macros=*/
FencedFrameReporter::ReportingMacros({}));
EXPECT_EQ(test_url_loader_factory_.NumPending(), 0);

GURL custom_report_destination{"https://"};

std::string error_message;
blink::mojom::ConsoleMessageLevel console_message_level =
blink::mojom::ConsoleMessageLevel::kError;
EXPECT_FALSE(reporter->SendReport(
DestinationURLEvent(custom_report_destination),
blink::FencedFrame::ReportingDestination::kBuyer, main_rfh_impl(),
network::AttributionReportingRuntimeFeatures(), error_message,
console_message_level));
EXPECT_EQ(
error_message,
"This frame attempted to send a report to an invalid custom "
"destination URL. No further reports to custom destination URLs will "
"be allowed for this fenced frame config.");
EXPECT_EQ(console_message_level, blink::mojom::ConsoleMessageLevel::kError);
EXPECT_EQ(test_url_loader_factory_.NumPending(), 0);
}

// Test that reports to custom destination URLs that are invalid after macro
// substitution will fail.
TEST_F(FencedFrameReporterTest, CustomDestinationInvalidURLAfterMacros) {
scoped_refptr<FencedFrameReporter> reporter =
FencedFrameReporter::CreateForFledge(
shared_url_loader_factory(), browser_context(),
/*direct_seller_is_seller=*/false, &private_aggregation_manager_,
main_frame_origin_,
/*winner_origin=*/report_destination_origin_,
/*allowed_reporting_origins=*/
{{report_destination_origin_}});

// Receive buyer mapping.
// This macro isn't the format that will be used by Protected Audience, but it
// makes it easy to test this particular case.
reporter->OnUrlMappingReady(blink::FencedFrame::ReportingDestination::kBuyer,
/*reporting_url_map=*/{{}},
/*reporting_ad_macros=*/
FencedFrameReporter::ReportingMacros(
{{"https://report_destination.test", ""}}));
EXPECT_EQ(test_url_loader_factory_.NumPending(), 0);

GURL custom_report_destination{"https://report_destination.test"};

std::string error_message;
blink::mojom::ConsoleMessageLevel console_message_level =
blink::mojom::ConsoleMessageLevel::kError;
EXPECT_FALSE(reporter->SendReport(
DestinationURLEvent(custom_report_destination),
blink::FencedFrame::ReportingDestination::kBuyer, main_rfh_impl(),
network::AttributionReportingRuntimeFeatures(), error_message,
console_message_level));
EXPECT_EQ(error_message,
"This frame attempted to send a report to a custom destination URL "
"that is invalid after macro substitution. No further reports to "
"custom destination URLs will be allowed for this fenced frame "
"config.");
EXPECT_EQ(console_message_level, blink::mojom::ConsoleMessageLevel::kError);
EXPECT_EQ(test_url_loader_factory_.NumPending(), 0);
}

// Test allowlist for reports to custom destination URLs.
TEST_F(FencedFrameReporterTest, CustomDestinationURLAllowlist) {
scoped_refptr<FencedFrameReporter> reporter =
Expand Down

0 comments on commit 10340ff

Please sign in to comment.