Add server certificate request parameters to be stored in SSLCertRequestInfo.

Currently SSLCertRequestInfo provides a list of applicable client certificates,
filtered against server request. This patch adds the server criteria to the
class as a part of a larger refactoring effort.

BUG=65546

Review URL: https://chromiumcodereview.appspot.com/11739004

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

6 files changed, 204 insertions, 15 deletions

diff --git a/net/socket/ssl_client_socket_mac.cc b/net/socket/ssl_client_socket_mac.cc
index 3efea48..69ef093 100644
--- a/net/socket/ssl_client_socket_mac.cc
+++ b/net/socket/ssl_client_socket_mac.cc
@@ -748,8 +748,15 @@ bool SSLClientSocketMac::GetSSLInfo(SSLInfo* ssl_info) {
 
 void SSLClientSocketMac::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
-  // I'm being asked for available client certs (identities).
-  // First, get the cert issuer names allowed by the server.
+  cert_request_info->host_and_port = host_and_port_.ToString();
+  cert_request_info->cert_authorities.clear();
+  cert_request_info->cert_key_types.clear();
+  cert_request_info->client_certs.clear();
+
+  // Retrieve the cert issuers accepted by the server. This information is
+  // currently (temporarily) being saved both in |valid_issuers| and
+  // |cert_authorities|, the latter being the target solution. The refactoring
+  // effort is being tracked in http://crbug.com/166642.
   std::vector<CertPrincipal> valid_issuers;
   CFArrayRef valid_issuer_names = NULL;
   if (SSLCopyDistinguishedNames(ssl_context_, &valid_issuer_names) == noErr &&
@@ -758,9 +765,14 @@ void SSLClientSocketMac::GetSSLCertRequestInfo(
             << " valid issuer names";
     int n = CFArrayGetCount(valid_issuer_names);
     for (int i = 0; i < n; i++) {
-      // Parse each name into a CertPrincipal object.
       CFDataRef issuer = reinterpret_cast<CFDataRef>(
           CFArrayGetValueAtIndex(valid_issuer_names, i));
+      // Add the DER-encoded issuer DistinguishedName to |cert_authorities|.
+      cert_request_info->cert_authorities.push_back(std::string(
+          reinterpret_cast<const char*>(CFDataGetBytePtr(issuer)),
+          static_cast<size_t>(CFDataGetLength(issuer))));
+      // Add the CertPrincipal object representing the issuer to
+      // |valid_issuers|.
       CertPrincipal p;
       if (p.ParseDistinguishedName(CFDataGetBytePtr(issuer),
                                    CFDataGetLength(issuer))) {
@@ -771,8 +783,6 @@ void SSLClientSocketMac::GetSSLCertRequestInfo(
   }
 
   // Now get the available client certs whose issuers are allowed by the server.
-  cert_request_info->host_and_port = host_and_port_.ToString();
-  cert_request_info->client_certs.clear();
   // TODO(rch):  we should consider passing a host-port pair as the first
   // argument to X509Certificate::GetSSLClientCertificates.
   X509Certificate::GetSSLClientCertificates(host_and_port_.host(),
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 92f6c37..a3c1b12 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -429,9 +429,14 @@ struct HandshakeState {
   bool channel_id_sent;
 
   // If the peer requests client certificate authentication, the set of
-  // certificates that matched the peer's criteria.
+  // certificates that matched the peer's criteria. This should be soon removed
+  // as being tracked in http://crbug.com/166642.
   CertificateList client_certs;
 
+  // List of DER-encoded X.509 DistinguishedName of certificate authorities
+  // allowed by the server.
+  std::vector<std::string> cert_authorities;
+
   // Set when the handshake fully completes.
   //
   // The server certificate is first received from NSS as an NSS certificate
@@ -1301,13 +1306,21 @@ SECStatus SSLClientSocketNSS::Core::PlatformClientAuthHandler(
   }
 
   core->nss_handshake_state_.client_certs.clear();
+  core->nss_handshake_state_.cert_authorities.clear();
 
   std::vector<CERT_NAME_BLOB> issuer_list(ca_names->nnames);
   for (int i = 0; i < ca_names->nnames; ++i) {
     issuer_list[i].cbData = ca_names->names[i].len;
     issuer_list[i].pbData = ca_names->names[i].data;
+    core->nss_handshake_state_.cert_authorities.push_back(std::string(
+        reinterpret_cast<const char*>(ca_names->names[i].data),
+        static_cast<size_t>(ca_names->names[i].len)));
   }
 
+  // Retrieve the list of matching client certificates. This is to be moved out
+  // of here as a part of refactoring effort being tracked in
+  // http://crbug.com/166642.
+
   // Client certificates of the user are in the "MY" system certificate store.
   HCERTSTORE my_cert_store = CertOpenSystemStore(NULL, L"MY");
   if (!my_cert_store) {
@@ -1480,12 +1493,21 @@ SECStatus SSLClientSocketNSS::Core::PlatformClientAuthHandler(
   }
 
   core->nss_handshake_state_.client_certs.clear();
+  core->nss_handshake_state_.cert_authorities.clear();
 
-  // First, get the cert issuer names allowed by the server.
+  // Retrieve the cert issuers accepted by the server. This information is
+  // currently (temporarily) being saved both in |valid_issuers| and
+  // |cert_authorities|, the latter being the target solution. The refactoring
+  // effort is being tracked in http://crbug.com/166642.
   std::vector<CertPrincipal> valid_issuers;
   int n = ca_names->nnames;
   for (int i = 0; i < n; i++) {
-    // Parse each name into a CertPrincipal object.
+    // Add the DER-encoded issuer DistinguishedName to |cert_authorities|.
+    core->nss_handshake_state_.cert_authorities.push_back(std::string(
+        reinterpret_cast<const char*>(ca_names->names[i].data),
+        static_cast<size_t>(ca_names->names[i].len)));
+    // Add the CertPrincipal object representing the issuer to
+    // |valid_issuers|.
     CertPrincipal p;
     if (p.ParseDistinguishedName(ca_names->names[i].data,
                                  ca_names->names[i].len)) {
@@ -1562,8 +1584,8 @@ SECStatus SSLClientSocketNSS::Core::ClientAuthHandler(
   core->client_auth_cert_needed_ = !core->ssl_config_.send_client_cert;
   void* wincx  = SSL_RevealPinArg(socket);
 
-  // Second pass: a client certificate should have been selected.
   if (core->ssl_config_.send_client_cert) {
+    // Second pass: a client certificate should have been selected.
     if (core->ssl_config_.client_cert) {
       CERTCertificate* cert = CERT_DupCertificate(
           core->ssl_config_.client_cert->os_cert_handle());
@@ -1587,9 +1609,22 @@ SECStatus SSLClientSocketNSS::Core::ClientAuthHandler(
     return SECFailure;
   }
 
+  // First pass: client certificate is needed.
   core->nss_handshake_state_.client_certs.clear();
+  core->nss_handshake_state_.cert_authorities.clear();
+
+  // Retrieve the DER-encoded DistinguishedName of the cert issuers accepted by
+  // the server and save them in |cert_authorities|.
+  for (int i = 0; i < ca_names->nnames; i++) {
+    core->nss_handshake_state_.cert_authorities.push_back(std::string(
+        reinterpret_cast<const char*>(ca_names->names[i].data),
+        static_cast<size_t>(ca_names->names[i].len)));
+  }
 
-  // Iterate over all client certificates.
+  // Iterate over all client certificates and put the ones matching the server
+  // criteria in |nss_handshake_state_.client_certs|. This is to be moved out of
+  // here as a part of refactoring effort being tracked in
+  // http://crbug.com/166642.
   CERTCertList* client_certs = CERT_FindUserCertsByUsage(
       CERT_GetDefaultCertDB(), certUsageSSLClient,
       PR_FALSE, PR_FALSE, wincx);
@@ -2779,6 +2814,8 @@ void SSLClientSocketNSS::GetSSLCertRequestInfo(
   EnterFunction("");
   // TODO(rch): switch SSLCertRequestInfo.host_and_port to a HostPortPair
   cert_request_info->host_and_port = host_and_port_.ToString();
+  cert_request_info->cert_authorities = core_->state().cert_authorities;
+  // This should be removed as being tracked in http://crbug.com/166642.
   cert_request_info->client_certs = core_->state().client_certs;
   LeaveFunction(cert_request_info->client_certs.size());
 }
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index 5a03cba..2c30703 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -565,7 +565,20 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
   DCHECK(*pkey == NULL);
 
   if (!ssl_config_.send_client_cert) {
+    // First pass: we know that a client certificate is needed, but we do not
+    // have one at hand.
     client_auth_cert_needed_ = true;
+    STACK_OF(X509_NAME) *authorities = SSL_get_client_CA_list(ssl);
+    for (int i = 0; i < sk_X509_NAME_num(authorities); i++) {
+      X509_NAME *ca_name = (X509_NAME *)sk_X509_NAME_value(authorities, i);
+      unsigned char* str = NULL;
+      int length = i2d_X509_NAME(ca_name, &str);
+      cert_authorities_.push_back(std::string(
+          reinterpret_cast<const char*>(str),
+          static_cast<size_t>(length)));
+      OPENSSL_free(str);
+    }
+
     return -1;  // Suspends handshake.
   }
 
@@ -637,6 +650,7 @@ bool SSLClientSocketOpenSSL::GetSSLInfo(SSLInfo* ssl_info) {
 void SSLClientSocketOpenSSL::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
   cert_request_info->host_and_port = host_and_port_.ToString();
+  cert_request_info->cert_authorities = cert_authorities_;
   cert_request_info->client_certs = client_certs_;
 }
 
@@ -759,6 +773,7 @@ void SSLClientSocketOpenSSL::Disconnect() {
   server_cert_verify_result_.Reset();
   completed_handshake_ = false;
 
+  cert_authorities_.clear();
   client_certs_.clear();
   client_auth_cert_needed_ = false;
 }
diff --git a/net/socket/ssl_client_socket_openssl.h b/net/socket/ssl_client_socket_openssl.h
index dccdb32..b0f61c7 100644
--- a/net/socket/ssl_client_socket_openssl.h
+++ b/net/socket/ssl_client_socket_openssl.h
@@ -147,8 +147,13 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
 
   // Stores client authentication information between ClientAuthHandler and
   // GetSSLCertRequestInfo calls.
-  std::vector<scoped_refptr<X509Certificate> > client_certs_;
   bool client_auth_cert_needed_;
+  // List of DER-encoded X.509 DistinguishedName of certificate authorities
+  // allowed by the server.
+  std::vector<std::string> cert_authorities_;
+  // Set of certificates that matches the server criteria. This should be
+  // removed soon as being tracked in http://crbug.com/166642.
+  std::vector<scoped_refptr<X509Certificate> > client_certs_;
 
   CertVerifier* const cert_verifier_;
   scoped_ptr<SingleRequestCertVerifier> verifier_;
diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc
index fb65847..ab6b306 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "net/socket/ssl_client_socket.h"
 
+#include "base/memory/ref_counted.h"
 #include "net/base/address_list.h"
 #include "net/base/cert_test_util.h"
 #include "net/base/host_resolver.h"
@@ -12,6 +13,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/net_log_unittest.h"
+#include "net/base/ssl_cert_request_info.h"
 #include "net/base/ssl_config_service.h"
 #include "net/base/test_completion_callback.h"
 #include "net/base/test_data_directory.h"
@@ -936,3 +938,112 @@ TEST_F(SSLClientSocketTest, VerifyReturnChainProperlyOrdered) {
   EXPECT_FALSE(sock->IsConnected());
 }
 
+// Verifies the correctness of GetSSLCertRequestInfo.
+class SSLClientSocketCertRequestInfoTest : public SSLClientSocketTest {
+ protected:
+  // Creates a test server with the given SSLOptions, connects to it and returns
+  // the SSLCertRequestInfo reported by the socket.
+  scoped_refptr<net::SSLCertRequestInfo> GetCertRequest(
+      net::TestServer::SSLOptions ssl_options) {
+    net::TestServer test_server(net::TestServer::TYPE_HTTPS,
+                                ssl_options,
+                                FilePath());
+    if (!test_server.Start())
+      return NULL;
+
+    net::AddressList addr;
+    if (!test_server.GetAddressList(&addr))
+      return NULL;
+
+    net::TestCompletionCallback callback;
+    net::CapturingNetLog log;
+    net::StreamSocket* transport = new net::TCPClientSocket(
+        addr, &log, net::NetLog::Source());
+    int rv = transport->Connect(callback.callback());
+    if (rv == net::ERR_IO_PENDING)
+      rv = callback.WaitForResult();
+    EXPECT_EQ(net::OK, rv);
+
+    scoped_ptr<net::SSLClientSocket> sock(
+        CreateSSLClientSocket(transport, test_server.host_port_pair(),
+                              kDefaultSSLConfig));
+    EXPECT_FALSE(sock->IsConnected());
+
+    rv = sock->Connect(callback.callback());
+    if (rv == net::ERR_IO_PENDING)
+      rv = callback.WaitForResult();
+    scoped_refptr<net::SSLCertRequestInfo> request_info =
+        new net::SSLCertRequestInfo();
+    sock->GetSSLCertRequestInfo(request_info.get());
+    sock->Disconnect();
+    EXPECT_FALSE(sock->IsConnected());
+
+    return request_info;
+  }
+
+  // The following is needed to construct paths to certificates passed as
+  // |client_authorities| in server SSLOptions. Current implementation of
+  // RemoteTestServer (used on Android) expects relative paths, as opposed to
+  // LocalTestServer, which expects absolute paths (what to fix?).
+  FilePath CertDirectory() {
+#ifdef OS_ANDROID
+    return net::GetTestCertsDirectoryRelative();
+#else
+    return net::GetTestCertsDirectory();
+#endif
+  }
+};
+
+TEST_F(SSLClientSocketCertRequestInfoTest, NoAuthorities) {
+  net::TestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+  scoped_refptr<net::SSLCertRequestInfo> request_info =
+      GetCertRequest(ssl_options);
+  ASSERT_TRUE(request_info);
+  EXPECT_EQ(0u, request_info->cert_authorities.size());
+}
+
+TEST_F(SSLClientSocketCertRequestInfoTest, TwoAuthorities) {
+  const FilePath::CharType kThawteFile[] =
+      FILE_PATH_LITERAL("thawte.single.pem");
+  const unsigned char kThawteDN[] = {
+    0x30, 0x4c, 0x31, 0x0b, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x06, 0x13,
+    0x02, 0x5a, 0x41, 0x31, 0x25, 0x30, 0x23, 0x06, 0x03, 0x55, 0x04, 0x0a,
+    0x13, 0x1c, 0x54, 0x68, 0x61, 0x77, 0x74, 0x65, 0x20, 0x43, 0x6f, 0x6e,
+    0x73, 0x75, 0x6c, 0x74, 0x69, 0x6e, 0x67, 0x20, 0x28, 0x50, 0x74, 0x79,
+    0x29, 0x20, 0x4c, 0x74, 0x64, 0x2e, 0x31, 0x16, 0x30, 0x14, 0x06, 0x03,
+    0x55, 0x04, 0x03, 0x13, 0x0d, 0x54, 0x68, 0x61, 0x77, 0x74, 0x65, 0x20,
+    0x53, 0x47, 0x43, 0x20, 0x43, 0x41
+  };
+  const size_t kThawteLen = sizeof(kThawteDN);
+
+  const FilePath::CharType kDiginotarFile[] =
+      FILE_PATH_LITERAL("diginotar_root_ca.pem");
+  const unsigned char kDiginotarDN[] = {
+    0x30, 0x5f, 0x31, 0x0b, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x06, 0x13,
+    0x02, 0x4e, 0x4c, 0x31, 0x12, 0x30, 0x10, 0x06, 0x03, 0x55, 0x04, 0x0a,
+    0x13, 0x09, 0x44, 0x69, 0x67, 0x69, 0x4e, 0x6f, 0x74, 0x61, 0x72, 0x31,
+    0x1a, 0x30, 0x18, 0x06, 0x03, 0x55, 0x04, 0x03, 0x13, 0x11, 0x44, 0x69,
+    0x67, 0x69, 0x4e, 0x6f, 0x74, 0x61, 0x72, 0x20, 0x52, 0x6f, 0x6f, 0x74,
+    0x20, 0x43, 0x41, 0x31, 0x20, 0x30, 0x1e, 0x06, 0x09, 0x2a, 0x86, 0x48,
+    0x86, 0xf7, 0x0d, 0x01, 0x09, 0x01, 0x16, 0x11, 0x69, 0x6e, 0x66, 0x6f,
+    0x40, 0x64, 0x69, 0x67, 0x69, 0x6e, 0x6f, 0x74, 0x61, 0x72, 0x2e, 0x6e,
+    0x6c
+  };
+  const size_t kDiginotarLen = sizeof(kDiginotarDN);
+
+  net::TestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+  ssl_options.client_authorities.push_back(CertDirectory().Append(kThawteFile));
+  ssl_options.client_authorities.push_back(
+      CertDirectory().Append(kDiginotarFile));
+  scoped_refptr<net::SSLCertRequestInfo> request_info =
+      GetCertRequest(ssl_options);
+  ASSERT_TRUE(request_info);
+  ASSERT_EQ(2u, request_info->cert_authorities.size());
+  EXPECT_EQ(std::string(reinterpret_cast<const char*>(kThawteDN), kThawteLen),
+            request_info->cert_authorities[0]);
+  EXPECT_EQ(
+      std::string(reinterpret_cast<const char*>(kDiginotarDN), kDiginotarLen),
+      request_info->cert_authorities[1]);
+}
diff --git a/net/socket/ssl_client_socket_win.cc b/net/socket/ssl_client_socket_win.cc
index 37e5309..ecc7f0e 100644
--- a/net/socket/ssl_client_socket_win.cc
+++ b/net/socket/ssl_client_socket_win.cc
@@ -456,12 +456,13 @@ bool SSLClientSocketWin::GetSSLInfo(SSLInfo* ssl_info) {
 void SSLClientSocketWin::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
   cert_request_info->host_and_port = host_and_port_.ToString();
+  cert_request_info->cert_authorities.clear();
+  cert_request_info->cert_key_types.clear();
   cert_request_info->client_certs.clear();
 
-  // Get the certificate_authorities field of the CertificateRequest message.
-  // Schannel doesn't return the certificate_types field of the
-  // CertificateRequest message to us, so we can't filter the client
-  // certificates properly. :-(
+  // Get the server criteria for client certificates. Schannel doesn't return
+  // the certificate_types field of the CertificateRequest message to us, so we
+  // can't fill the |cert_key_types| field.
   SecPkgContext_IssuerListInfoEx issuer_list;
   SECURITY_STATUS status = QueryContextAttributes(
       &ctxt_, SECPKG_ATTR_ISSUER_LIST_EX, &issuer_list);
@@ -470,6 +471,16 @@ void SSLClientSocketWin::GetSSLCertRequestInfo(
     return;
   }
 
+  for (size_t i = 0; i < issuer_list.cIssuers; i++) {
+    cert_request_info->cert_authorities.push_back(std::string(
+        reinterpret_cast<const char*>(issuer_list.aIssuers[i].pbData),
+        static_cast<size_t>(issuer_list.aIssuers[i].cbData)));
+  }
+
+  // Retrieve the list of matching client certificates. This is to be moved out
+  // of here as a part of refactoring effort being tracked in
+  // http://crbug.com/166642.
+
   // Client certificates of the user are in the "MY" system certificate store.
   HCERTSTORE my_cert_store = CertOpenSystemStore(NULL, L"MY");
   if (!my_cert_store) {