Allow TrustStore queries to be asynchronous.

Also changes FindTrustAnchorsByNormalizedName to FindTrustAnchorsForCert, as
some different implementations may do normalization differently. By passing in
the target cert, the implementation can decide whether to use the
pre-normalized issuer or the raw one.

BUG=635203

Review-Url: https://codereview.chromium.org/2266333002
Cr-Commit-Position: refs/heads/master@{#414875}

net/cert/internal/path_builder_unittest.cc

@@ -17,6 +17,7 @@
 #include "net/cert/internal/signature_policy.h"
 #include "net/cert/internal/test_helpers.h"
 #include "net/cert/internal/trust_store_in_memory.h"
+#include "net/cert/internal/trust_store_test_helpers.h"
 #include "net/cert/internal/verify_certificate_chain.h"
 #include "net/cert/pem_tokenizer.h"
 #include "net/der/input.h"
@@ -627,31 +628,141 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverBothRootsTrusted) {
   }
 }
 
-class MockTrustStore : public TrustStore {
- public:
-  MOCK_CONST_METHOD2(FindTrustAnchorsByNormalizedName,
-                     void(const der::Input& normalized_name,
-                          TrustAnchors* matches));
-};
+// If trust anchors are provided both synchronously and asynchronously for the
+// same cert, the synchronously provided ones should be tried first, and
+// pathbuilder should finish synchronously.
+TEST_F(PathBuilderKeyRolloverTest, TestSyncAnchorsPreferred) {
+  TrustStoreInMemoryAsync trust_store;
+  // Both oldintermediate and newintermediate are trusted, but oldintermediate
+  // is returned synchronously and newintermediate asynchronously.
+  trust_store.AddSyncTrustAnchor(
+      TrustAnchor::CreateFromCertificateNoConstraints(oldintermediate_));
+  trust_store.AddAsyncTrustAnchor(
+      TrustAnchor::CreateFromCertificateNoConstraints(newintermediate_));
+
+  CertPathBuilder::Result result;
+  CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_,
+                               &result);
+
+  EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
+
+  EXPECT_EQ(OK, result.error());
+
+  ASSERT_EQ(1U, result.paths.size());
+  const auto& path = result.paths[0]->path;
+  EXPECT_EQ(OK, result.paths[0]->error);
+  ASSERT_EQ(1U, path.certs.size());
+  EXPECT_EQ(target_, path.certs[0]);
+  EXPECT_EQ(oldintermediate_, path.trust_anchor->cert());
+}
+
+// Async trust anchor checks should be done before synchronous issuer checks are
+// considered. (Avoiding creating unnecessarily long paths.)
+//
+// Two valid paths could be built:
+// newintermediate <- newrootrollover <- oldroot
+// newintermediate <- newroot
+// One invalid path could be built:
+// newintermediate <- oldroot
+//
+// First: newintermediate <- oldroot will be tried, since oldroot is
+// available synchronously, but this path will not verify.
+// Second: newintermediate <- newroot should be built, even though
+// newrootrollover issuer is available synchronously and newroot is async. This
+// path should verify and pathbuilder will stop.
+TEST_F(PathBuilderKeyRolloverTest, TestAsyncAnchorsBeforeSyncIssuers) {
+  TrustStoreInMemoryAsync trust_store;
+  trust_store.AddSyncTrustAnchor(oldroot_);
+  trust_store.AddAsyncTrustAnchor(
+      TrustAnchor::CreateFromCertificateNoConstraints(newroot_));
+
+  CertIssuerSourceStatic sync_certs;
+  sync_certs.AddCert(newrootrollover_);
+
+  CertPathBuilder::Result result;
+  CertPathBuilder path_builder(newintermediate_, &trust_store,
+                               &signature_policy_, time_, &result);
+  path_builder.AddCertIssuerSource(&sync_certs);
+
+  EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
+
+  EXPECT_EQ(OK, result.error());
+
+  ASSERT_EQ(2U, result.paths.size());
+  {
+    const auto& path = result.paths[0]->path;
+    EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.paths[0]->error);
+    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);
+    ASSERT_EQ(1U, path.certs.size());
+    EXPECT_EQ(newintermediate_, path.certs[0]);
+    EXPECT_EQ(newroot_, path.trust_anchor->cert());
+  }
+}
+
+// If async trust anchor query returned no results, and there are no issuer
+// sources, path building should fail at that point.
+TEST_F(PathBuilderKeyRolloverTest, TestAsyncAnchorsNoMatchAndNoIssuerSources) {
+  TrustStoreInMemoryAsync trust_store;
+  trust_store.AddAsyncTrustAnchor(
+      TrustAnchor::CreateFromCertificateNoConstraints(newroot_));
+
+  CertPathBuilder::Result result;
+  CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_,
+                               &result);
+
+  EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
+
+  EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, result.error());
+
+  ASSERT_EQ(0U, result.paths.size());
+}
+
+// Both trust store and issuer source are async. Should successfully build a
+// path.
+TEST_F(PathBuilderKeyRolloverTest, TestAsyncAnchorsAndAsyncIssuers) {
+  TrustStoreInMemoryAsync trust_store;
+  trust_store.AddAsyncTrustAnchor(
+      TrustAnchor::CreateFromCertificateNoConstraints(newroot_));
+
+  AsyncCertIssuerSourceStatic async_certs;
+  async_certs.AddCert(newintermediate_);
+
+  CertPathBuilder::Result result;
+  CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_,
+                               &result);
+  path_builder.AddCertIssuerSource(&async_certs);
+
+  EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
+
+  EXPECT_EQ(OK, result.error());
+
+  ASSERT_EQ(1U, result.paths.size());
+  const auto& path = result.paths[0]->path;
+  EXPECT_EQ(OK, result.paths[0]->error);
+  ASSERT_EQ(2U, path.certs.size());
+  EXPECT_EQ(target_, path.certs[0]);
+  EXPECT_EQ(newintermediate_, path.certs[1]);
+  EXPECT_EQ(newroot_, path.trust_anchor->cert());
+}
 
 // Tests that multiple trust root matches on a single path will be considered.
 // Both roots have the same subject but different keys. Only one of them will
 // verify.
 TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) {
-  NiceMock<MockTrustStore> trust_store;
-  // Default handler for any other TrustStore requests.
-  EXPECT_CALL(trust_store, FindTrustAnchorsByNormalizedName(_, _))
-      .WillRepeatedly(Return());
-  // Both newroot and oldroot are trusted, and newroot is returned first in the
-  // matches vector.
-  EXPECT_CALL(trust_store, FindTrustAnchorsByNormalizedName(
-                               newroot_->normalized_subject(), _))
-      .WillRepeatedly(Invoke(
-          [this](const der::Input& normalized_name, TrustAnchors* matches) {
-            matches->push_back(
-                TrustAnchor::CreateFromCertificateNoConstraints(newroot_));
-            matches->push_back(oldroot_);
-          }));
+  TrustStoreInMemoryAsync trust_store;
+  // Since FindTrustAnchorsByNormalizedName returns newroot synchronously, it
+  // should be tried first.
+  trust_store.AddSyncTrustAnchor(
+      TrustAnchor::CreateFromCertificateNoConstraints(newroot_));
+  // oldroot is returned asynchronously, so it should only be tried after the
+  // path built with newroot fails.
+  trust_store.AddAsyncTrustAnchor(oldroot_);
 
   // Only oldintermediate is supplied, so the path with newroot should fail,
   // oldroot should succeed.
@@ -663,12 +774,9 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) {
                                &result);
   path_builder.AddCertIssuerSource(&sync_certs);
 
-  EXPECT_EQ(CompletionStatus::SYNC, RunPathBuilder(&path_builder));
+  EXPECT_EQ(CompletionStatus::ASYNC, RunPathBuilder(&path_builder));
 
   EXPECT_EQ(OK, result.error());
-  // Since FindTrustAnchorsByNormalizedName returns newroot first, it should be
-  // tried first. (Note: this may change if PathBuilder starts prioritizing the
-  // path building order.)
   ASSERT_EQ(2U, result.paths.size());
 
   {

net/cert/internal/trust_store.cc

@@ -36,6 +36,9 @@ TrustAnchor::TrustAnchor(scoped_refptr<ParsedCertificate> cert,
 
 TrustAnchor::~TrustAnchor() = default;
 
+TrustStore::Request::Request() = default;
+TrustStore::Request::~Request() = default;
+
 TrustStore::TrustStore() = default;
 TrustStore::~TrustStore() = default;
 

net/cert/internal/trust_store.h

@@ -7,6 +7,7 @@
 
 #include <vector>
 
+#include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "net/base/net_export.h"
 #include "net/cert/internal/parsed_certificate.h"
@@ -114,13 +115,35 @@ using TrustAnchors = std::vector<scoped_refptr<TrustAnchor>>;
 // Interface for finding trust anchors.
 class NET_EXPORT TrustStore {
  public:
+  class NET_EXPORT Request {
+   public:
+    Request();
+    // Destruction of the Request cancels it.
+    virtual ~Request();
+  };
+
   TrustStore();
   virtual ~TrustStore();
 
-  // Returns the trust anchors that match |name| in |*matches|, if any.
-  virtual void FindTrustAnchorsByNormalizedName(
-      const der::Input& normalized_name,
-      TrustAnchors* matches) const = 0;
+  using TrustAnchorsCallback = base::Callback<void(TrustAnchors)>;
+
+  // Returns the trust anchors that match |cert|'s issuer name in
+  // |*synchronous_matches| and/or through |callback|. |cert| and
+  // |synchronous_matches| must not be null.
+  //
+  // If results are available synchronously, they will be appended to
+  // |*synchronous_matches|. |*synchronous_matches| will not be modified
+  // asynchronously.
+  //
+  // If |callback| is not null and results may be available asynchronously,
+  // |*out_req| will be filled with a Request, and |callback| will be called
+  // when results are available. The Request may be destroyed to cancel
+  // the callback if it has not occurred yet.
+  virtual void FindTrustAnchorsForCert(
+      const ParsedCertificate* cert,
+      const TrustAnchorsCallback& callback,
+      TrustAnchors* synchronous_matches,
+      std::unique_ptr<Request>* out_req) const = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TrustStore);

net/cert/internal/trust_store_in_memory.cc

@@ -19,12 +19,14 @@ void TrustStoreInMemory::AddTrustAnchor(scoped_refptr<TrustAnchor> anchor) {
                                  std::move(anchor)));
 }
 
-void TrustStoreInMemory::FindTrustAnchorsByNormalizedName(
-    const der::Input& normalized_name,
-    TrustAnchors* matches) const {
-  auto range = anchors_.equal_range(normalized_name.AsStringPiece());
+void TrustStoreInMemory::FindTrustAnchorsForCert(
+    const ParsedCertificate* cert,
+    const TrustAnchorsCallback& callback,
+    TrustAnchors* synchronous_matches,
+    std::unique_ptr<Request>* out_req) const {
+  auto range = anchors_.equal_range(cert->normalized_issuer().AsStringPiece());
   for (auto it = range.first; it != range.second; ++it)
-    matches->push_back(it->second);
+    synchronous_matches->push_back(it->second);
 }
 
 }  // namespace net

net/cert/internal/trust_store_in_memory.h

@@ -30,9 +30,12 @@ class NET_EXPORT TrustStoreInMemory : public TrustStore {
 
   void AddTrustAnchor(scoped_refptr<TrustAnchor> anchor);
 
-  // Returns the trust anchors that match |name| in |*matches|, if any.
-  void FindTrustAnchorsByNormalizedName(const der::Input& normalized_name,
-                                        TrustAnchors* matches) const override;
+  // TrustStore implementation:
+  void FindTrustAnchorsForCert(
+      const ParsedCertificate* cert,
+      const TrustAnchorsCallback& callback,
+      TrustAnchors* synchronous_matches,
+      std::unique_ptr<Request>* out_req) const override;
 
  private:
   // Multimap from normalized subject -> TrustAnchor.

net/cert/internal/trust_store_test_helpers.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/cert/internal/trust_store_test_helpers.h"
+
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/thread_task_runner_handle.h"
+
+namespace net {
+
+namespace {
+
+class TrustStoreInMemoryAsyncRequest : public TrustStore::Request {
+ public:
+  explicit TrustStoreInMemoryAsyncRequest(
+      const TrustStore::TrustAnchorsCallback& callback)
+      : callback_(callback), weak_ptr_factory_(this) {}
+
+  void PostTrustCallback(TrustAnchors anchors) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(&TrustStoreInMemoryAsyncRequest::DoTrustCallback,
+                   weak_ptr_factory_.GetWeakPtr(), std::move(anchors)));
+  }
+
+ private:
+  void DoTrustCallback(TrustAnchors anchors) {
+    base::ResetAndReturn(&callback_).Run(std::move(anchors));
+    // |this| may be deleted here.
+  }
+
+  TrustStore::TrustAnchorsCallback callback_;
+  base::WeakPtrFactory<TrustStoreInMemoryAsyncRequest> weak_ptr_factory_;
+};
+
+}  // namespace
+
+void TrustStoreRequestDeleter(std::unique_ptr<TrustStore::Request>* req_owner,
+                              const base::Closure& done_callback,
+                              std::unique_ptr<TrustAnchors> anchors) {
+  req_owner->reset();
+  done_callback.Run();
+}
+
+TrustAnchorResultRecorder::TrustAnchorResultRecorder() = default;
+TrustAnchorResultRecorder::~TrustAnchorResultRecorder() = default;
+
+TrustStore::TrustAnchorsCallback TrustAnchorResultRecorder::Callback() {
+  return base::Bind(&TrustAnchorResultRecorder::OnGotAnchors,
+                    base::Unretained(this));
+}
+
+void TrustAnchorResultRecorder::OnGotAnchors(TrustAnchors anchors) {
+  anchors_ = std::move(anchors);
+  run_loop_.Quit();
+}
+
+TrustStoreInMemoryAsync::TrustStoreInMemoryAsync() = default;
+TrustStoreInMemoryAsync::~TrustStoreInMemoryAsync() = default;
+
+void TrustStoreInMemoryAsync::AddSyncTrustAnchor(
+    scoped_refptr<TrustAnchor> anchor) {
+  sync_store_.AddTrustAnchor(std::move(anchor));
+}
+
+void TrustStoreInMemoryAsync::AddAsyncTrustAnchor(
+    scoped_refptr<TrustAnchor> anchor) {
+  async_store_.AddTrustAnchor(std::move(anchor));
+}
+
+void TrustStoreInMemoryAsync::FindTrustAnchorsForCert(
+    const ParsedCertificate* cert,
+    const TrustAnchorsCallback& callback,
+    TrustAnchors* synchronous_matches,
+    std::unique_ptr<Request>* out_req) const {
+  sync_store_.FindTrustAnchorsForCert(cert, TrustAnchorsCallback(),
+                                      synchronous_matches, nullptr);
+  if (!callback.is_null()) {
+    TrustAnchors async_matches;
+    async_store_.FindTrustAnchorsForCert(cert, TrustAnchorsCallback(),
+                                         &async_matches, nullptr);
+
+    std::unique_ptr<TrustStoreInMemoryAsyncRequest> req(
+        base::MakeUnique<TrustStoreInMemoryAsyncRequest>(callback));
+    req->PostTrustCallback(std::move(async_matches));
+
+    *out_req = std::move(req);
+  }
+}
+
+}  // namespace net

net/cert/internal/trust_store_test_helpers.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef NET_CERT_INTERNAL_TRUST_STORE_TEST_HELPERS_H_
+#define NET_CERT_INTERNAL_TRUST_STORE_TEST_HELPERS_H_
+
+#include "base/callback.h"
+#include "base/run_loop.h"
+#include "net/cert/internal/trust_store.h"
+#include "net/cert/internal/trust_store_in_memory.h"
+
+namespace net {
+
+// Deletes the Request owned by |*req_owner|, then calls done_callback. Intended
+// to be passed as the TrustAnchorsCallback to FindTrustAnchorsForCert to test
+// deleting the Request during the request callback.
+void TrustStoreRequestDeleter(std::unique_ptr<TrustStore::Request>* req_owner,
+                              const base::Closure& done_callback,
+                              std::unique_ptr<TrustAnchors> anchors);
+
+// Helper to record async results from a FindTrustAnchorsForCert call.
+class TrustAnchorResultRecorder {
+ public:
+  TrustAnchorResultRecorder();
+  ~TrustAnchorResultRecorder();
+
+  TrustStore::TrustAnchorsCallback Callback();
+
+  void Run() { run_loop_.Run(); }
+
+  const TrustAnchors& matches() const { return anchors_; }
+
+ private:
+  void OnGotAnchors(TrustAnchors anchors);
+
+  base::RunLoop run_loop_;
+  TrustAnchors anchors_;
+};
+
+// In-memory TrustStore that can return results synchronously, asynchronously,
+// or both.
+class TrustStoreInMemoryAsync : public TrustStore {
+ public:
+  TrustStoreInMemoryAsync();
+  ~TrustStoreInMemoryAsync() override;
+
+  // Adds |anchor| to the set of results that will be returned synchronously.
+  void AddSyncTrustAnchor(scoped_refptr<TrustAnchor> anchor);
+
+  // Adds |anchor| to the set of results that will be returned asynchronously.
+  void AddAsyncTrustAnchor(scoped_refptr<TrustAnchor> anchor);
+
+  // TrustStore implementation:
+  void FindTrustAnchorsForCert(
+      const ParsedCertificate* cert,
+      const TrustAnchorsCallback& callback,
+      TrustAnchors* synchronous_matches,
+      std::unique_ptr<Request>* out_req) const override;
+
+ private:
+  TrustStoreInMemory sync_store_;
+  TrustStoreInMemory async_store_;
+};
+
+}  // namespace net
+
+#endif  // NET_CERT_INTERNAL_TRUST_STORE_TEST_HELPERS_H_

net/net.gypi

@@ -1416,6 +1416,8 @@
       'cert/internal/signature_algorithm_unittest.cc',
       'cert/internal/test_helpers.cc',
       'cert/internal/test_helpers.h',
+      'cert/internal/trust_store_test_helpers.cc',
+      'cert/internal/trust_store_test_helpers.h',
       'cert/internal/verify_certificate_chain_pkits_unittest.cc',
       'cert/internal/verify_certificate_chain_typed_unittest.h',
       'cert/internal/verify_certificate_chain_unittest.cc',