FLEDGE: Fix mojo validation of the special non-k-anon PA reporting.
The check in use would error out on non-event contributions with
non-null filter ID, while the invariant is actually that everything
must be an event contribution using reject-reason base-value.
(So everything rejected was illegal, but for the wrong reason,
and many things that should have been weren't).
Switch to the proper check, shared with what the worklet uses to
filter this field in the first place.
Change-Id: I6c8f9ea9619724dff40b64c4265f5c880aeb1c22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5841914
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Qingxin Wu <qingxinwu@google.com>
Cr-Commit-Position: refs/heads/main@{#1352791}
diff --git a/content/browser/interest_group/auction_runner_unittest.cc b/content/browser/interest_group/auction_runner_unittest.cc
index 343aff6..0a56429 100644
--- a/content/browser/interest_group/auction_runner_unittest.cc
+++ b/content/browser/interest_group/auction_runner_unittest.cc
@@ -18161,6 +18161,70 @@
testing::UnorderedElementsAre());
}
+TEST_F(AuctionRunnerTest, PrivateAggregationBuyerReservedOnceFeatureDisabled2) {
+ // Test that the validation happens on the non-k-anon list, too.
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndDisableFeature(
+ blink::features::
+ kPrivateAggregationApiProtectedAudienceAdditionalExtensions);
+
+ std::vector<auction_worklet::mojom::PrivateAggregationRequestPtr> pa_requests;
+ pa_requests.push_back(
+ BuildPrivateAggregationForEventRequest(
+ /*bucket=*/123, /*value=*/4,
+ /*event_type=*/
+ Reserved(auction_worklet::mojom::ReservedEventType::kReservedOnce),
+ /*filtering_id=*/std::nullopt)
+ .Clone());
+
+ StartStandardAuctionWithMockService();
+
+ auto seller_worklet = mock_auction_process_manager_->TakeSellerWorklet();
+ ASSERT_TRUE(seller_worklet);
+ auto bidder1_worklet =
+ mock_auction_process_manager_->TakeBidderWorklet(kBidder1Url);
+ ASSERT_TRUE(bidder1_worklet);
+ auto bidder2_worklet =
+ mock_auction_process_manager_->TakeBidderWorklet(kBidder2Url);
+ ASSERT_TRUE(bidder2_worklet);
+
+ bidder1_worklet->SetNonKAnonPARequests(std::move(pa_requests));
+ bidder1_worklet->InvokeGenerateBidCallback(
+ /*bid=*/6, /*bid_currency=*/std::nullopt,
+ blink::AdDescriptor(GURL("https://ad1.com/")),
+ auction_worklet::mojom::BidRole::kUnenforcedKAnon,
+ /*further_bids=*/{},
+ /*ad_component_descriptors=*/std::nullopt,
+ /*duration=*/base::TimeDelta(),
+ /*bidding_signals_data_version=*/std::nullopt,
+ /*debug_loss_report_url=*/std::nullopt,
+ /*debug_win_report_url=*/std::nullopt,
+ /*pa_requests=*/{},
+ /*real_time_contributions=*/{},
+ /*dependency_latencies=*/
+ auction_worklet::mojom::GenerateBidDependencyLatenciesPtr(),
+ auction_worklet::mojom::RejectReason::kNotAvailable);
+ // Bidder 2 doesn't bid.
+ bidder2_worklet->InvokeGenerateBidCallback(/*bid=*/std::nullopt);
+
+ // Since there's no acceptable bid, the seller worklet is never asked to
+ // score a bid.
+ auction_run_loop_->Run();
+
+ EXPECT_EQ("Private Aggregation request using disabled features",
+ TakeBadMessage());
+
+ // No bidder won.
+ EXPECT_FALSE(result_.winning_group_id);
+ EXPECT_FALSE(result_.ad_descriptor);
+ EXPECT_TRUE(result_.ad_component_descriptors.empty());
+ EXPECT_THAT(result_.errors, testing::ElementsAre());
+ EXPECT_THAT(result_.interest_groups_that_bid,
+ testing::UnorderedElementsAre());
+ EXPECT_THAT(private_aggregation_manager_.TakePrivateAggregationRequests(),
+ testing::UnorderedElementsAre());
+}
+
TEST_F(AuctionRunnerTest, PrivateAggregationSellerReservedOnceFeatureDisabled) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
@@ -18298,6 +18362,66 @@
}
}
+// Test that non-kanon PA contributions gets its special checks for the right
+// form. (It should get rejected since it doesn't use reject-reason).
+TEST_F(AuctionRunnerTest, PrivateAggregationNonKAnonBadContribution) {
+ std::vector<auction_worklet::mojom::PrivateAggregationRequestPtr> pa_requests;
+ pa_requests.push_back(
+ BuildPrivateAggregationForEventRequest(
+ /*bucket=*/123, /*value=*/4,
+ /*event_type=*/
+ Reserved(auction_worklet::mojom::ReservedEventType::kReservedAlways),
+ /*filtering_id=*/std::nullopt)
+ .Clone());
+
+ StartStandardAuctionWithMockService();
+
+ auto seller_worklet = mock_auction_process_manager_->TakeSellerWorklet();
+ ASSERT_TRUE(seller_worklet);
+ auto bidder1_worklet =
+ mock_auction_process_manager_->TakeBidderWorklet(kBidder1Url);
+ ASSERT_TRUE(bidder1_worklet);
+ auto bidder2_worklet =
+ mock_auction_process_manager_->TakeBidderWorklet(kBidder2Url);
+ ASSERT_TRUE(bidder2_worklet);
+
+ bidder1_worklet->SetNonKAnonPARequests(std::move(pa_requests));
+ bidder1_worklet->InvokeGenerateBidCallback(
+ /*bid=*/6, /*bid_currency=*/std::nullopt,
+ blink::AdDescriptor(GURL("https://ad1.com/")),
+ auction_worklet::mojom::BidRole::kUnenforcedKAnon,
+ /*further_bids=*/{},
+ /*ad_component_descriptors=*/std::nullopt,
+ /*duration=*/base::TimeDelta(),
+ /*bidding_signals_data_version=*/std::nullopt,
+ /*debug_loss_report_url=*/std::nullopt,
+ /*debug_win_report_url=*/std::nullopt,
+ /*pa_requests=*/{},
+ /*real_time_contributions=*/{},
+ /*dependency_latencies=*/
+ auction_worklet::mojom::GenerateBidDependencyLatenciesPtr(),
+ auction_worklet::mojom::RejectReason::kNotAvailable);
+ // Bidder 2 doesn't bid.
+ bidder2_worklet->InvokeGenerateBidCallback(/*bid=*/std::nullopt);
+
+ // Since there's no acceptable bid, the seller worklet is never asked to
+ // score a bid.
+ auction_run_loop_->Run();
+
+ EXPECT_EQ("Incorrect non-kanon Private Aggregation request",
+ TakeBadMessage());
+
+ // No bidder won.
+ EXPECT_FALSE(result_.winning_group_id);
+ EXPECT_FALSE(result_.ad_descriptor);
+ EXPECT_TRUE(result_.ad_component_descriptors.empty());
+ EXPECT_THAT(result_.errors, testing::ElementsAre());
+ EXPECT_THAT(result_.interest_groups_that_bid,
+ testing::UnorderedElementsAre());
+ EXPECT_THAT(private_aggregation_manager_.TakePrivateAggregationRequests(),
+ testing::UnorderedElementsAre());
+}
+
TEST_F(AuctionRunnerTest, RealTimeReportingSellerBadContribution) {
const struct TestCase {
const char* expected_error_message;
diff --git a/content/browser/interest_group/interest_group_auction.cc b/content/browser/interest_group/interest_group_auction.cc
index 9827c95..1f1d8b6 100644
--- a/content/browser/interest_group/interest_group_auction.cc
+++ b/content/browser/interest_group/interest_group_auction.cc
@@ -72,6 +72,7 @@
#include "content/public/browser/auction_result.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/content_browser_client.h"
+#include "content/services/auction_worklet/public/cpp/private_aggregation_reporting.h"
#include "content/services/auction_worklet/public/cpp/real_time_reporting.h"
#include "content/services/auction_worklet/public/mojom/bidder_worklet.mojom-forward.h"
#include "content/services/auction_worklet/public/mojom/bidder_worklet.mojom.h"
@@ -923,17 +924,12 @@
return false;
}
- if (base::ranges::any_of(
- non_kanon_pa_requests,
- [](const auction_worklet::mojom::PrivateAggregationRequestPtr&
- request_ptr) {
- return request_ptr->contribution->is_histogram_contribution() &&
- request_ptr->contribution->get_histogram_contribution()
- ->filtering_id.has_value();
- })) {
- generate_bid_client_receiver_set.ReportBadMessage(
- "Filtering ID set inappropriately");
- return false;
+ for (const auto& non_kanon_request : non_kanon_pa_requests) {
+ if (!auction_worklet::HasKAnonFailureComponent(*non_kanon_request)) {
+ generate_bid_client_receiver_set.ReportBadMessage(
+ "Incorrect non-kanon Private Aggregation request");
+ return false;
+ }
}
return true;
diff --git a/content/browser/interest_group/mock_auction_process_manager.cc b/content/browser/interest_group/mock_auction_process_manager.cc
index 80570687..f628777 100644
--- a/content/browser/interest_group/mock_auction_process_manager.cc
+++ b/content/browser/interest_group/mock_auction_process_manager.cc
@@ -262,6 +262,12 @@
/*generate_bid_finish_time=*/base::TimeTicks::Now());
}
+ std::vector<auction_worklet::mojom::PrivateAggregationRequestPtr>
+ non_kanon_pa_requests;
+ for (const auto& request : non_kanon_pa_requests_) {
+ non_kanon_pa_requests.push_back(request->Clone());
+ }
+
std::vector<auction_worklet::mojom::BidderWorkletBidPtr> bids;
if (!bid.has_value()) {
DCHECK(further_bids.empty());
@@ -274,7 +280,7 @@
base::flat_map<std::string,
auction_worklet::mojom::PrioritySignalsDoublePtr>(),
/*pa_requests=*/std::move(pa_requests),
- /*non_kanon_pa_requests=*/{},
+ /*non_kanon_pa_requests=*/std::move(non_kanon_pa_requests),
/*real_time_contributions=*/{},
/*bidding_latency=*/bidding_latency_,
/*generate_bid_dependency_latencies=*/std::move(dependency_latencies),
@@ -299,7 +305,7 @@
base::flat_map<std::string,
auction_worklet::mojom::PrioritySignalsDoublePtr>(),
/*pa_requests=*/std::move(pa_requests),
- /*non_kanon_pa_requests=*/{},
+ /*non_kanon_pa_requests=*/std::move(non_kanon_pa_requests),
/*real_time_contributions=*/std::move(real_time_contributions),
/*bidding_latency=*/bidding_latency_,
/*generate_bid_dependency_latencies=*/std::move(dependency_latencies),
diff --git a/content/browser/interest_group/mock_auction_process_manager.h b/content/browser/interest_group/mock_auction_process_manager.h
index e37efb6..23120da 100644
--- a/content/browser/interest_group/mock_auction_process_manager.h
+++ b/content/browser/interest_group/mock_auction_process_manager.h
@@ -148,6 +148,14 @@
reporting_latency_ = delta;
}
+ // Controls what's passed to `non_kanon_pa_requests` of the generate bid
+ // callback.
+ void SetNonKAnonPARequests(
+ std::vector<auction_worklet::mojom::PrivateAggregationRequestPtr>
+ non_kanon_pa_requests) {
+ non_kanon_pa_requests_ = std::move(non_kanon_pa_requests);
+ }
+
// Invokes the GenerateBid callback. A bid of base::nullopt means no bid
// should be offered. Waits for the GenerateBid() call first, if needed.
void InvokeGenerateBidCallback(
@@ -210,6 +218,9 @@
std::optional<std::string> selected_buyer_and_seller_reporting_id_;
+ std::vector<auction_worklet::mojom::PrivateAggregationRequestPtr>
+ non_kanon_pa_requests_;
+
std::unique_ptr<base::RunLoop> generate_bid_run_loop_;
std::unique_ptr<base::RunLoop> report_win_run_loop_;
ReportWinCallback report_win_callback_;
diff --git a/content/services/auction_worklet/bidder_worklet.cc b/content/services/auction_worklet/bidder_worklet.cc
index e189377..dcb3852 100644
--- a/content/services/auction_worklet/bidder_worklet.cc
+++ b/content/services/auction_worklet/bidder_worklet.cc
@@ -42,6 +42,7 @@
#include "content/services/auction_worklet/for_debugging_only_bindings.h"
#include "content/services/auction_worklet/private_aggregation_bindings.h"
#include "content/services/auction_worklet/public/cpp/auction_network_events_delegate.h"
+#include "content/services/auction_worklet/public/cpp/private_aggregation_reporting.h"
#include "content/services/auction_worklet/public/mojom/auction_worklet_service.mojom.h"
#include "content/services/auction_worklet/public/mojom/bidder_worklet.mojom-shared.h"
#include "content/services/auction_worklet/public/mojom/bidder_worklet.mojom.h"
@@ -137,26 +138,6 @@
return subresource_bundle_result.GetSignals(v8_helper, context, errors);
}
-bool HasKAnonFailureComponent(
- const auction_worklet::mojom::PrivateAggregationRequestPtr& request) {
- if (request->contribution->is_histogram_contribution()) {
- return false;
- }
- const auction_worklet::mojom::AggregatableReportForEventContributionPtr&
- event_contribution = request->contribution->get_for_event_contribution();
- if (event_contribution->bucket->is_signal_bucket() &&
- event_contribution->bucket->get_signal_bucket()->base_value ==
- auction_worklet::mojom::BaseValue::kBidRejectReason) {
- return true;
- }
- if (event_contribution->value->is_signal_value() &&
- event_contribution->value->get_signal_value()->base_value ==
- auction_worklet::mojom::BaseValue::kBidRejectReason) {
- return true;
- }
- return false;
-}
-
std::optional<base::TimeDelta> NullOptIfZero(base::TimeDelta delta) {
if (delta.is_zero()) {
return std::nullopt;
@@ -1282,7 +1263,7 @@
std::erase_if(
non_kanon_pa_requests,
[](const auction_worklet::mojom::PrivateAggregationRequestPtr&
- request) { return !HasKAnonFailureComponent(request); });
+ request) { return !HasKAnonFailureComponent(*request); });
// We are enforcing the k-anonymity, so the restricted result is the one
// to use for reporting, etc., and needs to succeed.
diff --git a/content/services/auction_worklet/public/cpp/private_aggregation_reporting.cc b/content/services/auction_worklet/public/cpp/private_aggregation_reporting.cc
index f9ad9ff3..eb826145 100644
--- a/content/services/auction_worklet/public/cpp/private_aggregation_reporting.cc
+++ b/content/services/auction_worklet/public/cpp/private_aggregation_reporting.cc
@@ -90,4 +90,23 @@
return true;
}
+bool HasKAnonFailureComponent(const mojom::PrivateAggregationRequest& request) {
+ if (request.contribution->is_histogram_contribution()) {
+ return false;
+ }
+ const mojom::AggregatableReportForEventContributionPtr& event_contribution =
+ request.contribution->get_for_event_contribution();
+ if (event_contribution->bucket->is_signal_bucket() &&
+ event_contribution->bucket->get_signal_bucket()->base_value ==
+ mojom::BaseValue::kBidRejectReason) {
+ return true;
+ }
+ if (event_contribution->value->is_signal_value() &&
+ event_contribution->value->get_signal_value()->base_value ==
+ mojom::BaseValue::kBidRejectReason) {
+ return true;
+ }
+ return false;
+}
+
} // namespace auction_worklet
diff --git a/content/services/auction_worklet/public/cpp/private_aggregation_reporting.h b/content/services/auction_worklet/public/cpp/private_aggregation_reporting.h
index ac1c49b..497bb27d 100644
--- a/content/services/auction_worklet/public/cpp/private_aggregation_reporting.h
+++ b/content/services/auction_worklet/public/cpp/private_aggregation_reporting.h
@@ -28,6 +28,12 @@
const auction_worklet::mojom::PrivateAggregationRequest& request,
bool additional_extensions_allowed);
+// Returns true if `request` is asking to record reject-reason, and therefore
+// can be used to report kBelowKAnonThreshold for the bid that would have
+// won if not for k-anonymity.
+CONTENT_EXPORT bool HasKAnonFailureComponent(
+ const mojom::PrivateAggregationRequest& request);
+
} // namespace auction_worklet
#endif // CONTENT_SERVICES_AUCTION_WORKLET_PUBLIC_CPP_PRIVATE_AGGREGATION_REPORTING_H_
diff --git a/content/services/auction_worklet/public/cpp/private_aggregation_reporting_unittest.cc b/content/services/auction_worklet/public/cpp/private_aggregation_reporting_unittest.cc
index 8871e216..09eb6e0a 100644
--- a/content/services/auction_worklet/public/cpp/private_aggregation_reporting_unittest.cc
+++ b/content/services/auction_worklet/public/cpp/private_aggregation_reporting_unittest.cc
@@ -72,6 +72,56 @@
/*filtering_id=*/std::nullopt)),
blink::mojom::AggregationServiceMode::kDefault,
blink::mojom::DebugModeDetails::New());
+
+ // Using kWinningBid base_value for bucket and value.
+ const mojom::PrivateAggregationRequestPtr kWinningBid =
+ mojom::PrivateAggregationRequest::New(
+ mojom::AggregatableReportContribution::NewForEventContribution(
+ mojom::AggregatableReportForEventContribution::New(
+ mojom::ForEventSignalBucket::NewSignalBucket(
+ mojom::SignalBucket::New(mojom::BaseValue::kWinningBid,
+ 1.0,
+ mojom::BucketOffsetPtr())),
+ mojom::ForEventSignalValue::NewSignalValue(
+ mojom::SignalValue::New(mojom::BaseValue::kWinningBid,
+ 1.0,
+ 0)),
+ /*filtering_id=*/std::nullopt,
+ mojom::EventType::NewNonReserved("event_type"))),
+ blink::mojom::AggregationServiceMode::kDefault,
+ blink::mojom::DebugModeDetails::New());
+
+ // Using kRejectReason for value.
+ const mojom::PrivateAggregationRequestPtr kWithRejectReasonValue =
+ mojom::PrivateAggregationRequest::New(
+ mojom::AggregatableReportContribution::NewForEventContribution(
+ mojom::AggregatableReportForEventContribution::New(
+ mojom::ForEventSignalBucket::NewIdBucket(1),
+ mojom::ForEventSignalValue::NewSignalValue(
+ mojom::SignalValue::New(
+ mojom::BaseValue::kBidRejectReason,
+ 1.0,
+ 0)),
+ /*filtering_id=*/std::nullopt,
+ mojom::EventType::NewNonReserved("event_type"))),
+ blink::mojom::AggregationServiceMode::kDefault,
+ blink::mojom::DebugModeDetails::New());
+
+ // Using kRejectReason for bucket.
+ const mojom::PrivateAggregationRequestPtr kWithRejectReasonBucket =
+ mojom::PrivateAggregationRequest::New(
+ mojom::AggregatableReportContribution::NewForEventContribution(
+ mojom::AggregatableReportForEventContribution::New(
+ mojom::ForEventSignalBucket::NewSignalBucket(
+ mojom::SignalBucket::New(
+ mojom::BaseValue::kBidRejectReason,
+ 1.0,
+ mojom::BucketOffsetPtr())),
+ mojom::ForEventSignalValue::NewIntValue(2),
+ /*filtering_id=*/std::nullopt,
+ mojom::EventType::NewNonReserved("event_type"))),
+ blink::mojom::AggregationServiceMode::kDefault,
+ blink::mojom::DebugModeDetails::New());
};
TEST_F(PaReportingTest,
@@ -97,5 +147,14 @@
*kNonEvent, /*additional_extensions_allowed=*/false));
}
+TEST_F(PaReportingTest, HasKAnonFailureComponent) {
+ EXPECT_FALSE(HasKAnonFailureComponent(*kNonEvent));
+ EXPECT_FALSE(HasKAnonFailureComponent(*kNonReserved));
+ EXPECT_FALSE(HasKAnonFailureComponent(*kWithReservedAlways));
+ EXPECT_FALSE(HasKAnonFailureComponent(*kWinningBid));
+ EXPECT_TRUE(HasKAnonFailureComponent(*kWithRejectReasonValue));
+ EXPECT_TRUE(HasKAnonFailureComponent(*kWithRejectReasonBucket));
+}
+
} // namespace
} // namespace auction_worklet
