Filter out certificates that cannot be used as client certs on Windows

This matches the other platforms and IE8 in not offering expired certificates
and checking the key usage extensions.

BUG=45353
TEST=logging into websites with client certificates still works

Review URL: http://codereview.chromium.org/2897006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@55607 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 71 insertions, 0 deletions

diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 62af6f8..e935f80 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -271,6 +271,40 @@ bool IsProblematicComodoEVCACert(const CERTCertificate& cert) {
                 cert.serialNumber.len) == 0;
 }
 
+// This callback is intended to be used with CertFindChainInStore. In addition
+// to filtering by extended/enhanced key usage, we do not show expired
+// certificates and require digital signature usage in the key usage
+// extension.
+//
+// This matches our behavior on Mac OS X and that of NSS. It also matches the
+// default behavior of IE8. See http://support.microsoft.com/kb/890326 and
+// http://blogs.msdn.com/b/askie/archive/2009/06/09/my-expired-client-certificates-no-longer-display-when-connecting-to-my-web-server-using-ie8.aspx
+BOOL WINAPI ClientCertFindCallback(PCCERT_CONTEXT cert_context,
+                                   void* find_arg) {
+  LOG(INFO) << "Calling ClientCertFindCallback from _nss";
+  // Verify the certificate's KU is good.
+  BYTE key_usage;
+  if (CertGetIntendedKeyUsage(X509_ASN_ENCODING, cert_context->pCertInfo,
+                              &key_usage, 1)) {
+    if (!(key_usage & CERT_DIGITAL_SIGNATURE_KEY_USAGE))
+      return FALSE;
+  } else {
+    DWORD err = GetLastError();
+    // If |err| is non-zero, it's an actual error. Otherwise the extension
+    // just isn't present, and we treat it as if everything was allowed.
+    if (err) {
+      DLOG(ERROR) << "CertGetIntendedKeyUsage failed: " << err;
+      return FALSE;
+    }
+  }
+
+  // Verify the current time is within the certificate's validity period.
+  if (CertVerifyTimeValidity(NULL, cert_context->pCertInfo) != 0)
+    return FALSE;
+
+  return TRUE;
+}
+
 #endif
 
 }  // namespace
@@ -1257,6 +1291,7 @@ SECStatus SSLClientSocketNSS::ClientAuthHandler(
   find_by_issuer_para.pszUsageIdentifier = szOID_PKIX_KP_CLIENT_AUTH;
   find_by_issuer_para.cIssuer = ca_names->nnames;
   find_by_issuer_para.rgIssuer = ca_names->nnames ? &issuer_list[0] : NULL;
+  find_by_issuer_para.pfnFindCallback = ClientCertFindCallback;
 
   PCCERT_CHAIN_CONTEXT chain_context = NULL;
 
diff --git a/net/socket/ssl_client_socket_win.cc b/net/socket/ssl_client_socket_win.cc
index 3d751d5..38bdfce 100644
--- a/net/socket/ssl_client_socket_win.cc
+++ b/net/socket/ssl_client_socket_win.cc
@@ -254,6 +254,41 @@ static CredHandle* GetCredHandle(PCCERT_CONTEXT client_cert,
 
 //-----------------------------------------------------------------------------
 
+// This callback is intended to be used with CertFindChainInStore. In addition
+// to filtering by extended/enhanced key usage, we do not show expired
+// certificates and require digital signature usage in the key usage
+// extension.
+//
+// This matches our behavior on Mac OS X and that of NSS. It also matches the
+// default behavior of IE8. See http://support.microsoft.com/kb/890326 and
+// http://blogs.msdn.com/b/askie/archive/2009/06/09/my-expired-client-certificates-no-longer-display-when-connecting-to-my-web-server-using-ie8.aspx
+static BOOL WINAPI ClientCertFindCallback(PCCERT_CONTEXT cert_context,
+                                          void* find_arg) {
+  // Verify the certificate's KU is good.
+  BYTE key_usage;
+  if (CertGetIntendedKeyUsage(X509_ASN_ENCODING, cert_context->pCertInfo,
+                              &key_usage, 1)) {
+    if (!(key_usage & CERT_DIGITAL_SIGNATURE_KEY_USAGE))
+      return FALSE;
+  } else {
+    DWORD err = GetLastError();
+    // If |err| is non-zero, it's an actual error. Otherwise the extension
+    // just isn't present, and we treat it as if everything was allowed.
+    if (err) {
+      DLOG(ERROR) << "CertGetIntendedKeyUsage failed: " << err;
+      return FALSE;
+    }
+  }
+
+  // Verify the current time is within the certificate's validity period.
+  if (CertVerifyTimeValidity(NULL, cert_context->pCertInfo) != 0)
+    return FALSE;
+
+  return TRUE;
+}
+
+//-----------------------------------------------------------------------------
+
 // A memory certificate store for client certificates.  This allows us to
 // close the "MY" system certificate store when we finish searching for
 // client certificates.
@@ -409,6 +444,7 @@ void SSLClientSocketWin::GetSSLCertRequestInfo(
   find_by_issuer_para.pszUsageIdentifier = szOID_PKIX_KP_CLIENT_AUTH;
   find_by_issuer_para.cIssuer = issuer_list.cIssuers;
   find_by_issuer_para.rgIssuer = issuer_list.aIssuers;
+  find_by_issuer_para.pfnFindCallback = ClientCertFindCallback;
 
   PCCERT_CHAIN_CONTEXT chain_context = NULL;