Support for client certs in ssl_server_socket.

net/socket/ssl_server_socket_openssl.cc

Index: net/socket/ssl_server_socket_openssl.cc
diff --git a/net/socket/ssl_server_socket_openssl.cc b/net/socket/ssl_server_socket_openssl.cc
index fe3a3694069ef21c987acd99c6d463b7527f9768..030cda5782ccdd7b39dcb44785a2dc279072c6a6 100644
--- a/net/socket/ssl_server_socket_openssl.cc
+++ b/net/socket/ssl_server_socket_openssl.cc
@@ -14,13 +14,61 @@
 #include "crypto/rsa_private_key.h"
 #include "crypto/scoped_openssl_types.h"
 #include "net/base/net_errors.h"
+#include "net/cert/cert_verify_result.h"
+#include "net/cert/client_cert_verifier.h"
 #include "net/ssl/openssl_ssl_util.h"
 #include "net/ssl/scoped_openssl_types.h"
+#include "net/ssl/ssl_connection_status_flags.h"
+#include "net/ssl/ssl_info.h"
 
 #define GotoState(s) next_handshake_state_ = s
 
 namespace net {
 
+namespace {
+
+bool GetDERFromX509(X509* cert, base::StringPiece* sp) {

[davidben, 2015/12/14 23:56:49]

Use net::x509_util::GetDER from x509_util_openssl.h.

[ryanchung, 2015/12/16 22:40:01]

Done.

+  unsigned char* cert_data = NULL;

[davidben, 2015/12/14 23:56:49]

uint8_t

[ryanchung, 2015/12/16 22:40:01]

Done.

+  int cert_data_length = i2d_X509(cert, &cert_data);
+  if (!cert_data_length || !cert_data) {
+    return false;
+  }
+  sp->set(reinterpret_cast<char*>(cert_data), cert_data_length);
+  return true;
+}
+
+scoped_refptr<X509Certificate> CreateX509Certificate(X509* cert,
+                                                     STACK_OF(X509) * chain) {
+  DCHECK(cert);
+  std::vector<base::StringPiece> der_chain;
+  base::StringPiece der_cert;
+  scoped_refptr<X509Certificate> client_cert;
+  if (!GetDERFromX509(cert, &der_cert))
+    return client_cert;
+  der_chain.push_back(der_cert);
+
+  ScopedX509Stack openssl_chain(X509_chain_up_ref(chain));
+  for (size_t i = 0; i < sk_X509_num(openssl_chain.get()); ++i) {
+    X509* x = sk_X509_value(openssl_chain.get(), i);
+    if (GetDERFromX509(x, &der_cert)) {
+      der_chain.push_back(der_cert);
+    }
+  }
+
+  client_cert = X509Certificate::CreateFromDERCertChain(der_chain);
+
+  for (size_t i = 0; i < der_chain.size(); ++i) {
+    OPENSSL_free(const_cast<char*>(der_chain[i].data()));
+  }
+  if (der_chain.size() - 1 !=
+      static_cast<size_t>(sk_X509_num(openssl_chain.get()))) {
+    client_cert = NULL;

[davidben, 2015/12/14 23:56:49]

nullptr

[ryanchung, 2015/12/16 22:40:01]

Done.

+  }
+  return client_cert;
+}
+
+}  // namespace
+
 void EnableSSLServerSockets() {
   // No-op because CreateSSLServerSocket() calls crypto::EnsureOpenSSLInit().
 }
@@ -29,17 +77,17 @@ scoped_ptr<SSLServerSocket> CreateSSLServerSocket(
     scoped_ptr<StreamSocket> socket,
     X509Certificate* certificate,
     crypto::RSAPrivateKey* key,
-    const SSLServerConfig& ssl_config) {
+    const SSLServerConfig& ssl_server_config) {
   crypto::EnsureOpenSSLInit();
-  return scoped_ptr<SSLServerSocket>(
-      new SSLServerSocketOpenSSL(socket.Pass(), certificate, key, ssl_config));
+  return scoped_ptr<SSLServerSocket>(new SSLServerSocketOpenSSL(
+      socket.Pass(), certificate, key, ssl_server_config));
 }
 
 SSLServerSocketOpenSSL::SSLServerSocketOpenSSL(
     scoped_ptr<StreamSocket> transport_socket,
     scoped_refptr<X509Certificate> certificate,
     crypto::RSAPrivateKey* key,
-    const SSLServerConfig& ssl_config)
+    const SSLServerConfig& ssl_server_config)
     : transport_send_busy_(false),
       transport_recv_busy_(false),
       transport_recv_eof_(false),
@@ -49,7 +97,7 @@ SSLServerSocketOpenSSL::SSLServerSocketOpenSSL(
       ssl_(NULL),
       transport_bio_(NULL),
       transport_socket_(transport_socket.Pass()),
-      ssl_config_(ssl_config),
+      ssl_server_config_(ssl_server_config),
       cert_(certificate),
       next_handshake_state_(STATE_NONE),
       completed_handshake_(false) {
@@ -244,8 +292,31 @@ NextProto SSLServerSocketOpenSSL::GetNegotiatedProtocol() const {
 }
 
 bool SSLServerSocketOpenSSL::GetSSLInfo(SSLInfo* ssl_info) {
-  NOTIMPLEMENTED();
-  return false;
+  ssl_info->Reset();
+  if (!completed_handshake_) {
+    return false;
+  }
+  ExtractClientCert();
+  ssl_info->cert = client_cert_;
+  ssl_info->client_cert_sent =
+      ssl_server_config_.require_client_cert && client_cert_.get();

[davidben, 2015/12/14 23:56:49]

Just client_cert_.get() seems right. Alternatively, I might just leave it false
since we didn't send a client cert. We received one.

[ryanchung, 2015/12/16 22:40:01]

Done. Leaving it false. We can know whether we received one by checking
client_cert_ .

+
+  const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl_);
+  CHECK(cipher);
+  ssl_info->security_bits = SSL_CIPHER_get_bits(cipher, NULL);
+
+  ssl_info->connection_status =
+      EncodeSSLConnectionStatus(SSL_CIPHER_get_id(cipher),
+                                0 /* no compression */, GetNetSSLVersion(ssl_));
+
+  if (!SSL_get_secure_renegotiation_support(ssl_))
+    ssl_info->connection_status |= SSL_CONNECTION_NO_RENEGOTIATION_EXTENSION;
+
+  ssl_info->handshake_type = SSL_session_reused(ssl_)
+                                 ? SSLInfo::HANDSHAKE_RESUME
+                                 : SSLInfo::HANDSHAKE_FULL;
+
+  return true;
 }
 
 void SSLServerSocketOpenSSL::GetConnectionAttempts(
@@ -573,6 +644,13 @@ int SSLServerSocketOpenSSL::DoHandshake() {
     OpenSSLErrorInfo error_info;
     net_error = MapOpenSSLErrorWithDetails(ssl_error, err_tracer, &error_info);
 
+    // This hack is necessary because the mapping of SSL error codes to
+    // net_errors assumes (correctly for client sockets, but erroneously for
+    // server sockets) that peer cert verification failure can only occur if
+    // the cert changed during a renego.

[davidben, 2015/12/14 23:56:49]

Want to file a bug about this? This is sort of a silly situation and seems
fixable.

[ryanchung, 2015/12/16 22:40:01]

Done. Created http://crbug.com/570351

+    if (net_error == ERR_SSL_SERVER_CERT_CHANGED)
+      net_error = ERR_BAD_SSL_CLIENT_AUTH_CERT;
+
     // If not done, stay in this state
     if (net_error == ERR_IO_PENDING) {
       GotoState(STATE_HANDSHAKE);
@@ -618,10 +696,10 @@ int SSLServerSocketOpenSSL::Init() {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
 
   ScopedSSL_CTX ssl_ctx(SSL_CTX_new(SSLv23_server_method()));
-
-  if (ssl_config_.require_client_cert)
+  if (ssl_server_config_.require_client_cert)
     SSL_CTX_set_verify(ssl_ctx.get(), SSL_VERIFY_PEER, NULL);
-
+  SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), CertVerifyCallback,
+                                   ssl_server_config_.client_cert_verifier);
   ssl_ = SSL_new(ssl_ctx.get());
   if (!ssl_)
     return ERR_UNEXPECTED;
@@ -668,10 +746,10 @@ int SSLServerSocketOpenSSL::Init() {
     return ERR_UNEXPECTED;
   }
 
-  DCHECK_LT(SSL3_VERSION, ssl_config_.version_min);
-  DCHECK_LT(SSL3_VERSION, ssl_config_.version_max);
-  SSL_set_min_version(ssl_, ssl_config_.version_min);
-  SSL_set_max_version(ssl_, ssl_config_.version_max);
+  DCHECK_LT(SSL3_VERSION, ssl_server_config_.version_min);
+  DCHECK_LT(SSL3_VERSION, ssl_server_config_.version_max);
+  SSL_set_min_version(ssl_, ssl_server_config_.version_min);
+  SSL_set_max_version(ssl_, ssl_server_config_.version_max);
 
   // OpenSSL defaults some options to on, others to off. To avoid ambiguity,
   // set everything we care about to an absolute value.
@@ -695,11 +773,11 @@ int SSLServerSocketOpenSSL::Init() {
   // as the handshake hash.
   std::string command("DEFAULT:!SHA256:!SHA384:!AESGCM+AES256:!aPSK");
 
-  if (ssl_config_.require_ecdhe)
+  if (ssl_server_config_.require_ecdhe)
     command.append(":!kRSA:!kDHE");
 
   // Remove any disabled ciphers.
-  for (uint16_t id : ssl_config_.disabled_cipher_suites) {
+  for (uint16_t id : ssl_server_config_.disabled_cipher_suites) {
     const SSL_CIPHER* cipher = SSL_get_cipher_by_value(id);
     if (cipher) {
       command.append(":!");
@@ -714,7 +792,54 @@ int SSLServerSocketOpenSSL::Init() {
   LOG_IF(WARNING, rv != 1) << "SSL_set_cipher_list('" << command
                            << "') returned " << rv;
 
+  if (ssl_server_config_.require_client_cert) {
+    if (ssl_server_config_.client_cert_verifier)
+      ssl_->verify_mode |= SSL_VERIFY_FAIL_IF_NO_PEER_CERT;

[davidben, 2015/12/14 23:56:49]

Please don't reach into BoringSSL's internal structs. Call the public APIs.

https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#C...

Also why are these separate from line 701? Seems you could configure it on the
SSL_CTX just fine too. (Also I suspect you want SSL_VERIFY_FAIL_IF_NO_PEER_CERT
in the require_client_cert case too. Rejecting that was probably an oversight.)

[ryanchung, 2015/12/16 22:40:01]

Done.

+    if (!ssl_server_config_.client_cert_ca_list.empty()) {
+      ScopedX509NameStack stack(sk_X509_NAME_new_null());
+      for (const auto& certificate : ssl_server_config_.client_cert_ca_list) {
+        ScopedX509 ca_cert =
+            OSCertHandleToOpenSSL(certificate->os_cert_handle());
+        ScopedX509Name subj(X509_NAME_dup(ca_cert->cert_info->subject));
+        sk_X509_NAME_push(stack.get(), subj.release());
+      }
+      SSL_set_client_CA_list(ssl_, stack.release());
+    }
+  }
+
   return OK;
 }
 
+void SSLServerSocketOpenSSL::ExtractClientCert() {
+  if (client_cert_.get() || !completed_handshake_) {
+    return;

[davidben, 2015/12/14 23:56:49]

Rather than being a function that caches stuff and has to make sure it's called
at the right time, just extract the certificate once when the handshake is done.

[ryanchung, 2015/12/16 22:40:01]

Done. Will call this function only after handshake.

+  }
+  X509* cert = SSL_get_peer_certificate(ssl_);
+  STACK_OF(X509)* chain = SSL_get_peer_cert_chain(ssl_);

[davidben, 2015/12/14 23:56:49]

NB: This is NOT the verified chain. It is the list of certificates that the
client sent you that may or may not be the chain your verifier ended up
building.

+  client_cert_ = CreateX509Certificate(cert, chain);
+}
+
+// static
+int SSLServerSocketOpenSSL::CertVerifyCallback(X509_STORE_CTX* store_ctx,
+                                               void* arg) {
+  ClientCertVerifier* verifier = reinterpret_cast<ClientCertVerifier*>(arg);
+  DCHECK(verifier);

[davidben, 2015/12/14 23:56:49]

This DCHECK is false. You sometimes install NULL ones. I'd replace it with a
comment:

  // If no ClientCertVerifier was supplied, all certificates are accepted.

[ryanchung, 2015/12/16 22:40:01]

Done.

+  if (!verifier)
+    return 1;
+  SSL* ssl = reinterpret_cast<SSL*>(X509_STORE_CTX_get_ex_data(
+      store_ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
+  DCHECK(ssl);
+  X509* x = store_ctx->cert;
+  STACK_OF(X509)* chain = store_ctx->chain;

[davidben, 2015/12/14 23:56:49]

I believe this is always NULL. You want store_ctx->untrusted because you have
not verified a chain yet at this point. Also, store_ctx->untrusted already
includes the leaf.

[ryanchung, 2015/12/16 22:40:01]

Done.

+  scoped_refptr<X509Certificate> client_cert(CreateX509Certificate(x, chain));
+
+  int res = verifier->Verify(client_cert.get());
+  if (res == OK) {
+    return 1;
+  } else {
+    X509_STORE_CTX_set_error(store_ctx, X509_V_ERR_CERT_REJECTED);

[davidben, 2015/12/14 23:56:49]

Optional: If you also want to preserve res, OpenSSLPutNetError will make the
error code come back out of MapOpenSSLErrorWithDetails.

+    return 0;
+  }
+}
+
 }  // namespace net