Commit f0222a0d authored by David Van Cleve's avatar David Van Cleve Committed by Commit Bot

Trust Tokens: Clear data more selectively

A previous CL (crrev.com/c/2120102) implemented preliminary data
clearing logic for Trust Tokens [*] persistent state, in a manner such
that any data removal action clearing cookies would also clear Trust
Tokens state.

In particular, this clears Trust Tokens state whenever the browser
receives a Clear-Site-Data header specifying that cookies should be
cleared. This is a problem because persistent Trust Tokens state
includes metadata used to enforce privacy invariants.  For instance, it
contains lists of token issuers associated with each top-level origin
and the statuses of origin-scoped rate limits on issuance and
redemption.

This CL creates a new data removal type for Trust Tokens and adds it to
DATA_TYPE_SITE_DATA, with the effect that Trust Tokens state will be
cleared through the UI when the user clears cookies and other site data,
but the state will not be cleared through the Clear-Site-Data header.

[*]
https://docs.google.com/document/u/1/d/1TNnya6B8pyomDK2F1R9CL3dY10OAmqWlnCxsWyOBDVQ/edit

R=msramek

Bug: 1064747
Change-Id: I9c5fbd0ad660199345ab04a89999731d79fded16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144411
Commit-Queue: David Van Cleve <davidvc@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarChristian Dullweber <dullweber@chromium.org>
Reviewed-by: default avatarMartin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759216}
parent 855f80a1
......@@ -92,7 +92,8 @@ class ChromeBrowsingDataRemoverDelegate
DATA_TYPE_WEB_APP_DATA |
#endif
DATA_TYPE_SITE_USAGE_DATA | DATA_TYPE_DURABLE_PERMISSION |
DATA_TYPE_EXTERNAL_PROTOCOL_DATA | DATA_TYPE_ISOLATED_ORIGINS,
DATA_TYPE_EXTERNAL_PROTOCOL_DATA | DATA_TYPE_ISOLATED_ORIGINS |
content::BrowsingDataRemover::DATA_TYPE_TRUST_TOKENS,
// Datatypes protected by Important Sites.
IMPORTANT_SITES_DATA_TYPES =
......
......@@ -112,6 +112,7 @@
#include "net/url_request/url_request_test_util.h"
#include "services/network/network_context.h"
#include "services/network/network_service.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/mojom/cookie_manager.mojom.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/favicon_size.h"
......
......@@ -475,7 +475,7 @@ void BrowsingDataRemoverImpl::RemoveImpl(
// network error logging below, a data removal request for certain
// sites/origins that has the Trust Tokens type in scope will clear all Trust
// Tokens data associated with the requested sites/origins.
if (remove_mask & DATA_TYPE_COOKIES) {
if (remove_mask & DATA_TYPE_TRUST_TOKENS) {
network::mojom::NetworkContext* network_context =
storage_partition->GetNetworkContext();
network_context->ClearTrustTokenData(
......
......@@ -9,6 +9,7 @@
#include "base/bind.h"
#include "base/files/file_path.h"
#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browsing_data_filter_builder.h"
#include "content/public/browser/browsing_data_remover.h"
......@@ -29,6 +30,7 @@
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "services/network/public/mojom/network_service.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -215,6 +217,12 @@ class BrowsingDataRemoverImplBrowserTest : public ContentBrowserTest {
.get();
}
network::mojom::NetworkContext* network_context() {
return BrowserContext::GetDefaultStoragePartition(
shell()->web_contents()->GetBrowserContext())
->GetNetworkContext();
}
private:
net::test_server::EmbeddedTestServer ssl_server_;
};
......@@ -271,4 +279,193 @@ IN_PROC_BROWSER_TEST_F(BrowsingDataRemoverImplBrowserTest,
EXPECT_FALSE(IsHttpAuthCacheSet());
}
namespace {
// Provide BrowsingDataRemoverImplTrustTokenTest the Trust Tokens
// feature as a mixin so that it gets set before the superclass initializes the
// test's NetworkContext, as the NetworkContext's initialization must occur with
// the feature enabled.
class WithTrustTokensEnabled {
public:
WithTrustTokensEnabled() {
feature_list_.InitAndEnableFeature(network::features::kTrustTokens);
}
private:
base::test::ScopedFeatureList feature_list_;
};
// Tests Trust Tokens clearing by calling HasTrustTokensAnswerer::HasTrustTokens
// with a HasTrustTokensAnswerer obtained from the provided NetworkContext.
//
// The Trust Tokens functionality places a cap of 2 distinct arguments to the
// |issuer| argument of
// HasTrustTokensAnswerer(origin)::HasTrustTokens(issuer)
// for each top-frame origin |origin|. (This limit is recorded in persistent
// storage scoped to the origin |origin| and is not related to the lifetime of
// the specific HasTrustTokensAnswerer object.)
//
// To add an origin, the tester creates a HasTrustTokensAnswerer parameterized
// by |origin| and calls HasTrustTokens with two distinct "priming" issuer
// arguments. This will make the Trust Tokens persistent storage record that
// |origin| is associated with each of these issuers, with the effect that
// (barring a data clear) subsequent HasTrustTokens calls with different issuer
// arguments will fail. To check if an origin is present, the tester calls
// HasTrustTokensAnswerer(origin)::HasTrustTokens(issuer)
// with an |issuer| argument distinct from the two earlier "priming" issuers.
// This third HasTrustTokens call will error out exactly if |origin| was
// previously added by AddOrigin.
//
// Usage:
// >= 0 AddOrigin() - origins must be HTTPS
// (clear data)
// >= 0 HasOrigin()
class TrustTokensTester {
public:
explicit TrustTokensTester(network::mojom::NetworkContext* network_context)
: network_context_(network_context) {}
void AddOrigin(const url::Origin& origin) {
mojo::Remote<network::mojom::HasTrustTokensAnswerer> answerer;
network_context_->GetHasTrustTokensAnswerer(
answerer.BindNewPipeAndPassReceiver(), origin);
// Calling HasTrustTokens will associate the issuer argument with the
// origin |origin|.
//
// Do this until the |origin| is associated with
// network::kTrustTokenPerToplevelMaxNumberOfAssociatedIssuers many issuers
// (namely 2; this value is not expected to change frequently).
//
// After the limit is reached, subsequent HasTrustToken(origin, issuer)
// queries will fail for any issuers not in {https://prime0.example,
// https://prime1.example} --- unless data for |origin| is cleared.
for (int i = 0; i < 2; ++i) {
base::RunLoop run_loop;
answerer->HasTrustTokens(
url::Origin::Create(
GURL(base::StringPrintf("https://prime%d.example", i))),
base::BindLambdaForTesting(
[&](network::mojom::HasTrustTokensResultPtr) {
run_loop.Quit();
}));
run_loop.Run();
}
}
bool HasOrigin(const url::Origin& origin) {
mojo::Remote<network::mojom::HasTrustTokensAnswerer> answerer;
network_context_->GetHasTrustTokensAnswerer(
answerer.BindNewPipeAndPassReceiver(), origin);
base::RunLoop run_loop;
bool has_origin = false;
// Since https://probe.example is not among the issuer origins previously
// provided to HasTrustTokens(origin, _) calls in AddOrigin:
// - If data has not been cleared,
// HasTrustToken(origin, https://probe.example)
// is expected to fail with kResourceExhausted because |origin| is at its
// number-of-associated-issuers limit, so the answerer will refuse to
// answer a query for an origin it has not yet seen.
// - If data has been cleared, the answerer should be able to fulfill the
// query.
answerer->HasTrustTokens(
url::Origin::Create(GURL("https://probe.example")),
base::BindLambdaForTesting([&](network::mojom::HasTrustTokensResultPtr
result) {
// HasTrustTokens will error out with kResourceExhausted exactly
// when the top-frame origin |origin| was previously added by
// AddOrigin.
if (result->status ==
network::mojom::TrustTokenOperationStatus::kResourceExhausted) {
has_origin = true;
}
run_loop.Quit();
}));
run_loop.Run();
return has_origin;
}
private:
network::mojom::NetworkContext* network_context_;
};
} // namespace
class BrowsingDataRemoverImplTrustTokenTest
: public WithTrustTokensEnabled,
public BrowsingDataRemoverImplBrowserTest {};
IN_PROC_BROWSER_TEST_F(BrowsingDataRemoverImplTrustTokenTest, Remove) {
TrustTokensTester tester(network_context());
auto origin = url::Origin::Create(GURL("https://topframe.example"));
tester.AddOrigin(origin);
ASSERT_TRUE(tester.HasOrigin(origin));
RemoveAndWait(BrowsingDataRemover::DATA_TYPE_TRUST_TOKENS);
EXPECT_FALSE(tester.HasOrigin(origin));
}
IN_PROC_BROWSER_TEST_F(BrowsingDataRemoverImplTrustTokenTest, RemoveByDomain) {
TrustTokensTester tester(network_context());
auto origin = url::Origin::Create(GURL("https://topframe.example"));
auto sub_origin = url::Origin::Create(GURL("https://sub.topframe.example"));
auto another_origin =
url::Origin::Create(GURL("https://another-topframe.example"));
tester.AddOrigin(origin);
tester.AddOrigin(sub_origin);
tester.AddOrigin(another_origin);
ASSERT_TRUE(tester.HasOrigin(origin));
ASSERT_TRUE(tester.HasOrigin(sub_origin));
ASSERT_TRUE(tester.HasOrigin(another_origin));
std::unique_ptr<BrowsingDataFilterBuilder> builder(
BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::WHITELIST));
builder->AddRegisterableDomain("topframe.example");
RemoveWithFilterAndWait(BrowsingDataRemover::DATA_TYPE_TRUST_TOKENS,
std::move(builder));
EXPECT_FALSE(tester.HasOrigin(origin));
EXPECT_FALSE(tester.HasOrigin(sub_origin));
EXPECT_TRUE(tester.HasOrigin(another_origin));
}
IN_PROC_BROWSER_TEST_F(BrowsingDataRemoverImplTrustTokenTest,
PreserveByDomain) {
TrustTokensTester tester(network_context());
auto origin = url::Origin::Create(GURL("https://topframe.example"));
auto sub_origin = url::Origin::Create(GURL("https://sub.topframe.example"));
auto another_origin =
url::Origin::Create(GURL("https://another-topframe.example"));
tester.AddOrigin(origin);
tester.AddOrigin(sub_origin);
tester.AddOrigin(another_origin);
ASSERT_TRUE(tester.HasOrigin(origin));
ASSERT_TRUE(tester.HasOrigin(sub_origin));
ASSERT_TRUE(tester.HasOrigin(another_origin));
// Delete all data *except* that specified by the filter.
std::unique_ptr<BrowsingDataFilterBuilder> builder(
BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::BLACKLIST));
builder->AddRegisterableDomain("topframe.example");
RemoveWithFilterAndWait(BrowsingDataRemover::DATA_TYPE_TRUST_TOKENS,
std::move(builder));
EXPECT_TRUE(tester.HasOrigin(origin));
EXPECT_TRUE(tester.HasOrigin(sub_origin));
EXPECT_FALSE(tester.HasOrigin(another_origin));
}
} // namespace content
......@@ -1518,9 +1518,8 @@ TEST_F(BrowsingDataRemoverImplTest, ClearsTrustTokens) {
EXPECT_CALL(context, ClearTrustTokenData(_, _)).WillOnce(RunOnceClosure<1>());
// Trust Tokens storage is cleared whenever cookies are cleared.
BlockUntilBrowsingDataRemoved(base::Time(), base::Time::Max(),
BrowsingDataRemover::DATA_TYPE_COOKIES,
BrowsingDataRemover::DATA_TYPE_TRUST_TOKENS,
/*include_protected_origins=*/false);
}
......@@ -1528,10 +1527,10 @@ TEST_F(BrowsingDataRemoverImplTest, PreservesTrustTokens) {
StrictMock<MockNetworkContext> context;
set_network_context_override(&context);
// When cookies aren't cleared, Trust Tokens state shouldn't be.
// When DATA_TYPE_TRUST_TOKENS isn't cleared, Trust Tokens state shouldn't be.
BlockUntilBrowsingDataRemoved(
base::Time(), base::Time::Max(),
BrowsingDataRemover::DATA_TYPE_CACHE, // arbitrary non-cookie type
BrowsingDataRemover::DATA_TYPE_CACHE, // arbitrary non-Trust Tokens type
/*include_protected_origins=*/false);
// (The strict mock will fail the test if its mocked method is called.)
......@@ -1557,10 +1556,8 @@ TEST_F(BrowsingDataRemoverImplTest, ClearsTrustTokensForSite) {
BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::WHITELIST));
builder->AddRegisterableDomain("host1.com");
// Trust Tokens storage is cleared whenever cookies are cleared: when clearing
// cookies for a site, we should clear Trust Tokens state for the site.
BlockUntilOriginDataRemoved(base::Time(), base::Time::Max(),
BrowsingDataRemover::DATA_TYPE_COOKIES,
BrowsingDataRemover::DATA_TYPE_TRUST_TOKENS,
std::move(builder));
}
......@@ -1584,15 +1581,12 @@ TEST_F(BrowsingDataRemoverImplTest, ClearsTrustTokensForSiteDespiteTimeRange) {
BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::WHITELIST));
builder->AddRegisterableDomain("host1.com");
// Trust Tokens storage is cleared whenever cookies are cleared: when clearing
// cookies for a site, we should clear Trust Tokens state for the site.
//
// Since Trust Tokens data is not associated with particular timestamps, we
// should observe the same clearing behavior with a non-default time range as
// with the default time range.
BlockUntilOriginDataRemoved(
base::Time(), base::Time() + base::TimeDelta::FromSeconds(1),
BrowsingDataRemover::DATA_TYPE_COOKIES, std::move(builder));
BrowsingDataRemover::DATA_TYPE_TRUST_TOKENS, std::move(builder));
}
} // namespace content
......@@ -103,8 +103,12 @@ class BrowsingDataRemover {
// TODO(crbug.com/798760): Remove when fixed.
DATA_TYPE_AVOID_CLOSING_CONNECTIONS = 1 << 15,
// Trust Token API (https://github.com/wicg/trust-token-api) persistent
// storage.
DATA_TYPE_TRUST_TOKENS = 1 << 16,
// Embedders can add more datatypes beyond this point.
DATA_TYPE_CONTENT_END = DATA_TYPE_AVOID_CLOSING_CONNECTIONS,
DATA_TYPE_CONTENT_END = DATA_TYPE_TRUST_TOKENS,
};
enum OriginType {
......
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