Skip to content

Commit

Permalink
Make TrustTokenStore origins type-safe
Browse files Browse the repository at this point in the history 
At a fundamental level, TrustTokenStore requires that all of its
url::Origin arguments have unique serializations, in order to key
persistent state by their serializations. It currently enforces this by
requiring that all of its operations' origin parameters be non-opaque
and either HTTP or HTTPS.

SuitableTrustTokenOrigin is a url::Origin wrapper encoding slightly
stricter preconditions (IsPotentiallyTrustworthy and HTTP or HTTPS) under
which an origin is suitable for use in a Trust Tokens operation.

This change modifies TrustTokenStore's interface to take
SuitableTrustTokenOrigins rather than url::Origins.

SuitableTrustTokenOrigin's properties are not exactly the same as what
TrustTokenStore needs to require in order to satisfy its contract (put
in state keyed to an origin, get out state corresponding to the same
origin), but all use of the store is expected to be of origins that
satisfy SuitableTrustTokenOrigin's requirements, so the difference
between the preconditions it enforces and TrustTokenStore's current
interface seems mostly academic.

This change makes the interface much simpler (not to mention safer) as
preconditions can now be enforced implicitly rather than through method
comments and DCHECKs.

R=csharrison

Bug: 1042962
Change-Id: I38db356efe0d85f76662d814ee973c0e2a5c9349
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134598
Commit-Queue: David Van Cleve <davidvc@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Auto-Submit: David Van Cleve <davidvc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757577}
  • Loading branch information
David Van Cleve authored and Commit Bot committed Apr 8, 2020
1 parent 507e933 commit 292fe40
Show file tree
Hide file tree
Showing 9 changed files with 231 additions and 198 deletions.
8 changes: 5 additions & 3 deletions services/network/network_context_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6451,7 +6451,8 @@ TEST_F(NetworkContextTestWithMockTime, EnableTrustTokensWithStoreOnDisk) {
network_context->trust_token_store()->ExecuteOrEnqueue(
base::BindLambdaForTesting([&](TrustTokenStore* store) {
DCHECK(store);
store->AddTokens(url::Origin::Create(GURL("https://trusttoken.com/")),
store->AddTokens(*SuitableTrustTokenOrigin::Create(
GURL("https://trusttoken.com/")),
std::vector<std::string>{"token"}, "issuing key");
run_loop.Quit();
}));
Expand All @@ -6478,8 +6479,9 @@ TEST_F(NetworkContextTestWithMockTime, EnableTrustTokensWithStoreOnDisk) {
base::BindLambdaForTesting(
[&obtained_num_tokens, &run_loop](TrustTokenStore* store) {
DCHECK(store);
obtained_num_tokens = store->CountTokens(
url::Origin::Create(GURL("https://trusttoken.com/")));
obtained_num_tokens =
store->CountTokens(*SuitableTrustTokenOrigin::Create(
GURL("https://trusttoken.com/")));
run_loop.Quit();
}));

Expand Down
5 changes: 2 additions & 3 deletions services/network/trust_tokens/has_trust_tokens_answerer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,14 @@ void HasTrustTokensAnswerer::AnswerQueryWithStore(
TrustTokenStore* trust_token_store) {
DCHECK(trust_token_store);

if (!trust_token_store->SetAssociation(issuer.origin(),
top_frame_origin_.origin())) {
if (!trust_token_store->SetAssociation(issuer, top_frame_origin_)) {
std::move(callback).Run(mojom::HasTrustTokensResult::New(
mojom::TrustTokenOperationStatus::kResourceExhausted,
/*has_trust_tokens=*/false));
return;
}

bool has_trust_tokens = trust_token_store->CountTokens(issuer.origin());
bool has_trust_tokens = trust_token_store->CountTokens(issuer);
std::move(callback).Run(mojom::HasTrustTokensResult::New(
mojom::TrustTokenOperationStatus::kOk, has_trust_tokens));
}
Expand Down
12 changes: 7 additions & 5 deletions services/network/trust_tokens/has_trust_tokens_answerer_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ TEST(HasTrustTokensAnswerer, HandlesFailureToAssociateIssuer) {
// writing, 2).
for (int i = 0; i < kTrustTokenPerToplevelMaxNumberOfAssociatedIssuers; ++i) {
ASSERT_TRUE(store->SetAssociation(
url::Origin::Create(
*SuitableTrustTokenOrigin::Create(
GURL(base::StringPrintf("https://issuer%d.com", i))),
kToplevel.origin()));
kToplevel));
}

PendingTrustTokenStore pending_store;
Expand Down Expand Up @@ -97,7 +97,8 @@ TEST(HasTrustTokensAnswerer, SuccessWithNoTokens) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();
TrustTokenStore* raw_store = store.get();

const url::Origin kIssuer = url::Origin::Create(GURL("https://issuer.com"));
const SuitableTrustTokenOrigin kIssuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com"));
const SuitableTrustTokenOrigin kToplevel =
*SuitableTrustTokenOrigin::Create(GURL("https://toplevel.com"));

Expand All @@ -122,14 +123,15 @@ TEST(HasTrustTokensAnswerer, SuccessWithNoTokens) {
EXPECT_FALSE(result->has_trust_tokens);

// The query should have associated the issuer with the top-level origin.
EXPECT_TRUE(raw_store->IsAssociated(kIssuer, kToplevel.origin()));
EXPECT_TRUE(raw_store->IsAssociated(kIssuer, kToplevel));
}

TEST(HasTrustTokensAnswerer, SuccessWithTokens) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();
TrustTokenStore* raw_store = store.get();

const url::Origin kIssuer = url::Origin::Create(GURL("https://issuer.com"));
const SuitableTrustTokenOrigin kIssuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com"));
const SuitableTrustTokenOrigin kToplevel =
*SuitableTrustTokenOrigin::Create(GURL("https://toplevel.com"));

Expand Down
39 changes: 25 additions & 14 deletions services/network/trust_tokens/trust_token_request_issuance_helper_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class MockCryptographer
TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfTooManyIssuers) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

auto issuer = url::Origin::Create(GURL("https://issuer.com/"));
auto issuer = *SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));
auto toplevel =
*SuitableTrustTokenOrigin::Create(GURL("https://toplevel.com/"));

Expand All @@ -91,7 +91,7 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfTooManyIssuers) {
// requirements of the Trust Tokens protocol.)
for (int i = 0; i < kTrustTokenPerToplevelMaxNumberOfAssociatedIssuers; ++i) {
ASSERT_TRUE(store->SetAssociation(
url::Origin::Create(
*SuitableTrustTokenOrigin::Create(
GURL(base::StringPrintf("https://issuer%d.com/", i))),
toplevel));
}
Expand All @@ -111,7 +111,7 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfTooManyIssuers) {
TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfAtCapacity) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

auto issuer = url::Origin::Create(GURL("https://issuer.com/"));
auto issuer = *SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

// Fill up the store with tokens; issuance should fail the tokens for |issuer|
// are at capacity.
Expand All @@ -134,7 +134,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfAtCapacity) {
TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfKeyCommitmentFails) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

// Have the key commitment getter return nullptr, denoting that the key
// commitment fetch failed.
Expand All @@ -144,7 +145,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfKeyCommitmentFails) {
std::make_unique<MockCryptographer>());

auto request = MakeURLRequest("https://issuer.com/");
request->set_initiator(url::Origin::Create(GURL("https://issuer.com/")));
request->set_initiator(
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/")));

EXPECT_EQ(ExecuteBeginOperationAndWaitForResult(&helper, request.get()),
mojom::TrustTokenOperationStatus::kFailedPrecondition);
Expand All @@ -154,7 +156,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfKeyCommitmentFails) {
TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfAddingKeyFails) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

auto key_commitment_result = mojom::TrustTokenKeyCommitmentResult::New();
key_commitment_result->keys.push_back(
Expand All @@ -181,7 +184,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest,
RejectsIfGettingBlindedTokensFails) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

auto key_commitment_result = mojom::TrustTokenKeyCommitmentResult::New();
key_commitment_result->keys.push_back(
Expand Down Expand Up @@ -214,7 +218,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest,
TEST_F(TrustTokenRequestIssuanceHelperTest, SetsRequestHeader) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

auto key_commitment_result = mojom::TrustTokenKeyCommitmentResult::New();
key_commitment_result->keys.push_back(
Expand Down Expand Up @@ -251,7 +256,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, SetsRequestHeader) {
TEST_F(TrustTokenRequestIssuanceHelperTest, SetsLoadFlag) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

auto key_commitment_result = mojom::TrustTokenKeyCommitmentResult::New();
key_commitment_result->keys.push_back(
Expand Down Expand Up @@ -284,7 +290,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, SetsLoadFlag) {
TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfResponseOmitsHeader) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

auto key_commitment_result = mojom::TrustTokenKeyCommitmentResult::New();
key_commitment_result->keys.push_back(
Expand Down Expand Up @@ -320,7 +327,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfResponseOmitsHeader) {
TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfResponseIsUnusable) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

auto key_commitment_result = mojom::TrustTokenKeyCommitmentResult::New();
key_commitment_result->keys.push_back(
Expand Down Expand Up @@ -367,7 +375,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, RejectsIfResponseIsUnusable) {
TEST_F(TrustTokenRequestIssuanceHelperTest, Success) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

auto key_commitment_result = mojom::TrustTokenKeyCommitmentResult::New();
key_commitment_result->keys.push_back(
Expand Down Expand Up @@ -413,7 +422,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, Success) {
TEST_F(TrustTokenRequestIssuanceHelperTest, AssociatesIssuerWithToplevel) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

auto key_commitment_result = mojom::TrustTokenKeyCommitmentResult::New();
key_commitment_result->keys.push_back(
Expand Down Expand Up @@ -448,7 +458,8 @@ TEST_F(TrustTokenRequestIssuanceHelperTest, AssociatesIssuerWithToplevel) {
TEST_F(TrustTokenRequestIssuanceHelperTest, StoresObtainedTokens) {
std::unique_ptr<TrustTokenStore> store = TrustTokenStore::CreateInMemory();

url::Origin issuer = url::Origin::Create(GURL("https://issuer.com/"));
SuitableTrustTokenOrigin issuer =
*SuitableTrustTokenOrigin::Create(GURL("https://issuer.com/"));

auto key_commitment_result = mojom::TrustTokenKeyCommitmentResult::New();
key_commitment_result->keys.push_back(mojom::TrustTokenVerificationKey::New(
Expand Down
2 changes: 1 addition & 1 deletion services/network/trust_tokens/trust_token_request_redemption_helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class TrustTokenRequestRedemptionHelper : public TrustTokenRequestHelper {
// but, once initialized, it will never be empty over the course of the
// operation's execution.
base::Optional<SuitableTrustTokenOrigin> issuer_;
const url::Origin top_level_origin_;
const SuitableTrustTokenOrigin top_level_origin_;
const mojom::TrustTokenRefreshPolicy refresh_policy_;

// |signing_key_| and |verification_key_| are generated speculatively near the
Expand Down
Loading

0 comments on commit 292fe40

Please sign in to comment.