Skip to content

Commit

Permalink
K-anonymity clean-up.
Browse files Browse the repository at this point in the history 
- Remove unused interest group name and update URL k-anonymity fields.
- Remove bidding_wasm_helper URL from Ad k-anonymity keys.
- Move interest group name to the end of the Ad Reporting k-anonymity
  key.

Bug: 1234419
Change-Id: I004cae5f5e806b268b62a5e527d52ed7fe55b9de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4044582
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Russ Hamilton <behamilton@google.com>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1076340}
  • Loading branch information
@brusshamilton
brusshamilton authored and Chromium LUCI CQ committed Nov 28, 2022
1 parent a53df31 commit 84df761
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 370 deletions.
32 changes: 2 additions & 30 deletions content/browser/interest_group/interest_group_k_anonymity_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@

namespace content {

std::string KAnonKeyFor(const url::Origin& owner, const std::string& name) {
return owner.GetURL().spec() + '\n' + name;
}

std::string KAnonKeyForAdBid(const blink::InterestGroup& group,
const blink::InterestGroup::Ad& ad) {
DCHECK(group.ads);
Expand All @@ -23,7 +19,6 @@ std::string KAnonKeyForAdBid(const blink::InterestGroup& group,
DCHECK(group.bidding_url);
return group.owner.GetURL().spec() + '\n' +
group.bidding_url.value_or(GURL()).spec() + '\n' +
group.bidding_wasm_helper_url.value_or(GURL()).spec() + '\n' +
ad.render_url.spec();
}

Expand All @@ -32,10 +27,9 @@ std::string KAnonKeyForAdNameReporting(const blink::InterestGroup& group,
DCHECK(group.ads);
DCHECK(base::Contains(*group.ads, ad));
DCHECK(group.bidding_url);
return group.owner.GetURL().spec() + '\n' + group.name + '\n' +
return group.owner.GetURL().spec() + '\n' +
group.bidding_url.value_or(GURL()).spec() + '\n' +
group.bidding_wasm_helper_url.value_or(GURL()).spec() + '\n' +
ad.render_url.spec();
ad.render_url.spec() + '\n' + group.name;
}

InterestGroupKAnonymityManager::InterestGroupKAnonymityManager(
Expand All @@ -56,21 +50,6 @@ void InterestGroupKAnonymityManager::QueryKAnonymityForInterestGroup(
base::Time check_time = base::Time::Now();
base::TimeDelta min_wait = k_anonymity_service_->GetQueryInterval();

if (!storage_group.name_kanon ||
storage_group.name_kanon->last_updated < check_time - min_wait) {
ids_to_query.push_back(KAnonKeyFor(storage_group.interest_group.owner,
storage_group.interest_group.name));
}

if (storage_group.interest_group.daily_update_url) {
if (!storage_group.daily_update_url_kanon ||
storage_group.daily_update_url_kanon->last_updated <
check_time - min_wait) {
ids_to_query.push_back(
storage_group.interest_group.daily_update_url->spec());
}
}

for (const auto& ad : storage_group.bidding_ads_kanon) {
if (ad.last_updated < check_time - min_wait) {
ids_to_query.push_back(ad.key);
Expand Down Expand Up @@ -112,13 +91,6 @@ void InterestGroupKAnonymityManager::QuerySetsCallback(
}
}

void InterestGroupKAnonymityManager::RegisterInterestGroupAsJoined(
const blink::InterestGroup& group) {
RegisterIDAsJoined(KAnonKeyFor(group.owner, group.name));
if (group.daily_update_url)
RegisterIDAsJoined(group.daily_update_url->spec());
}

void InterestGroupKAnonymityManager::RegisterAdAsWon(
const blink::InterestGroup& group,
const blink::InterestGroup::Ad& ad) {
Expand Down
5 changes: 0 additions & 5 deletions content/browser/interest_group/interest_group_k_anonymity_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,6 @@ class InterestGroupKAnonymityManager {
void QueryKAnonymityForInterestGroup(
const StorageInterestGroup& storage_group);

// Notify the k-anonymity service that we are joining this interest group.
// Internally this calls RegisterIDAsJoined() for interest group name and
// update URL.
void RegisterInterestGroupAsJoined(const blink::InterestGroup& group);

// Notify the k-anonymity service that this ad won an auction. Internally this
// calls RegisterIDAsJoined().
void RegisterAdAsWon(const blink::InterestGroup& group,
Expand Down
78 changes: 15 additions & 63 deletions content/browser/interest_group/interest_group_k_anonymity_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,14 @@ TEST_F(InterestGroupKAnonymityManagerTest,
manager->JoinInterestGroup(MakeInterestGroup(owner, "foo"), top_frame);
auto maybe_group = getGroup(manager.get(), owner, name);
ASSERT_TRUE(maybe_group);
EXPECT_EQ(base::Time::Min(), maybe_group->name_kanon->last_updated);
EXPECT_EQ(base::Time::Min(), maybe_group->bidding_ads_kanon[0].last_updated);

// k-anonymity update happens here.
task_environment().FastForwardBy(base::Minutes(1));

maybe_group = getGroup(manager.get(), owner, name);
ASSERT_TRUE(maybe_group);
base::Time last_updated = maybe_group->name_kanon->last_updated;
base::Time last_updated = maybe_group->bidding_ads_kanon[0].last_updated;
EXPECT_LE(before_join, last_updated);
EXPECT_GT(base::Time::Now(), last_updated);

Expand All @@ -167,7 +167,7 @@ TEST_F(InterestGroupKAnonymityManagerTest,

maybe_group = getGroup(manager.get(), owner, name);
ASSERT_TRUE(maybe_group);
EXPECT_EQ(last_updated, maybe_group->name_kanon->last_updated);
EXPECT_EQ(last_updated, maybe_group->bidding_ads_kanon[0].last_updated);

task_environment().FastForwardBy(kQueryInterval);

Expand All @@ -176,59 +176,7 @@ TEST_F(InterestGroupKAnonymityManagerTest,
task_environment().RunUntilIdle();
maybe_group = getGroup(manager.get(), owner, name);
ASSERT_TRUE(maybe_group);
EXPECT_LT(last_updated, maybe_group->name_kanon->last_updated);
}

TEST_F(InterestGroupKAnonymityManagerTest, QueueUpdatePerformsJoinSetForGroup) {
const GURL top_frame = GURL("https://www.example.com/foo");
const url::Origin owner = url::Origin::Create(top_frame);
const std::string name = "foo";

std::string group_name_url = "https://www.example.com/\nfoo";
std::string group_update_url = kUpdateURL;

auto manager = CreateManager();
EXPECT_EQ(base::Time::Min(), GetLastReported(manager.get(), group_name_url));
EXPECT_FALSE(getGroup(manager.get(), owner, name));
base::Time before_join = base::Time::Now();

// JoinInterestGroup should call QueueKAnonymityUpdateForInterestGroup.
manager->JoinInterestGroup(MakeInterestGroup(owner, "foo"), top_frame);

// k-anonymity update happens here.
task_environment().FastForwardBy(base::Minutes(1));

EXPECT_TRUE(getGroup(manager.get(), owner, name));

absl::optional<base::Time> group_name_reported =
GetLastReported(manager.get(), group_name_url);
ASSERT_TRUE(group_name_reported);
EXPECT_LE(before_join, group_name_reported);

absl::optional<base::Time> update_url_reported =
GetLastReported(manager.get(), kUpdateURL);
ASSERT_TRUE(update_url_reported);
EXPECT_LE(before_join, update_url_reported);

auto maybe_group = getGroup(manager.get(), owner, name);
ASSERT_TRUE(maybe_group);

manager->QueueKAnonymityUpdateForInterestGroup(*maybe_group);

// k-anonymity update would happen here.
task_environment().FastForwardBy(base::Minutes(1));

// Second update shouldn't change anything.
EXPECT_EQ(group_name_reported,
GetLastReported(manager.get(), group_name_url));
EXPECT_EQ(update_url_reported, GetLastReported(manager.get(), kUpdateURL));

task_environment().FastForwardBy(kJoinInterval);

// Updated more than GetJoinInterval() ago, so update.
manager->QueueKAnonymityUpdateForInterestGroup(*maybe_group);
task_environment().RunUntilIdle();
EXPECT_LT(update_url_reported, GetLastReported(manager.get(), kUpdateURL));
EXPECT_LT(last_updated, maybe_group->bidding_ads_kanon[0].last_updated);
}

TEST_F(InterestGroupKAnonymityManagerTest, RegisterAdAsWonPerformsJoinSet) {
Expand Down Expand Up @@ -290,9 +238,12 @@ TEST_F(InterestGroupKAnonymityManagerTest, HandlesServerErrors) {
base::Time start_time = base::Time::Now();

auto manager = CreateManager(/*has_error=*/true);
manager->JoinInterestGroup(MakeInterestGroup(owner, "foo"), top_frame);
blink::InterestGroup g = MakeInterestGroup(owner, "foo");

manager->JoinInterestGroup(g, top_frame);
// The group *must* exist when JoinInterestGroup returns.
ASSERT_TRUE(getGroup(manager.get(), owner, name));
manager->RegisterAdAsWon(g, g.ads.value()[0]);

// k-anonymity update happens here.
task_environment().FastForwardBy(base::Minutes(1));
Expand All @@ -304,20 +255,21 @@ TEST_F(InterestGroupKAnonymityManagerTest, HandlesServerErrors) {
// When the server is actually implemented we'll need to change the expected
// values below.

absl::optional<base::Time> group_name_reported =
GetLastReported(manager.get(), kUpdateURL);
ASSERT_TRUE(group_name_reported);
absl::optional<base::Time> ad_reported =
GetLastAdReported(manager.get(), g, g.ads.value()[0]);
ASSERT_TRUE(ad_reported);

// TODO(behamilton): Change this once we expect the server to be stable.
EXPECT_LE(start_time, group_name_reported);
EXPECT_LE(start_time, ad_reported);
// EXPECT_EQ(base::Time::Min(), group_name_reported);

auto maybe_group = getGroup(manager.get(), owner, name);
ASSERT_TRUE(maybe_group);

// TODO(behamilton): Change this once we expect the server to be stable.
EXPECT_LE(start_time, maybe_group->name_kanon->last_updated);
// EXPECT_EQ(base::Time::Min(), maybe_group->name_kanon->last_updated);
EXPECT_LE(start_time, maybe_group->bidding_ads_kanon[0].last_updated);
// EXPECT_EQ(base::Time::Min(),
// maybe_group->bidding_ads_kanon[0].last_updated);
}

class MockAnonymityServiceDelegate : public KAnonymityServiceDelegate {
Expand Down
1 change: 0 additions & 1 deletion content/browser/interest_group/interest_group_manager_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,6 @@ void InterestGroupManagerImpl::ClearPermissionsCache() {
void InterestGroupManagerImpl::QueueKAnonymityUpdateForInterestGroup(
const StorageInterestGroup& group) {
k_anonymity_manager_->QueryKAnonymityForInterestGroup(group);
k_anonymity_manager_->RegisterInterestGroupAsJoined(group.interest_group);
}

void InterestGroupManagerImpl::UpdateKAnonymity(
Expand Down
35 changes: 0 additions & 35 deletions content/browser/interest_group/interest_group_storage.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1394,21 +1394,6 @@ bool DoGetKAnonymity(
return interest_group_kanon.Succeeded();
}

bool DoGetInterestGroupNameKAnonymity(
sql::Database& db,
const url::Origin& owner,
const std::string& name,
absl::optional<StorageInterestGroup::KAnonymityData>& output) {
return DoGetKAnonymity(db, KAnonKeyFor(owner, name), output);
}

bool DoGetURLKAnonymity(
sql::Database& db,
const GURL& url,
absl::optional<StorageInterestGroup::KAnonymityData>& output) {
return DoGetKAnonymity(db, url.spec(), output);
}

bool GetPreviousWins(sql::Database& db,
const blink::InterestGroupKey& group_key,
base::Time win_time_after,
Expand Down Expand Up @@ -1536,26 +1521,6 @@ absl::optional<StorageInterestGroup> DoGetStoredInterestGroup(
return absl::nullopt;
}

if (!DoGetInterestGroupNameKAnonymity(db, group_key.owner, group_key.name,
db_interest_group.name_kanon)) {
// This should only happen if the database was created with an older version
// of the k-anon key for interest group names. Try the old group name.
// TODO(behamilton): Remove this in a new version
if (!DoGetKAnonymity(db,
group_key.owner.GetURL()
.Resolve(base::EscapePath(group_key.name))
.spec(),
db_interest_group.name_kanon))
return absl::nullopt;
db_interest_group.name_kanon->key =
KAnonKeyFor(group_key.owner, group_key.name);
}
if (db_interest_group.interest_group.daily_update_url &&
!DoGetURLKAnonymity(
db, db_interest_group.interest_group.daily_update_url.value(),
db_interest_group.daily_update_url_kanon)) {
return absl::nullopt;
}
if (db_interest_group.interest_group.bidding_url) {
if (db_interest_group.interest_group.ads) {
for (auto& ad : db_interest_group.interest_group.ads.value()) {
Expand Down

0 comments on commit 84df761

Please sign in to comment.