Build a chain in ClientCertStoreNSS to send intermediates to the server.

net/ssl/client_cert_store_nss.cc

 // Copyright 2013 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/ssl/client_cert_store_nss.h"
 
 #include <nss.h>
 #include <ssl.h>
 
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string_piece.h"
 #include "base/threading/worker_pool.h"
 #include "crypto/nss_crypto_module_delegate.h"
+#include "net/cert/scoped_nss_types.h"
 #include "net/cert/x509_util.h"
 #include "net/ssl/ssl_cert_request_info.h"
 
 namespace net {
 
 ClientCertStoreNSS::ClientCertStoreNSS(
     const PasswordDelegateFactory& password_delegate_factory)
     : password_delegate_factory_(password_delegate_factory) {}
 
 ClientCertStoreNSS::~ClientCertStoreNSS() {}
@@ skipping 19 matching lines @@
   // If the task could not be posted, behave as if there were no certificates
   // which requires to clear |selected_certs|.
   selected_certs->clear();
   callback.Run();
 }
 
 // static
 void ClientCertStoreNSS::FilterCertsOnWorkerThread(
     const CertificateList& certs,
     const SSLCertRequestInfo& request,
-    bool query_nssdb,
     CertificateList* filtered_certs) {
   DCHECK(filtered_certs);
 
   filtered_certs->clear();
 
   // Create a "fake" CERTDistNames structure. No public API exists to create
   // one from a list of issuers.
   CERTDistNames ca_names;
   ca_names.arena = NULL;
   ca_names.nnames = 0;
@@ skipping 19 matching lines @@
 
     // Only offer unexpired certificates.
     if (CERT_CheckCertValidTimes(handle, PR_Now(), PR_TRUE) !=
         secCertTimeValid) {
       DVLOG(2) << "skipped expired cert: "
                << base::StringPiece(handle->nickname);
       continue;
     }
 
     // Check if the certificate issuer is allowed by the server.
-    if (request.cert_authorities.empty() ||
-        (!query_nssdb && cert->IsIssuedByEncoded(request.cert_authorities)) ||
-        (query_nssdb &&
-         NSS_CmpCertChainWCANames(handle, &ca_names) == SECSuccess)) {
-      DVLOG(2) << "matched cert: " << base::StringPiece(handle->nickname);
-      filtered_certs->push_back(cert);
-    } else {
+    if (!request.cert_authorities.empty() &&
+        NSS_CmpCertChainWCANames(handle, &ca_names) != SECSuccess) {
       DVLOG(2) << "skipped non-matching cert: "
                << base::StringPiece(handle->nickname);
+      continue;
     }
+
+    DVLOG(2) << "matched cert: " << base::StringPiece(handle->nickname);
+
+    // Build a certificate chain from |handle| to include any
+    // intermediates. Some deployments expect the client to supply intermediates
+    // out of the local store. https://crbug.com/548631

[Ryan Sleevi, 2016/02/05 23:51:24]

Blah, I remember what the 'bug' was now.

So NSS_CmpCertChainWCANames builds the chain up to a depth of 20 using
CERT_FindCertByName() to validate that |ca_names| matches, but
CERT_CertChainFromCert may find a different chain.

Both are bugged in that they don't handle cross-certification properly, which is
why I thought it should DIAF, but to be fair, the OS X logic doesn't handle it
properly either (the Windows code, unsurprisingly, does)

At least now I remember more about my hatred ;)

+    ScopedCERTCertificateList chain(CERT_CertChainFromCert(
+        handle, certUsageSSLClient, PR_FALSE /* includeRoot */));
+    if (!chain) {
+      DVLOG(2) << "could not build chain: "
+               << base::StringPiece(handle->nickname);
+      continue;
+    }
+    std::vector<base::StringPiece> der_chain;
+    for (int i = 0; i < chain->len; i++) {
+      der_chain.push_back(
+          base::StringPiece(reinterpret_cast<const char*>(chain->certs[i].data),
+                            chain->certs[i].len));
+    }
+    scoped_refptr<X509Certificate> full_cert =
+        X509Certificate::CreateFromDERCertChain(der_chain);

[Ryan Sleevi, 2016/02/05 23:51:24]

This actually forces NSS through a full reparse and token scan (to see if it
already has handles allocated)

It would be better to build from the OSCertHandles returned in |chain| and
CreateFromOSCertHandles

+    if (!full_cert.get()) {
+      DVLOG(2) << "could not decode chain: "
+               << base::StringPiece(handle->nickname);
+      continue;
+    }
+    filtered_certs->push_back(full_cert);
   }
   DVLOG(2) << "num_raw:" << num_raw
            << " num_filtered:" << filtered_certs->size();
 
   std::sort(filtered_certs->begin(), filtered_certs->end(),
             x509_util::ClientCertSorter());
 }
 
 void ClientCertStoreNSS::GetAndFilterCertsOnWorkerThread(
     scoped_ptr<crypto::CryptoModuleBlockingPasswordDelegate> password_delegate,
     const SSLCertRequestInfo* request,
     CertificateList* selected_certs) {
   CertificateList platform_certs;
   GetPlatformCertsOnWorkerThread(std::move(password_delegate), &platform_certs);
-  FilterCertsOnWorkerThread(platform_certs, *request, true, selected_certs);
+  FilterCertsOnWorkerThread(platform_certs, *request, selected_certs);
 }
 
 // static
 void ClientCertStoreNSS::GetPlatformCertsOnWorkerThread(
     scoped_ptr<crypto::CryptoModuleBlockingPasswordDelegate> password_delegate,
     net::CertificateList* certs) {
   CERTCertList* found_certs =
       CERT_FindUserCertsByUsage(CERT_GetDefaultCertDB(), certUsageSSLClient,
                                 PR_FALSE, PR_FALSE, password_delegate.get());
   if (!found_certs) {
     DVLOG(2) << "No client certs found.";
     return;
   }
   for (CERTCertListNode* node = CERT_LIST_HEAD(found_certs);
        !CERT_LIST_END(node, found_certs); node = CERT_LIST_NEXT(node)) {
     certs->push_back(X509Certificate::CreateFromHandle(
         node->cert, X509Certificate::OSCertHandles()));
   }
   CERT_DestroyCertList(found_certs);
 }
 
 }  // namespace net