Add TrustStoreNSS and TrustStoreCollection

net/cert/internal/trust_store_nss.cc

Index: net/cert/internal/trust_store_nss.cc
diff --git a/net/cert/internal/trust_store_nss.cc b/net/cert/internal/trust_store_nss.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b23973c781fad574e0746532dd95a742a933618c
--- /dev/null
+++ b/net/cert/internal/trust_store_nss.cc
@@ -0,0 +1,133 @@
+// 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_nss.h"
+
+#include <cert.h>
+#include <certdb.h>
+
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
+#include "base/task_runner.h"
+#include "crypto/nss_util.h"
+#include "net/cert/internal/parsed_certificate.h"
+
+// TODO(mattm): structure so that supporting chromeos stuff is doable (

[eroman, 2016/08/27 01:53:39]

nit on capitalization: ChromeOS ?

[mattm, 2016/08/29 20:38:17]

Done.

+// TrustStoreChromeOS which uses net::NSSProfileFilterChromeOS.. similar to

[eroman, 2016/08/27 01:53:38]

Not sure I follow this comment. Was it a note-to-self, or a comment for this
file?

[mattm, 2016/08/29 20:38:16]

Yeah, I guess it was mostly a note-to-self. I'll remove it.

+// CertVerifyProcChromeOS )
+//   Could have protected version of FindTrustAnchorsForCert that takes a
+//   cert_filter callback param, which is then passed into GetAnchors, and
+//   subclass of TrustStoreNSS that calls that? Or take the callback in
+//   constructor?
+
+namespace net {
+
+namespace {
+
+// Get all certs in NSS which have a subject matching |der_name| and which are
+// marked as a trusted CA.
+void GetAnchors(scoped_refptr<ParsedCertificate> cert,

[eroman, 2016/08/27 01:53:38]

is our Bind() smart enough to avoid copying from storage to the invocation?

I would probably just express this as const-reference to ensure doesn't do a
spurious add-ref.

[mattm, 2016/08/29 20:38:17]

I think it is, but changed to be safe.

+                TrustAnchors* out_anchors) {
+  crypto::EnsureNSSInit();
+
+  SECItem name;
+  name.len = cert->tbs().issuer_tlv.Length();
+  name.data = const_cast<uint8_t*>(cert->tbs().issuer_tlv.UnsafeData());
+  CERTCertList* found_certs = CERT_CreateSubjectCertList(
+      nullptr /* certList */, CERT_GetDefaultCertDB(), &name,
+      PR_Now() /* sorttime */, PR_FALSE /* validOnly */);

[eroman, 2016/08/27 01:53:38]

Is validOnly=false necessary? (i.e. are trust anchors going to be invalid
certificates)

[mattm, 2016/08/29 20:38:17]

validOnly here refers to the time validity checking, so we want that to be false
for trust anchors. Added a comment.

+  if (!found_certs)
+    return;
+
+  for (CERTCertListNode* node = CERT_LIST_HEAD(found_certs);
+       !CERT_LIST_END(node, found_certs); node = CERT_LIST_NEXT(node)) {
+    CERTCertTrust trust;
+    if (CERT_GetCertTrust(node->cert, &trust) != SECSuccess)
+      continue;
+
+    // TODO(mattm): handle explicit distrust (blacklisting)?
+    const int ca_trust = CERTDB_TRUSTED_CA;
+    if ((trust.sslFlags & ca_trust) != ca_trust)
+      continue;
+
+    scoped_refptr<ParsedCertificate> anchor_cert =
+        ParsedCertificate::CreateFromCertificateData(
+            node->cert->derCert.data, node->cert->derCert.len,
+            ParsedCertificate::DataSource::INTERNAL_COPY, {});
+    if (!anchor_cert) {
+      // TODO(mattm): return errors better.

[eroman, 2016/08/27 01:53:39]

In fact we may need to be less strict and just do a dumb extraction of SPKI and
subject, without all the other enforcements. (food for thought, no change needed
here).

[mattm, 2016/08/29 20:38:17]

Acknowledged.

+      LOG(ERROR) << "error parsing issuer certificate";
+      continue;
+    }
+
+    out_anchors->push_back(TrustAnchor::CreateFromCertificateNoConstraints(

[eroman, 2016/08/27 01:53:38]

Does NSS trust store have the notion of attached constraints, or enforcing any
constraints from the certificate?

[mattm, 2016/08/29 20:38:16]

I don't think it does. Ryan?

[mattm, 2016/09/01 00:59:02]

I chatted with Ryan about that. The current state is there is only one public
API for this in NSS, CERT_GetImposedNameConstraints (added in NSS 3.19, which we
also don't require yet). It's only used for the hardcoded ANSSI constraints and
doesn't have a way for clients to add additional constraints. So as long as we
do our own enforcement of imposed constraints like we do for CertVerifyProc, we
don't need to worry about that at the moment.

+        std::move(anchor_cert)));
+  }
+  CERT_DestroyCertList(found_certs);
+}
+
+class GetAnchorsRequest : public TrustStore::Request {
+ public:
+  explicit GetAnchorsRequest(const TrustStore::TrustAnchorsCallback& callback);
+  // Destruction of the Request cancels it. GetAnchors will still run, but the
+  // callback will not be called since the WeakPtr will be invalidated.
+  ~GetAnchorsRequest() override = default;
+
+  void Start(scoped_refptr<ParsedCertificate> cert,
+             base::TaskRunner* task_runner);
+
+ private:
+  void HandleGetAnchors(std::unique_ptr<TrustAnchors> anchors);
+
+  TrustStore::TrustAnchorsCallback callback_;
+  base::WeakPtrFactory<GetAnchorsRequest> weak_ptr_factory_;
+};
+
+GetAnchorsRequest::GetAnchorsRequest(
+    const TrustStore::TrustAnchorsCallback& callback)
+    : callback_(callback), weak_ptr_factory_(this) {}
+
+void GetAnchorsRequest::Start(scoped_refptr<ParsedCertificate> cert,
+                              base::TaskRunner* task_runner) {
+  auto anchors = base::MakeUnique<TrustAnchors>();
+
+  auto* anchors_ptr = anchors.get();

[eroman, 2016/08/27 01:53:38]

heh. gotta love C++'s undefined argument evaluation order :)

+  task_runner->PostTaskAndReply(
+      FROM_HERE, base::Bind(&GetAnchors, std::move(cert), anchors_ptr),
+      base::Bind(&GetAnchorsRequest::HandleGetAnchors,
+                 weak_ptr_factory_.GetWeakPtr(), base::Passed(&anchors)));
+}
+
+void GetAnchorsRequest::HandleGetAnchors(
+    std::unique_ptr<TrustAnchors> anchors) {
+  base::ResetAndReturn(&callback_).Run(std::move(*anchors));
+  // |this| may be deleted here.
+}
+
+}  // namespace
+
+TrustStoreNSS::TrustStoreNSS(scoped_refptr<base::TaskRunner> nss_task_runner)
+    : nss_task_runner_(std::move(nss_task_runner)) {}
+
+TrustStoreNSS::~TrustStoreNSS() = default;
+
+void TrustStoreNSS::FindTrustAnchorsForCert(
+    scoped_refptr<ParsedCertificate> cert,
+    const TrustAnchorsCallback& callback,
+    TrustAnchors* synchronous_matches,
+    std::unique_ptr<Request>* out_req) const {
+  if (callback.is_null())
+    return;
+
+  auto req(base::MakeUnique<GetAnchorsRequest>(callback));

[eroman, 2016/08/27 01:53:38]

style nit: for consistency with higher up, use "auto req = "

[mattm, 2016/08/29 20:38:16]

Done.

+  // Use the original issuer value instead of the normalized version. NSS does a

[eroman, 2016/08/27 01:53:38]

How about moving this comment into GetAnchors() since that is where this
decision is being made now.

[mattm, 2016/08/29 20:38:16]

Done.

+  // less extensive normalization in its Name comparisons, so our normalized
+  // version may not match the unnormalized version.
+  req->Start(std::move(cert), nss_task_runner_.get());
+  *out_req = std::move(req);
+}
+
+}  // namespace net