Return the constructed certificate chain in X509Certificate::Verify()

net/base/x509_certificate_nss.cc

Index: net/base/x509_certificate_nss.cc
diff --git a/net/base/x509_certificate_nss.cc b/net/base/x509_certificate_nss.cc
index 9ec937600909307b75f7e23ad95ff0987efc0075..7c578890ce44f3000db7960015d2a2c828ff89f4 100644
--- a/net/base/x509_certificate_nss.cc
+++ b/net/base/x509_certificate_nss.cc
@@ -168,19 +168,28 @@ int MapCertErrorToCertStatus(int err) {
 // Saves some information about the certificate chain cert_list in
 // *verify_result.  The caller MUST initialize *verify_result before calling
 // this function.
-// Note that cert_list[0] is the end entity certificate and cert_list doesn't
-// contain the root CA certificate.
+// Note that cert_list[0] is the end entity certificate.
 void GetCertChainInfo(CERTCertList* cert_list,
+                      CERTCertificate* root_cert,
                       CertVerifyResult* verify_result) {
   // NOTE: Using a NSS library before 3.12.3.1 will crash below.  To see the
   // NSS version currently in use:
   // 1. use ldd on the chrome executable for NSS's location (ie. libnss3.so*)
   // 2. use ident libnss3.so* for the library's version
-  DCHECK(cert_list);
+  if (!cert_list)
+    return;

[wtc, 2011/07/26 00:16:35]

Why do you want to allow cert_list to be NULL?
This is not necessary if you don't move the GetCertChainInfo
call.

Have you verified that cvout[cvout_cert_list_index].value.pointer.chain
can be used when PKIXVerifyCert fails?  It is uncommon
for a function to return outputs on failure, so I'm not
sure if PKIXVerifyCert does that.

[Ryan Sleevi, 2011/07/26 01:44:50]

Thanks for catching this. In digging down further into libpkix, I see that the
chain is not made available if there are any exceptions processing/validating.

This is incredibly unfortunate, because, combined with the error log not
containing any/all of the errors, it makes it nigh impossible to reliably
display the chain that was built with errors.

This should probably be fixed upstream, but like the error log, I don't think
this would be a trivial change to libpkix, based on how it builds/evaluates
chains.

For this reason, I removed the check on line 214-215, since it represents an
impossible condition. I left the check in for lines 218/219, but removed the
comment, because I believe it's reasonable to expect that NSS may change this
behaviour in the future, and a root cert may not always be available.

+
+  CERTCertificate* verified_cert = NULL;
+  std::vector<CERTCertificate*> verified_chain;
   int i = 0;
   for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list);
        !CERT_LIST_END(node, cert_list);
-       node = CERT_LIST_NEXT(node), i++) {
+       node = CERT_LIST_NEXT(node), ++i) {
+    if (i == 0) {
+      verified_cert = node->cert;
+    } else {
+      verified_chain.push_back(node->cert);
+    }
     SECAlgorithmID& signature = node->cert->signature;
     SECOidTag oid_tag = SECOID_FindOIDTag(&signature.algorithm);
     switch (oid_tag) {
@@ -201,6 +210,15 @@ void GetCertChainInfo(CERTCertList* cert_list,
         break;
     }
   }
+
+  if (!verified_cert)
+    return;
+
+  // If the chain was not trusted, |root_cert| may be NULL.
+  if (root_cert)
+    verified_chain.push_back(root_cert);
+  verify_result->verified_cert =
+      X509Certificate::CreateFromHandle(verified_cert, verified_chain);
 }
 
 // IsKnownRoot returns true if the given certificate is one that we believe
@@ -769,6 +787,8 @@ int X509Certificate::Verify(const std::string& hostname,
                             int flags,
                             CertVerifyResult* verify_result) const {
   verify_result->Reset();
+  verify_result->verified_cert =
+      CreateFromHandle(cert_handle_, GetIntermediateCertificates());
 
   if (IsBlacklisted()) {
     verify_result->cert_status |= CERT_STATUS_REVOKED;
@@ -807,6 +827,11 @@ int X509Certificate::Verify(const std::string& hostname,
     flags &= ~VERIFY_EV_CERT;
   }
   status = PKIXVerifyCert(cert_handle_, check_revocation, NULL, 0, cvout);
+
+  GetCertChainInfo(cvout[cvout_cert_list_index].value.pointer.chain,
+                   cvout[cvout_trust_anchor_index].value.pointer.cert,
+                   verify_result);
+
   if (status != SECSuccess) {
     int err = PORT_GetError();
     LOG(ERROR) << "CERT_PKIXVerifyCert for " << hostname
@@ -825,8 +850,6 @@ int X509Certificate::Verify(const std::string& hostname,
     return MapSecurityError(err);
   }
 
-  GetCertChainInfo(cvout[cvout_cert_list_index].value.pointer.chain,
-                   verify_result);
   if (IsCertStatusError(verify_result->cert_status))
     return MapCertStatusToNetError(verify_result->cert_status);