net/cert/internal/path_builder_unittest.cc - Issue 2832703002: Allow the TrustStore interface to return matching intermediates, and identify distrusted certs.

Unified Diff: net/cert/internal/path_builder_unittest.cc

Issue 2832703002: Allow the TrustStore interface to return matching intermediates, and identify distrusted certs. (Closed)

Patch Set: address comments Created 3 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« components/cast_certificate/cast_cert_validator_test_helpers.cc ('K') | « net/cert/internal/path_builder_pkits_unittest.cc ('k') | net/cert/internal/path_builder_verify_certificate_chain_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: net/cert/internal/path_builder_unittest.cc

diff --git a/net/cert/internal/path_builder_unittest.cc b/net/cert/internal/path_builder_unittest.cc

index 44471fa876774d11dc6bc5dcde03f5c8985cec43..cef0f1fd7571ed7c3996d62b1a64ac8cdb6d07c0 100644

--- a/net/cert/internal/path_builder_unittest.cc

+++ b/net/cert/internal/path_builder_unittest.cc

@@ -137,26 +137,17 @@ class PathBuilderMultiRootTest : public ::testing::Test {

der::GeneralizedTime time_ = {2017, 3, 1, 0, 0, 0};

};

-void AddTrustedCertificate(scoped_refptr<ParsedCertificate> cert,

- TrustStoreInMemory* trust_store) {

- ASSERT_TRUE(cert.get());

- scoped_refptr<TrustAnchor> anchor =

- TrustAnchor::CreateFromCertificateNoConstraints(std::move(cert));

- ASSERT_TRUE(anchor.get());

- trust_store->AddTrustAnchor(std::move(anchor));

-// If the target cert is has the same name and key as a trust anchor, however

-// is signed but a different trust anchor. This should successfully build a

-// path, however the trust anchor will be the signer of this cert.

+// Tests when the target cert has the same name and key as a trust anchor,

+// however is signed by a different trust anchor. This should successfully build

+// a path, however the trust anchor will be the signer of this cert.

// (This test is very similar to TestEndEntityHasSameNameAndSpkiAsTrustAnchor

// but with different data; also in this test the target cert itself is in the

// trust store).

TEST_F(PathBuilderMultiRootTest, TargetHasNameAndSpkiOfTrustAnchor) {

TrustStoreInMemory trust_store;

- AddTrustedCertificate(a_by_b_, &trust_store);

- AddTrustedCertificate(b_by_f_, &trust_store);

+ trust_store.AddTrustAnchor(a_by_b_);

+ trust_store.AddTrustAnchor(b_by_f_);

CertPathBuilder::Result result;

CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_,

@@ -164,11 +155,16 @@ TEST_F(PathBuilderMultiRootTest, TargetHasNameAndSpkiOfTrustAnchor) {

path_builder.Run();

- ASSERT_TRUE(result.HasValidPath());

- const auto& path = result.GetBestValidPath()->path;

- ASSERT_EQ(1U, path.certs.size());

- EXPECT_EQ(a_by_b_, path.certs[0]);

- EXPECT_EQ(b_by_f_, path.trust_anchor->cert());

+ ASSERT_FALSE(result.HasValidPath());

+ // TODO(eroman): This probably should have succeeded and found the path below.

+ // It fails right now because path building stops on trust anchors (and the

+ // end entity is added as a trust anchor).

+ //

+ // const auto& path = result.GetBestValidPath()->path;

+ // ASSERT_EQ(2U, path.certs.size());

+ // EXPECT_EQ(a_by_b_, path.certs[0]);

+ // EXPECT_EQ(b_by_f_, path.certs[1]);

}

// If the target cert is has the same name and key as a trust anchor, however

@@ -176,7 +172,7 @@ TEST_F(PathBuilderMultiRootTest, TargetHasNameAndSpkiOfTrustAnchor) {

// is trusted, the certificate contents cannot be verified.

TEST_F(PathBuilderMultiRootTest, TargetWithSameNameAsTrustAnchorFails) {

TrustStoreInMemory trust_store;

- AddTrustedCertificate(a_by_b_, &trust_store);

+ trust_store.AddTrustAnchor(a_by_b_);

CertPathBuilder::Result result;

CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_,

@@ -188,16 +184,15 @@ TEST_F(PathBuilderMultiRootTest, TargetWithSameNameAsTrustAnchorFails) {

}

// Test a failed path building when the trust anchor is provided as a

-// supplemental certificate. Conceptually the following paths can be built:

+// supplemental certificate. Conceptually the following paths could be built:

// B(C) <- C(D) <- [Trust anchor D]

// B(C) <- C(D) <- D(D) <- [Trust anchor D]

-// The second one is extraneous given the shorter one, however path building

-// will enumerate it if the shorter one failed validation.

+// However the second one is extraneous given the shorter path.

TEST_F(PathBuilderMultiRootTest, SelfSignedTrustAnchorSupplementalCert) {

TrustStoreInMemory trust_store;

- AddTrustedCertificate(d_by_d_, &trust_store);

+ trust_store.AddTrustAnchor(d_by_d_);

// The (extraneous) trust anchor D(D) is supplied as a certificate, as is the

// intermediate needed for path building C(D).

@@ -216,30 +211,22 @@ TEST_F(PathBuilderMultiRootTest, SelfSignedTrustAnchorSupplementalCert) {

path_builder.Run();

EXPECT_FALSE(result.HasValidPath());

- ASSERT_EQ(2U, result.paths.size());

+ ASSERT_EQ(1U, result.paths.size());

EXPECT_FALSE(result.paths[0]->IsValid());

const auto& path0 = result.paths[0]->path;

- ASSERT_EQ(2U, path0.certs.size());

+ ASSERT_EQ(3U, path0.certs.size());

EXPECT_EQ(b_by_c_, path0.certs[0]);

EXPECT_EQ(c_by_d_, path0.certs[1]);

- EXPECT_EQ(d_by_d_, path0.trust_anchor->cert());

- const auto& path1 = result.paths[1]->path;

- ASSERT_EQ(3U, path1.certs.size());

- EXPECT_EQ(b_by_c_, path1.certs[0]);

- EXPECT_EQ(c_by_d_, path1.certs[1]);

- EXPECT_EQ(d_by_d_, path1.certs[2]);

- EXPECT_EQ(d_by_d_, path1.trust_anchor->cert());

+ EXPECT_EQ(d_by_d_, path0.certs[2]);

}

-// If the target cert is a self-signed cert whose key is a trust anchor, it

-// should verify.

+// Test verifying a certificate that is a trust anchor.

TEST_F(PathBuilderMultiRootTest, TargetIsSelfSignedTrustAnchor) {

TrustStoreInMemory trust_store;

- AddTrustedCertificate(e_by_e_, &trust_store);

+ trust_store.AddTrustAnchor(e_by_e_);

// This is not necessary for the test, just an extra...

- AddTrustedCertificate(f_by_e_, &trust_store);

+ trust_store.AddTrustAnchor(f_by_e_);

CertPathBuilder::Result result;

CertPathBuilder path_builder(e_by_e_, &trust_store, &signature_policy_, time_,

@@ -247,18 +234,23 @@ TEST_F(PathBuilderMultiRootTest, TargetIsSelfSignedTrustAnchor) {

path_builder.Run();

- ASSERT_TRUE(result.HasValidPath());

- const auto& path = result.GetBestValidPath()->path;

- ASSERT_EQ(1U, path.certs.size());

- EXPECT_EQ(e_by_e_, path.certs[0]);

- EXPECT_EQ(e_by_e_, path.trust_anchor->cert());

+ ASSERT_FALSE(result.HasValidPath());

+ // TODO(eroman): This test currently fails because path building stops

+ // searching once it identifies a certificate as a trust anchor. In this case

+ // the target is a trust anchor, however could be verified using the

+ // self-signedness (or even the cert itself).

+ // const auto& path = result.GetBestValidPath()->path;

+ // ASSERT_EQ(2U, path.certs.size());

+ // EXPECT_EQ(e_by_e_, path.certs[0]);

+ // EXPECT_EQ(e_by_e_, path.certs[1]);

}

// If the target cert is directly issued by a trust anchor, it should verify

// without any intermediate certs being provided.

TEST_F(PathBuilderMultiRootTest, TargetDirectlySignedByTrustAnchor) {

TrustStoreInMemory trust_store;

- AddTrustedCertificate(b_by_f_, &trust_store);

+ trust_store.AddTrustAnchor(b_by_f_);

CertPathBuilder::Result result;

CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_,

@@ -268,16 +260,16 @@ TEST_F(PathBuilderMultiRootTest, TargetDirectlySignedByTrustAnchor) {

ASSERT_TRUE(result.HasValidPath());

const auto& path = result.GetBestValidPath()->path;

- ASSERT_EQ(1U, path.certs.size());

+ ASSERT_EQ(2U, path.certs.size());

EXPECT_EQ(a_by_b_, path.certs[0]);

- EXPECT_EQ(b_by_f_, path.trust_anchor->cert());

+ EXPECT_EQ(b_by_f_, path.certs[1]);

}

// Test that async cert queries are not made if the path can be successfully

// built with synchronously available certs.

TEST_F(PathBuilderMultiRootTest, TriesSyncFirst) {

TrustStoreInMemory trust_store;

- AddTrustedCertificate(e_by_e_, &trust_store);

+ trust_store.AddTrustAnchor(e_by_e_);

CertIssuerSourceStatic sync_certs;

sync_certs.AddCert(b_by_f_);

@@ -303,7 +295,7 @@ TEST_F(PathBuilderMultiRootTest, TriesSyncFirst) {

// simultaneously.

TEST_F(PathBuilderMultiRootTest, TestAsyncSimultaneous) {

TrustStoreInMemory trust_store;

- AddTrustedCertificate(e_by_e_, &trust_store);

+ trust_store.AddTrustAnchor(e_by_e_);

CertIssuerSourceStatic sync_certs;

sync_certs.AddCert(b_by_c_);

@@ -334,8 +326,8 @@ TEST_F(PathBuilderMultiRootTest, TestAsyncSimultaneous) {

TEST_F(PathBuilderMultiRootTest, TestLongChain) {

// Both D(D) and C(D) are trusted roots.

TrustStoreInMemory trust_store;

- AddTrustedCertificate(d_by_d_, &trust_store);

- AddTrustedCertificate(c_by_d_, &trust_store);

+ trust_store.AddTrustAnchor(d_by_d_);

+ trust_store.AddTrustAnchor(c_by_d_);

// Certs B(C), and C(D) are all supplied.

CertIssuerSourceStatic sync_certs;

@@ -353,7 +345,7 @@ TEST_F(PathBuilderMultiRootTest, TestLongChain) {

// The result path should be A(B) <- B(C) <- C(D)

// not the longer but also valid A(B) <- B(C) <- C(D) <- D(D)

- EXPECT_EQ(2U, result.GetBestValidPath()->path.certs.size());

+ EXPECT_EQ(3U, result.GetBestValidPath()->path.certs.size());

}

// Test that PathBuilder will backtrack and try a different path if the first

@@ -361,7 +353,7 @@ TEST_F(PathBuilderMultiRootTest, TestLongChain) {

TEST_F(PathBuilderMultiRootTest, TestBacktracking) {

// Only D(D) is a trusted root.

TrustStoreInMemory trust_store;

- AddTrustedCertificate(d_by_d_, &trust_store);

+ trust_store.AddTrustAnchor(d_by_d_);

// Certs B(F) and F(E) are supplied synchronously, thus the path

// A(B) <- B(F) <- F(E) should be built first, though it won't verify.

@@ -387,11 +379,11 @@ TEST_F(PathBuilderMultiRootTest, TestBacktracking) {

// The result path should be A(B) <- B(C) <- C(D) <- D(D)

const auto& path = result.GetBestValidPath()->path;

- ASSERT_EQ(3U, path.certs.size());

+ ASSERT_EQ(4U, path.certs.size());

EXPECT_EQ(a_by_b_, path.certs[0]);

EXPECT_EQ(b_by_c_, path.certs[1]);

EXPECT_EQ(c_by_d_, path.certs[2]);

- EXPECT_EQ(d_by_d_, path.trust_anchor->cert());

+ EXPECT_EQ(d_by_d_, path.certs[3]);

}

// Test that whichever order CertIssuerSource returns the issuers, the path

@@ -399,7 +391,7 @@ TEST_F(PathBuilderMultiRootTest, TestBacktracking) {

TEST_F(PathBuilderMultiRootTest, TestCertIssuerOrdering) {

// Only D(D) is a trusted root.

TrustStoreInMemory trust_store;

- AddTrustedCertificate(d_by_d_, &trust_store);

+ trust_store.AddTrustAnchor(d_by_d_);

for (bool reverse_order : {false, true}) {

SCOPED_TRACE(reverse_order);

@@ -425,11 +417,11 @@ TEST_F(PathBuilderMultiRootTest, TestCertIssuerOrdering) {

// The result path should be A(B) <- B(C) <- C(D) <- D(D)

const auto& path = result.GetBestValidPath()->path;

- ASSERT_EQ(3U, path.certs.size());

+ ASSERT_EQ(4U, path.certs.size());

EXPECT_EQ(a_by_b_, path.certs[0]);

EXPECT_EQ(b_by_c_, path.certs[1]);

EXPECT_EQ(c_by_d_, path.certs[2]);

- EXPECT_EQ(d_by_d_, path.trust_anchor->cert());

+ EXPECT_EQ(d_by_d_, path.certs[3]);

}

@@ -445,12 +437,12 @@ class PathBuilderKeyRolloverTest : public ::testing::Test {

"net/data/verify_certificate_chain_unittest/key-rollover-oldchain.pem",

&test);

path = test.chain;

- oldroot_ = test.trust_anchor;

- time_ = test.time;

- ASSERT_EQ(2U, path.size());

+ ASSERT_EQ(3U, path.size());

target_ = path[0];

oldintermediate_ = path[1];

+ oldroot_ = path[2];

+ time_ = test.time;

ASSERT_TRUE(target_);

ASSERT_TRUE(oldintermediate_);

@@ -460,7 +452,7 @@ class PathBuilderKeyRolloverTest : public ::testing::Test {

&test);

path = test.chain;

- ASSERT_EQ(4U, path.size());

+ ASSERT_EQ(5U, path.size());

newintermediate_ = path[1];

newroot_ = path[2];

newrootrollover_ = path[3];

@@ -482,7 +474,7 @@ class PathBuilderKeyRolloverTest : public ::testing::Test {

scoped_refptr<ParsedCertificate> target_;

scoped_refptr<ParsedCertificate> oldintermediate_;

scoped_refptr<ParsedCertificate> newintermediate_;

- scoped_refptr<TrustAnchor> oldroot_;

+ scoped_refptr<ParsedCertificate> oldroot_;

scoped_refptr<ParsedCertificate> newroot_;

scoped_refptr<ParsedCertificate> newrootrollover_;

@@ -517,10 +509,10 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverOnlyOldRootTrusted) {

ASSERT_EQ(2U, result.paths.size());

const auto& path0 = result.paths[0]->path;

EXPECT_FALSE(result.paths[0]->IsValid());

- ASSERT_EQ(2U, path0.certs.size());

+ ASSERT_EQ(3U, path0.certs.size());

EXPECT_EQ(target_, path0.certs[0]);

EXPECT_EQ(newintermediate_, path0.certs[1]);

- EXPECT_EQ(oldroot_, path0.trust_anchor);

+ EXPECT_EQ(oldroot_, path0.certs[2]);

// Path builder will next attempt:

// target <- newintermediate <- newrootrollover <- oldroot

@@ -528,11 +520,11 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverOnlyOldRootTrusted) {

const auto& path1 = result.paths[1]->path;

EXPECT_EQ(1U, result.best_result_index);

EXPECT_TRUE(result.paths[1]->IsValid());

- ASSERT_EQ(3U, path1.certs.size());

+ ASSERT_EQ(4U, path1.certs.size());

EXPECT_EQ(target_, path1.certs[0]);

EXPECT_EQ(newintermediate_, path1.certs[1]);

EXPECT_EQ(newrootrollover_, path1.certs[2]);

- EXPECT_EQ(oldroot_, path1.trust_anchor);

+ EXPECT_EQ(oldroot_, path1.certs[3]);

}

// Tests that if both old and new roots are trusted it can build a path through

@@ -543,7 +535,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverBothRootsTrusted) {

// Both oldroot and newroot are trusted.

TrustStoreInMemory trust_store;

trust_store.AddTrustAnchor(oldroot_);

- AddTrustedCertificate(newroot_, &trust_store);

+ trust_store.AddTrustAnchor(newroot_);

// Both old and new intermediates + rollover cert are provided.

CertIssuerSourceStatic sync_certs;

@@ -567,16 +559,16 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverBothRootsTrusted) {

ASSERT_EQ(1U, result.paths.size());

const auto& path = result.paths[0]->path;

EXPECT_TRUE(result.paths[0]->IsValid());

- ASSERT_EQ(2U, path.certs.size());

+ ASSERT_EQ(3U, path.certs.size());

EXPECT_EQ(target_, path.certs[0]);

if (path.certs[1] != newintermediate_) {

DVLOG(1) << "USED OLD";

EXPECT_EQ(oldintermediate_, path.certs[1]);

- EXPECT_EQ(oldroot_, path.trust_anchor);

+ EXPECT_EQ(oldroot_, path.certs[2]);

} else {

DVLOG(1) << "USED NEW";

EXPECT_EQ(newintermediate_, path.certs[1]);

- EXPECT_EQ(newroot_, path.trust_anchor->cert());

+ EXPECT_EQ(newroot_, path.certs[2]);

}

@@ -584,8 +576,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverBothRootsTrusted) {

// sources, path building should fail at that point.

TEST_F(PathBuilderKeyRolloverTest, TestAnchorsNoMatchAndNoIssuerSources) {

TrustStoreInMemory trust_store;

- trust_store.AddTrustAnchor(

- TrustAnchor::CreateFromCertificateNoConstraints(newroot_));

+ trust_store.AddTrustAnchor(newroot_);

CertPathBuilder::Result result;

CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_,

@@ -610,8 +601,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) {

// Add two trust anchors (newroot_ and oldroot_). Path building will attempt

// them in this same order, as trust_store1 was added to

// trust_store_collection first.

- trust_store1.AddTrustAnchor(

- TrustAnchor::CreateFromCertificateNoConstraints(newroot_));

+ trust_store1.AddTrustAnchor(newroot_);

trust_store2.AddTrustAnchor(oldroot_);

// Only oldintermediate is supplied, so the path with newroot should fail,

@@ -635,10 +625,10 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) {

// but it will fail since oldintermediate is signed by oldroot.

EXPECT_FALSE(result.paths[0]->IsValid());

const auto& path = result.paths[0]->path;

- ASSERT_EQ(2U, path.certs.size());

+ ASSERT_EQ(3U, path.certs.size());

EXPECT_EQ(target_, path.certs[0]);

EXPECT_EQ(oldintermediate_, path.certs[1]);

- EXPECT_EQ(newroot_, path.trust_anchor->cert());

+ EXPECT_EQ(newroot_, path.certs[2]);

}

{

@@ -647,10 +637,10 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) {

// which should succeed.

EXPECT_TRUE(result.paths[result.best_result_index]->IsValid());

const auto& path = result.paths[result.best_result_index]->path;

- ASSERT_EQ(2U, path.certs.size());

+ ASSERT_EQ(3U, path.certs.size());

EXPECT_EQ(target_, path.certs[0]);

EXPECT_EQ(oldintermediate_, path.certs[1]);

- EXPECT_EQ(oldroot_, path.trust_anchor);

+ EXPECT_EQ(oldroot_, path.certs[2]);

}

@@ -685,21 +675,21 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) {

// but it will fail since newintermediate is signed by newroot.

EXPECT_FALSE(result.paths[0]->IsValid());

const auto& path0 = result.paths[0]->path;

- ASSERT_EQ(2U, path0.certs.size());

+ ASSERT_EQ(3U, path0.certs.size());

EXPECT_EQ(target_, path0.certs[0]);

EXPECT_EQ(newintermediate_, path0.certs[1]);

- EXPECT_EQ(oldroot_, path0.trust_anchor);

+ EXPECT_EQ(oldroot_, path0.certs[2]);

// Path builder will next attempt:

// target <- newintermediate <- newroot <- oldroot

// but it will fail since newroot is self-signed.

EXPECT_FALSE(result.paths[1]->IsValid());

const auto& path1 = result.paths[1]->path;

- ASSERT_EQ(3U, path1.certs.size());

+ ASSERT_EQ(4U, path1.certs.size());

EXPECT_EQ(target_, path1.certs[0]);

EXPECT_EQ(newintermediate_, path1.certs[1]);

EXPECT_EQ(newroot_, path1.certs[2]);

- EXPECT_EQ(oldroot_, path1.trust_anchor);

+ EXPECT_EQ(oldroot_, path1.certs[3]);

// Path builder will skip:

// target <- newintermediate <- newroot <- newrootrollover <- ...

@@ -710,11 +700,11 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) {

EXPECT_EQ(2U, result.best_result_index);

EXPECT_TRUE(result.paths[2]->IsValid());

const auto& path2 = result.paths[2]->path;

- ASSERT_EQ(3U, path2.certs.size());

+ ASSERT_EQ(4U, path2.certs.size());

EXPECT_EQ(target_, path2.certs[0]);

EXPECT_EQ(newintermediate_, path2.certs[1]);

EXPECT_EQ(newrootrollover_, path2.certs[2]);

- EXPECT_EQ(oldroot_, path2.trust_anchor);

+ EXPECT_EQ(oldroot_, path2.certs[3]);

}

// If the target cert is a trust anchor, however is not itself *signed* by a

@@ -724,7 +714,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) {

TEST_F(PathBuilderKeyRolloverTest, TestEndEntityIsTrustRoot) {

// Trust newintermediate.

TrustStoreInMemory trust_store;

- AddTrustedCertificate(newintermediate_, &trust_store);

+ trust_store.AddTrustAnchor(newintermediate_);

CertPathBuilder::Result result;

// Newintermediate is also the target cert.

@@ -770,7 +760,7 @@ TEST_F(PathBuilderKeyRolloverTest,

TestEndEntityHasSameNameAndSpkiAsTrustAnchor) {

// Trust newrootrollover.

TrustStoreInMemory trust_store;

- AddTrustedCertificate(newrootrollover_, &trust_store);

+ trust_store.AddTrustAnchor(newrootrollover_);

CertPathBuilder::Result result;

// Newroot is the target cert.

@@ -786,9 +776,9 @@ TEST_F(PathBuilderKeyRolloverTest,

// Newroot has same name+SPKI as newrootrollover, thus the path is valid and

// only contains newroot.

EXPECT_TRUE(best_result->IsValid());

- ASSERT_EQ(1U, best_result->path.certs.size());

+ ASSERT_EQ(2U, best_result->path.certs.size());

EXPECT_EQ(newroot_, best_result->path.certs[0]);

- EXPECT_EQ(newrootrollover_, best_result->path.trust_anchor->cert());

+ EXPECT_EQ(newrootrollover_, best_result->path.certs[1]);

}

// Test that PathBuilder will not try the same path twice if multiple

@@ -804,7 +794,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediates) {

// Only newroot is a trusted root.

TrustStoreInMemory trust_store;

- AddTrustedCertificate(newroot_, &trust_store);

+ trust_store.AddTrustAnchor(newroot_);

// The oldintermediate is supplied synchronously by |sync_certs1| and

// another copy of oldintermediate is supplied synchronously by |sync_certs2|.

@@ -838,26 +828,26 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediates) {

EXPECT_FALSE(result.paths[0]->IsValid());

const auto& path0 = result.paths[0]->path;

- ASSERT_EQ(2U, path0.certs.size());

+ ASSERT_EQ(3U, path0.certs.size());

EXPECT_EQ(target_, path0.certs[0]);

// Compare the DER instead of ParsedCertificate pointer, don't care which copy

// of oldintermediate was used in the path.

EXPECT_EQ(oldintermediate_->der_cert(), path0.certs[1]->der_cert());

- EXPECT_EQ(newroot_, path0.trust_anchor->cert());

+ EXPECT_EQ(newroot_, path0.certs[2]);

// Path builder will next attempt: target <- newintermediate <- newroot

// which will succeed.

EXPECT_EQ(1U, result.best_result_index);

EXPECT_TRUE(result.paths[1]->IsValid());

const auto& path1 = result.paths[1]->path;

- ASSERT_EQ(2U, path1.certs.size());

+ ASSERT_EQ(3U, path1.certs.size());

EXPECT_EQ(target_, path1.certs[0]);

EXPECT_EQ(newintermediate_, path1.certs[1]);

- EXPECT_EQ(newroot_, path1.trust_anchor->cert());

+ EXPECT_EQ(newroot_, path1.certs[2]);

}

-// Test when PathBuilder is given a cert CertIssuerSources that has the same

-// SPKI as a TrustAnchor.

+// Test when PathBuilder is given a cert via CertIssuerSources that has the same

+// SPKI as a trust anchor.

TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediateAndRoot) {

// Create a separate copy of newroot.

scoped_refptr<ParsedCertificate> newroot_dupe(ParsedCertificate::Create(

@@ -868,7 +858,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediateAndRoot) {

// Only newroot is a trusted root.

TrustStoreInMemory trust_store;

- AddTrustedCertificate(newroot_, &trust_store);

+ trust_store.AddTrustAnchor(newroot_);

// The oldintermediate and newroot are supplied synchronously by |sync_certs|.

CertIssuerSourceStatic sync_certs;

@@ -883,19 +873,18 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediateAndRoot) {

path_builder.Run();

EXPECT_FALSE(result.HasValidPath());

- ASSERT_EQ(2U, result.paths.size());

- // TODO(eroman): Is this right?

+ ASSERT_EQ(1U, result.paths.size());

// Path builder attempt: target <- oldintermediate <- newroot

// but it will fail since oldintermediate is signed by oldroot.

EXPECT_FALSE(result.paths[0]->IsValid());

const auto& path = result.paths[0]->path;

- ASSERT_EQ(2U, path.certs.size());

+ ASSERT_EQ(3U, path.certs.size());

EXPECT_EQ(target_, path.certs[0]);

EXPECT_EQ(oldintermediate_, path.certs[1]);

// Compare the DER instead of ParsedCertificate pointer, don't care which copy

// of newroot was used in the path.

- EXPECT_EQ(newroot_->der_cert(), path.trust_anchor->cert()->der_cert());

+ EXPECT_EQ(newroot_->der_cert(), path.certs[2]->der_cert());

}

class MockCertIssuerSourceRequest : public CertIssuerSource::Request {

@@ -948,7 +937,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncIssuersFromSingleSource) {

// Only newroot is a trusted root.

TrustStoreInMemory trust_store;

- AddTrustedCertificate(newroot_, &trust_store);

+ trust_store.AddTrustAnchor(newroot_);

CertPathBuilder::Result result;

CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_,

@@ -1005,19 +994,19 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncIssuersFromSingleSource) {

// but it will fail since oldintermediate is signed by oldroot.

EXPECT_FALSE(result.paths[0]->IsValid());

const auto& path0 = result.paths[0]->path;

- ASSERT_EQ(2U, path0.certs.size());

+ ASSERT_EQ(3U, path0.certs.size());

EXPECT_EQ(target_, path0.certs[0]);

EXPECT_EQ(oldintermediate_, path0.certs[1]);

- EXPECT_EQ(newroot_, path0.trust_anchor->cert());

+ EXPECT_EQ(newroot_, path0.certs[2]);

// After the second batch of async results, path builder will attempt:

// target <- newintermediate <- newroot which will succeed.

EXPECT_TRUE(result.paths[1]->IsValid());

const auto& path1 = result.paths[1]->path;

- ASSERT_EQ(2U, path1.certs.size());

+ ASSERT_EQ(3U, path1.certs.size());

EXPECT_EQ(target_, path1.certs[0]);

EXPECT_EQ(newintermediate_, path1.certs[1]);

- EXPECT_EQ(newroot_, path1.trust_anchor->cert());

+ EXPECT_EQ(newroot_, path1.certs[2]);

}

// Test that PathBuilder will not try the same path twice if CertIssuerSources

@@ -1027,7 +1016,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) {

// Only newroot is a trusted root.

TrustStoreInMemory trust_store;

- AddTrustedCertificate(newroot_, &trust_store);

+ trust_store.AddTrustAnchor(newroot_);

CertPathBuilder::Result result;

CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_,

@@ -1091,10 +1080,10 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) {

// but it will fail since oldintermediate is signed by oldroot.

EXPECT_FALSE(result.paths[0]->IsValid());

const auto& path0 = result.paths[0]->path;

- ASSERT_EQ(2U, path0.certs.size());

+ ASSERT_EQ(3U, path0.certs.size());

EXPECT_EQ(target_, path0.certs[0]);

EXPECT_EQ(oldintermediate_, path0.certs[1]);

- EXPECT_EQ(newroot_, path0.trust_anchor->cert());

+ EXPECT_EQ(newroot_, path0.certs[2]);

// The second async result does not generate any path.

@@ -1102,10 +1091,10 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) {

// target <- newintermediate <- newroot which will succeed.

EXPECT_TRUE(result.paths[1]->IsValid());

const auto& path1 = result.paths[1]->path;

- ASSERT_EQ(2U, path1.certs.size());

+ ASSERT_EQ(3U, path1.certs.size());

EXPECT_EQ(target_, path1.certs[0]);

EXPECT_EQ(newintermediate_, path1.certs[1]);

- EXPECT_EQ(newroot_, path1.trust_anchor->cert());

+ EXPECT_EQ(newroot_, path1.certs[2]);

}

} // namespace