Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(119)

Issues Search
    My Issues | Starred     Open | Closed | All

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

Issue 2292333002: Add errors per ResultPath for CertPathBuilder. (Closed)
Patch Set: remove error for null trust anchor Created 4 years, 4 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
« no previous file with comments | « 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') | Show Comments 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 4264d459e94f6447f232aced82412349ab461733..338178a9121d3490cdce411c1e9be7a2aebee0d6 100644
--- a/net/cert/internal/path_builder_unittest.cc
+++ b/net/cert/internal/path_builder_unittest.cc
@@ -10,7 +10,6 @@
#include "base/location.h"
#include "base/path_service.h"
#include "base/threading/thread_task_runner_handle.h"
-#include "net/base/net_errors.h"
#include "net/base/test_completion_callback.h"
#include "net/cert/internal/cert_issuer_source_static.h"
#include "net/cert/internal/parsed_certificate.h"
@@ -28,6 +27,8 @@
namespace net {
+// TODO(crbug.com/634443): Assert the errors for each ResultPath.
+
namespace {
using ::testing::_;
@@ -191,9 +192,8 @@ TEST_F(PathBuilderMultiRootTest, TargetHasNameAndSpkiOfTrustAnchor) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
- ASSERT_FALSE(result.paths.empty());
- const auto& path = result.paths[result.best_result_index]->path;
+ 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());
@@ -212,7 +212,7 @@ TEST_F(PathBuilderMultiRootTest, TargetWithSameNameAsTrustAnchorFails) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.error());
+ EXPECT_FALSE(result.HasValidPath());
}
// Test a failed path building when the trust anchor is provided as a
@@ -243,10 +243,10 @@ TEST_F(PathBuilderMultiRootTest, SelfSignedTrustAnchorSupplementalCert) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.error());
+ EXPECT_FALSE(result.HasValidPath());
ASSERT_EQ(2U, result.paths.size());
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+ EXPECT_FALSE(result.paths[0]->valid);
const auto& path0 = result.paths[0]->path;
ASSERT_EQ(2U, path0.certs.size());
EXPECT_EQ(b_by_c_, path0.certs[0]);
@@ -275,9 +275,8 @@ TEST_F(PathBuilderMultiRootTest, TargetIsSelfSignedTrustAnchor) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
- ASSERT_FALSE(result.paths.empty());
- const auto& path = result.paths[result.best_result_index]->path;
+ 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());
@@ -295,9 +294,8 @@ TEST_F(PathBuilderMultiRootTest, TargetDirectlySignedByTrustAnchor) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- ASSERT_EQ(OK, result.error());
- ASSERT_FALSE(result.paths.empty());
- const auto& path = result.paths[result.best_result_index]->path;
+ 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());
@@ -325,7 +323,7 @@ TEST_F(PathBuilderMultiRootTest, TriesSyncFirst) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
EXPECT_EQ(0, async_certs.num_async_gets());
}
@@ -348,7 +346,7 @@ TEST_F(PathBuilderMultiRootTest, SychronousOnlyMode) {
EXPECT_EQ(CompletionStatus::SYNC, path_builder.Run(base::Closure()));
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.error());
+ EXPECT_FALSE(result.HasValidPath());
EXPECT_EQ(0, async_certs.num_async_gets());
}
@@ -377,7 +375,7 @@ TEST_F(PathBuilderMultiRootTest, TestAsyncSimultaneous) {
EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
EXPECT_EQ(1, async_certs1.num_async_gets());
EXPECT_EQ(1, async_certs2.num_async_gets());
}
@@ -402,13 +400,11 @@ TEST_F(PathBuilderMultiRootTest, TestLongChain) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ ASSERT_TRUE(result.HasValidPath());
// 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)
- ASSERT_FALSE(result.paths.empty());
- const auto& path = result.paths[result.best_result_index]->path;
- EXPECT_EQ(2U, path.certs.size());
+ EXPECT_EQ(2U, result.GetBestValidPath()->path.certs.size());
}
// Test that PathBuilder will backtrack and try a different path if the first
@@ -438,11 +434,10 @@ TEST_F(PathBuilderMultiRootTest, TestBacktracking) {
EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ ASSERT_TRUE(result.HasValidPath());
// The result path should be A(B) <- B(C) <- C(D) <- D(D)
- ASSERT_FALSE(result.paths.empty());
- const auto& path = result.paths[result.best_result_index]->path;
+ const auto& path = result.GetBestValidPath()->path;
ASSERT_EQ(3U, path.certs.size());
EXPECT_EQ(a_by_b_, path.certs[0]);
EXPECT_EQ(b_by_c_, path.certs[1]);
@@ -477,11 +472,10 @@ TEST_F(PathBuilderMultiRootTest, TestCertIssuerOrdering) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ ASSERT_TRUE(result.HasValidPath());
// The result path should be A(B) <- B(C) <- C(D) <- D(D)
- ASSERT_FALSE(result.paths.empty());
- const auto& path = result.paths[result.best_result_index]->path;
+ const auto& path = result.GetBestValidPath()->path;
ASSERT_EQ(3U, path.certs.size());
EXPECT_EQ(a_by_b_, path.certs[0]);
EXPECT_EQ(b_by_c_, path.certs[1]);
@@ -561,13 +555,13 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverOnlyOldRootTrusted) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
// Path builder will first attempt: target <- newintermediate <- oldroot
// but it will fail since newintermediate is signed by newroot.
ASSERT_EQ(2U, result.paths.size());
const auto& path0 = result.paths[0]->path;
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+ EXPECT_FALSE(result.paths[0]->valid);
ASSERT_EQ(2U, path0.certs.size());
EXPECT_EQ(target_, path0.certs[0]);
EXPECT_EQ(newintermediate_, path0.certs[1]);
@@ -578,7 +572,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverOnlyOldRootTrusted) {
// which will succeed.
const auto& path1 = result.paths[1]->path;
EXPECT_EQ(1U, result.best_result_index);
- EXPECT_EQ(OK, result.paths[1]->error);
+ EXPECT_TRUE(result.paths[1]->valid);
ASSERT_EQ(3U, path1.certs.size());
EXPECT_EQ(target_, path1.certs[0]);
EXPECT_EQ(newintermediate_, path1.certs[1]);
@@ -609,7 +603,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverBothRootsTrusted) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
// Path builder willattempt one of:
// target <- oldintermediate <- oldroot
@@ -617,7 +611,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverBothRootsTrusted) {
// either will succeed.
ASSERT_EQ(1U, result.paths.size());
const auto& path = result.paths[0]->path;
- EXPECT_EQ(OK, result.paths[0]->error);
+ EXPECT_TRUE(result.paths[0]->valid);
ASSERT_EQ(2U, path.certs.size());
EXPECT_EQ(target_, path.certs[0]);
if (path.certs[1] != newintermediate_) {
@@ -649,11 +643,11 @@ TEST_F(PathBuilderKeyRolloverTest, TestSyncAnchorsPreferred) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
ASSERT_EQ(1U, result.paths.size());
const auto& path = result.paths[0]->path;
- EXPECT_EQ(OK, result.paths[0]->error);
+ EXPECT_TRUE(result.paths[0]->valid);
ASSERT_EQ(1U, path.certs.size());
EXPECT_EQ(target_, path.certs[0]);
EXPECT_EQ(oldintermediate_, path.trust_anchor->cert());
@@ -689,19 +683,19 @@ TEST_F(PathBuilderKeyRolloverTest, TestAsyncAnchorsBeforeSyncIssuers) {
EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
ASSERT_EQ(2U, result.paths.size());
{
const auto& path = result.paths[0]->path;
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+ EXPECT_FALSE(result.paths[0]->valid);
ASSERT_EQ(1U, path.certs.size());
EXPECT_EQ(newintermediate_, path.certs[0]);
EXPECT_EQ(oldroot_, path.trust_anchor);
}
{
const auto& path = result.paths[1]->path;
- EXPECT_EQ(OK, result.paths[1]->error);
+ EXPECT_TRUE(result.paths[1]->valid);
ASSERT_EQ(1U, path.certs.size());
EXPECT_EQ(newintermediate_, path.certs[0]);
EXPECT_EQ(newroot_, path.trust_anchor->cert());
@@ -721,7 +715,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestAsyncAnchorsNoMatchAndNoIssuerSources) {
EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.error());
+ EXPECT_FALSE(result.HasValidPath());
ASSERT_EQ(0U, result.paths.size());
}
@@ -743,11 +737,11 @@ TEST_F(PathBuilderKeyRolloverTest, TestAsyncAnchorsAndAsyncIssuers) {
EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
ASSERT_EQ(1U, result.paths.size());
const auto& path = result.paths[0]->path;
- EXPECT_EQ(OK, result.paths[0]->error);
+ EXPECT_TRUE(result.paths[0]->valid);
ASSERT_EQ(2U, path.certs.size());
EXPECT_EQ(target_, path.certs[0]);
EXPECT_EQ(newintermediate_, path.certs[1]);
@@ -779,13 +773,13 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) {
EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
ASSERT_EQ(2U, result.paths.size());
{
// Path builder may first attempt: target <- oldintermediate <- newroot
// but it will fail since oldintermediate is signed by oldroot.
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+ EXPECT_FALSE(result.paths[0]->valid);
const auto& path = result.paths[0]->path;
ASSERT_EQ(2U, path.certs.size());
EXPECT_EQ(target_, path.certs[0]);
@@ -797,7 +791,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) {
// Path builder will next attempt:
// target <- old intermediate <- oldroot
// which should succeed.
- EXPECT_EQ(OK, result.paths[result.best_result_index]->error);
+ EXPECT_TRUE(result.paths[result.best_result_index]->valid);
const auto& path = result.paths[result.best_result_index]->path;
ASSERT_EQ(2U, path.certs.size());
EXPECT_EQ(target_, path.certs[0]);
@@ -830,12 +824,12 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) {
EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
ASSERT_EQ(3U, result.paths.size());
// Path builder will first attempt: target <- newintermediate <- oldroot
// but it will fail since newintermediate is signed by newroot.
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+ EXPECT_FALSE(result.paths[0]->valid);
const auto& path0 = result.paths[0]->path;
ASSERT_EQ(2U, path0.certs.size());
EXPECT_EQ(target_, path0.certs[0]);
@@ -845,7 +839,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) {
// Path builder will next attempt:
// target <- newintermediate <- newroot <- oldroot
// but it will fail since newroot is self-signed.
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[1]->error);
+ EXPECT_FALSE(result.paths[1]->valid);
const auto& path1 = result.paths[1]->path;
ASSERT_EQ(3U, path1.certs.size());
EXPECT_EQ(target_, path1.certs[0]);
@@ -860,7 +854,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) {
// Finally path builder will use:
// target <- newintermediate <- newrootrollover <- oldroot
EXPECT_EQ(2U, result.best_result_index);
- EXPECT_EQ(OK, result.paths[2]->error);
+ EXPECT_TRUE(result.paths[2]->valid);
const auto& path2 = result.paths[2]->path;
ASSERT_EQ(3U, path2.certs.size());
EXPECT_EQ(target_, path2.certs[0]);
@@ -885,7 +879,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestEndEntityIsTrustRoot) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.error());
+ EXPECT_FALSE(result.HasValidPath());
}
// If target has same Name+SAN+SPKI as a necessary intermediate, test if a path
@@ -912,7 +906,7 @@ TEST_F(PathBuilderKeyRolloverTest,
// This could actually be OK, but CertPathBuilder does not build the
// newroot <- newrootrollover <- oldroot path.
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.error());
+ EXPECT_FALSE(result.HasValidPath());
}
// If target has same Name+SAN+SPKI as the trust root, test that a (trivial)
@@ -930,15 +924,13 @@ TEST_F(PathBuilderKeyRolloverTest,
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ ASSERT_TRUE(result.HasValidPath());
- ASSERT_FALSE(result.paths.empty());
- const CertPathBuilder::ResultPath* best_result =
- result.paths[result.best_result_index].get();
+ const CertPathBuilder::ResultPath* best_result = result.GetBestValidPath();
// Newroot has same name+SPKI as newrootrollover, thus the path is valid and
// only contains newroot.
- EXPECT_EQ(OK, best_result->error);
+ EXPECT_TRUE(best_result->valid);
ASSERT_EQ(1U, best_result->path.certs.size());
EXPECT_EQ(newroot_, best_result->path.certs[0]);
EXPECT_EQ(newrootrollover_, best_result->path.trust_anchor->cert());
@@ -980,12 +972,12 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediates) {
EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
ASSERT_EQ(2U, result.paths.size());
// Path builder will first attempt: target <- oldintermediate <- newroot
// but it will fail since oldintermediate is signed by oldroot.
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+ EXPECT_FALSE(result.paths[0]->valid);
const auto& path0 = result.paths[0]->path;
ASSERT_EQ(2U, path0.certs.size());
@@ -998,7 +990,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediates) {
// Path builder will next attempt: target <- newintermediate <- newroot
// which will succeed.
EXPECT_EQ(1U, result.best_result_index);
- EXPECT_EQ(OK, result.paths[1]->error);
+ EXPECT_TRUE(result.paths[1]->valid);
const auto& path1 = result.paths[1]->path;
ASSERT_EQ(2U, path1.certs.size());
EXPECT_EQ(target_, path1.certs[0]);
@@ -1030,13 +1022,13 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediateAndRoot) {
EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.error());
+ EXPECT_FALSE(result.HasValidPath());
ASSERT_EQ(2U, result.paths.size());
// TODO(eroman): Is this right?
// Path builder attempt: target <- oldintermediate <- newroot
// but it will fail since oldintermediate is signed by oldroot.
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+ EXPECT_FALSE(result.paths[0]->valid);
const auto& path = result.paths[0]->path;
ASSERT_EQ(2U, path.certs.size());
EXPECT_EQ(target_, path.certs[0]);
@@ -1157,12 +1149,12 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncCallbacksFromSingleSource) {
// Ensure pathbuilder finished and filled result.
callback.WaitForResult();
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
ASSERT_EQ(2U, result.paths.size());
// Path builder first attempts: target <- oldintermediate <- newroot
// but it will fail since oldintermediate is signed by oldroot.
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+ EXPECT_FALSE(result.paths[0]->valid);
const auto& path0 = result.paths[0]->path;
ASSERT_EQ(2U, path0.certs.size());
EXPECT_EQ(target_, path0.certs[0]);
@@ -1171,7 +1163,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncCallbacksFromSingleSource) {
// After the second batch of async results, path builder will attempt:
// target <- newintermediate <- newroot which will succeed.
- EXPECT_EQ(OK, result.paths[1]->error);
+ EXPECT_TRUE(result.paths[1]->valid);
const auto& path1 = result.paths[1]->path;
ASSERT_EQ(2U, path1.certs.size());
EXPECT_EQ(target_, path1.certs[0]);
@@ -1272,12 +1264,12 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) {
// Ensure pathbuilder finished and filled result.
callback.WaitForResult();
- EXPECT_EQ(OK, result.error());
+ EXPECT_TRUE(result.HasValidPath());
ASSERT_EQ(2U, result.paths.size());
// Path builder first attempts: target <- oldintermediate <- newroot
// but it will fail since oldintermediate is signed by oldroot.
- EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+ EXPECT_FALSE(result.paths[0]->valid);
const auto& path0 = result.paths[0]->path;
ASSERT_EQ(2U, path0.certs.size());
EXPECT_EQ(target_, path0.certs[0]);
@@ -1288,7 +1280,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) {
// After the third batch of async results, path builder will attempt:
// target <- newintermediate <- newroot which will succeed.
- EXPECT_EQ(OK, result.paths[1]->error);
+ EXPECT_TRUE(result.paths[1]->valid);
const auto& path1 = result.paths[1]->path;
ASSERT_EQ(2U, path1.certs.size());
EXPECT_EQ(target_, path1.certs[0]);
« no previous file with comments | « 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 »

Powered by Google App Engine
This is Rietveld 408576698