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 "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 59 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);
+    if (!request.cert_authorities.empty()) {
+      if ((!query_nssdb &&
+           !cert->IsIssuedByEncoded(request.cert_authorities)) ||
+          (query_nssdb &&
+           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);
+
+    if (!query_nssdb) {

[davidben, 2015/12/14 22:26:32]

This is only false in tests. Which... you could argue this means we're barely
testing anything and I would agree. Didn't see a clear way to avoid that, so I
left things as they were here.

[Ryan Sleevi, 2015/12/16 01:46:54]

Wait, it's false for all tests? There shouldn't be any reason we can't enable
this for tests at this point.

       filtered_certs->push_back(cert);
-    } else {
-      DVLOG(2) << "skipped non-matching cert: "
+      continue;
+    }
+
+    // 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, 2015/12/16 01:46:53]

// Attempt to construct a complete certificate chain from |handle|,
// as some servers expect the client to send intermediates.
//
// Consider a chain of EE -> A -> B, where -> indicates "issued by".
// The server may indicate that "B" is an acceptable certificate
// authority, and expect the client to send both EE and A, so that it
// can construct the path back to B. See https://crbug.com/548631 for
// an example.
//
// Note that there is an edge case, in the event of cross-certificates;
// namely, consider the following chains:
// EE -> A -> B
// EE -> A' -> C
// Where A and A' have the same subject and public key, but different
// issuers. Unfortunately, NSS does not offer an API to indicate which
// trust anchor(s) are preferred in chain building, thus it's possible
// for the client to send EE and A, when the server needs EE and A'.
//
// Ideally, servers would be properly configured such that they can
// always handle clients sending only EE, and that should be the first
// response if any other edge cases emerge. However, this best-effort
// case is enough to resolve some incompatabilities.

+    ScopedCERTCertificateList chain(CERT_CertChainFromCert(
+        handle, certUsageSSLClient, PR_FALSE /* includeRoot */));
+    if (!chain) {
+      DVLOG(2) << "could not build chain: "
                << base::StringPiece(handle->nickname);
+      continue;

[Ryan Sleevi, 2015/12/16 01:46:53]

Wow, NSS is dumb; it shouldn't choke if it can't build a chain, it should just
send the leaf.

     }
+    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);
+    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,
@@ skipping 17 matching lines @@
   }
   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