Support for client certs in ssl_server_socket.

net/socket/ssl_server_socket_openssl.cc

 // Copyright (c) 2012 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/socket/ssl_server_socket_openssl.h"
 
 #include <openssl/err.h>
 #include <openssl/ssl.h>
 #include <utility>
 
 #include "base/callback_helpers.h"
 #include "base/logging.h"
 #include "base/strings/string_util.h"
 #include "crypto/openssl_util.h"
 #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/cert/x509_util_openssl.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 {
+
+// Creates an X509Certificate out of the concatenation of |cert|, if non-null,
+// with |chain|.
+scoped_refptr<X509Certificate> CreateX509Certificate(X509* cert,
+                                                     STACK_OF(X509) * chain) {
+  std::vector<base::StringPiece> der_chain;
+  base::StringPiece der_cert;
+  scoped_refptr<X509Certificate> client_cert;
+  if (cert) {
+    if (!x509_util::GetDER(cert, &der_cert))
+      return nullptr;
+    der_chain.push_back(der_cert);
+  }
+
+  for (size_t i = 0; i < sk_X509_num(chain); ++i) {
+    X509* x = sk_X509_value(chain, i);
+    if (x509_util::GetDER(x, &der_cert)) {

[davidben, 2016/02/04 00:40:11]

Style nit: typically errors paths are the early return when possible. So
something like:

  if (!x509_util::GetDER(x, &der_cert))
    return nullptr;
  der_chain.push_back(der_cert);

The idea is to keep the main code indented normally and have error paths out of
the way.

[ryanchung, 2016/02/05 01:56:13]

Done.

+      der_chain.push_back(der_cert);
+    } else {
+      return nullptr;
+    }
+  }
+
+  return X509Certificate::CreateFromDERCertChain(der_chain);
+}
+
+scoped_refptr<X509Certificate> GetClientCert(SSL* ssl) {
+  // The results of SSL_get_peer_certificate() must be explicitly freed
+  ScopedX509 cert(SSL_get_peer_certificate(ssl));
+  if (!cert.get())

[davidben, 2016/02/04 00:40:11]

Nit: .get() isn't needed here.

(You need it for scoped_refptr, but not scoped_ptr. It's weird.)

[ryanchung, 2016/02/05 01:56:13]

Done.

+    return nullptr;
+  // The caller does not take ownership of SSL_get_peer_cert_chain's results

[davidben, 2016/02/04 00:40:11]

Nit: period at end.

Also, yeesh! I had not realized those two APIs had completely different
ownerships. Yay OpenSSL?

[ryanchung, 2016/02/05 01:56:13]

Done.

+  STACK_OF(X509)* chain = SSL_get_peer_cert_chain(ssl);
+  return CreateX509Certificate(cert.get(), chain);
+}

[davidben, 2016/02/04 00:40:11]

So the issue is that GetClientCert doesn't let the caller tell if we're
returning null because ssl has no cert (ok), or because ssl's cert can't be
parsed (bad).

One option is to just inline this into the call site. It's pretty short anyway.

  ScopedX509 cert(SSL_get_peer_certificate(ssl_));
  if (cert) {
    client_cert_ = CreateX509Certificate(cert.get(),
SSL_get_peer_cert_chain(ssl_));
    if (!client_cert.get())
      return ERR_SSL_CLIENT_AUTH_CERT_BAD_FORMAT;
  }

[ryanchung, 2016/02/05 01:56:13]

Done.

+
+void DoNothingOnCompletion(int ignore) {}
+
+}  // namespace
+
 void EnableSSLServerSockets() {
   // No-op because CreateSSLServerSocket() calls crypto::EnsureOpenSSLInit().
 }
 
 scoped_ptr<SSLServerSocket> CreateSSLServerSocket(
     scoped_ptr<StreamSocket> socket,
     X509Certificate* certificate,
     const crypto::RSAPrivateKey& key,
-    const SSLServerConfig& ssl_config) {
+    const SSLServerConfig& ssl_server_config) {
   crypto::EnsureOpenSSLInit();
   return scoped_ptr<SSLServerSocket>(new SSLServerSocketOpenSSL(
-      std::move(socket), certificate, key, ssl_config));
+      std::move(socket), certificate, key, ssl_server_config));
 }
 
 SSLServerSocketOpenSSL::SSLServerSocketOpenSSL(
     scoped_ptr<StreamSocket> transport_socket,
     scoped_refptr<X509Certificate> certificate,
     const crypto::RSAPrivateKey& key,
-    const SSLServerConfig& ssl_config)
+    const SSLServerConfig& ssl_server_config)
     : transport_send_busy_(false),
       transport_recv_busy_(false),
       transport_recv_eof_(false),
       user_read_buf_len_(0),
       user_write_buf_len_(0),
       transport_write_error_(OK),
       ssl_(NULL),
       transport_bio_(NULL),
       transport_socket_(std::move(transport_socket)),
-      ssl_config_(ssl_config),
+      ssl_server_config_(ssl_server_config),
       cert_(certificate),
       key_(key.Copy()),
       next_handshake_state_(STATE_NONE),
       completed_handshake_(false) {
   CHECK(key_);
 }
 
 SSLServerSocketOpenSSL::~SSLServerSocketOpenSSL() {
   if (ssl_) {
     // Calling SSL_shutdown prevents the session from being marked as
@@ skipping 171 matching lines @@
   NOTIMPLEMENTED();
   return false;
 }
 
 NextProto SSLServerSocketOpenSSL::GetNegotiatedProtocol() const {
   // NPN is not supported by this class.
   return kProtoUnknown;
 }
 
 bool SSLServerSocketOpenSSL::GetSSLInfo(SSLInfo* ssl_info) {
-  NOTIMPLEMENTED();
-  return false;
+  ssl_info->Reset();
+  if (!completed_handshake_)
+    return false;
+
+  ssl_info->cert = client_cert_;
+
+  const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl_);
+  CHECK(cipher);
+  ssl_info->security_bits = SSL_CIPHER_get_bits(cipher, NULL);
+
+  SSLConnectionStatusSetCipherSuite(
+      static_cast<uint16_t>(SSL_CIPHER_get_id(cipher)),
+      &ssl_info->connection_status);
+  SSLConnectionStatusSetVersion(GetNetSSLVersion(ssl_),
+                                &ssl_info->connection_status);
+
+  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(
     ConnectionAttempts* out) const {
   out->clear();
 }
 
 int64_t SSLServerSocketOpenSSL::GetTotalReceivedBytes() const {
   return transport_socket_->GetTotalReceivedBytes();
 }
@@ skipping 302 matching lines @@
   return rv;
 }
 
 int SSLServerSocketOpenSSL::DoHandshake() {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
   int net_error = OK;
   int rv = SSL_do_handshake(ssl_);
 
   if (rv == 1) {
     completed_handshake_ = true;
+    client_cert_ = GetClientCert(ssl_);
   } else {
     int ssl_error = SSL_get_error(ssl_, rv);
     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. 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);
     } else {
       LOG(ERROR) << "handshake failed; returned " << rv
                  << ", SSL error code " << ssl_error
                  << ", net_error " << net_error;
       net_log_.AddEvent(
           NetLog::TYPE_SSL_HANDSHAKE_ERROR,
           CreateNetLogOpenSSLErrorCallback(net_error, ssl_error, error_info));
@@ skipping 24 matching lines @@
   user_write_buf_len_ = 0;
   ResetAndReturn(&user_write_callback_).Run(rv);
 }
 
 int SSLServerSocketOpenSSL::Init() {
   DCHECK(!ssl_);
   DCHECK(!transport_bio_);
 
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
 
-  ScopedSSL_CTX ssl_ctx(SSL_CTX_new(SSLv23_server_method()));
-
-  if (ssl_config_.require_client_cert)
-    SSL_CTX_set_verify(ssl_ctx.get(), SSL_VERIFY_PEER, NULL);
-
+  ScopedSSL_CTX ssl_ctx(SSL_CTX_new(TLS_method()));
+  if (ssl_server_config_.require_client_cert) {
+    SSL_CTX_set_verify(ssl_ctx.get(),
+                       SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
+                       nullptr);
+    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;
 
   BIO* ssl_bio = NULL;
   // 0 => use default buffer sizes.
   if (!BIO_new_bio_pair(&ssl_bio, 0, &transport_bio_, 0))
     return ERR_UNEXPECTED;
   DCHECK(ssl_bio);
   DCHECK(transport_bio_);
@@ skipping 26 matching lines @@
     return ERR_UNEXPECTED;
   }
 #endif  // USE_OPENSSL_CERTS
 
   DCHECK(key_->key());
   if (SSL_use_PrivateKey(ssl_, key_->key()) != 1) {
     LOG(ERROR) << "Cannot set private key.";
     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.
   SslSetClearMask options;
   options.ConfigureFlag(SSL_OP_NO_COMPRESSION, true);
 
   SSL_set_options(ssl_, options.set_mask);
   SSL_clear_options(ssl_, options.clear_mask);
 
   // Same as above, this time for the SSL mode.
   SslSetClearMask mode;
 
   mode.ConfigureFlag(SSL_MODE_RELEASE_BUFFERS, true);
 
   SSL_set_mode(ssl_, mode.set_mask);
   SSL_clear_mode(ssl_, mode.clear_mask);
 
   // See SSLServerConfig::disabled_cipher_suites for description of the suites
   // disabled by default. Note that !SHA256 and !SHA384 only remove HMAC-SHA256
   // and HMAC-SHA384 cipher suites, not GCM cipher suites with SHA256 or SHA384
   // 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(":!");
       command.append(SSL_CIPHER_get_name(cipher));
     }
   }
 
   int rv = SSL_set_cipher_list(ssl_, command.c_str());
   // If this fails (rv = 0) it means there are no ciphers enabled on this SSL.
   // This will almost certainly result in the socket failing to complete the
   // handshake at which point the appropriate error is bubbled up to the client.
   LOG_IF(WARNING, rv != 1) << "SSL_set_cipher_list('" << command
                            << "') returned " << rv;
 
+  if (ssl_server_config_.require_client_cert &&
+      !ssl_server_config_.cert_authorities_.empty()) {
+    ScopedX509NameStack stack(sk_X509_NAME_new_null());
+    for (const auto& authority : ssl_server_config_.cert_authorities_) {
+      const uint8_t* name = reinterpret_cast<const uint8_t*>(authority.c_str());
+      const uint8_t* name_start = name;
+      ScopedX509_NAME subj(d2i_X509_NAME(nullptr, &name, authority.length()));
+      if (subj && name == name_start + authority.length())

[davidben, 2016/02/04 00:40:11]

I'd probably suggest:

  if (!subj || name != name_start + authority.length())
    return ERR_UNEXPECTED;

so it's not silent.

[ryanchung, 2016/02/05 01:56:13]

Done.

+        sk_X509_NAME_push(stack.get(), subj.release());
+    }
+    SSL_set_client_CA_list(ssl_, stack.release());
+  }
+
   return OK;
 }
 
+// static
+int SSLServerSocketOpenSSL::CertVerifyCallback(X509_STORE_CTX* store_ctx,
+                                               void* arg) {
+  ClientCertVerifier* verifier = reinterpret_cast<ClientCertVerifier*>(arg);
+  // If a verifier was not supplied, all certificates are accepted.
+  if (!verifier)
+    return 1;
+  STACK_OF(X509)* chain = store_ctx->untrusted;
+  scoped_refptr<X509Certificate> client_cert(
+      CreateX509Certificate(nullptr, chain));
+  if (sk_X509_num(chain) > 0 && client_cert == nullptr) {

[davidben, 2016/02/04 00:40:11]

Does this need an X509_STORE_CTX_set_error call? If this codepath is hit, we
want to reject the certificate too.

[ryanchung, 2016/02/05 01:56:13]

Done. It seems like the most appropriate error would still be
X509_V_ERR_CERT_REJECTED? There doesn't seem to be a generic certificate error
code.

+    return 0;
+  }
+  // Asynchronous completion of Verify is currently not supported.
+  // http://crbug.com/347402
+  // The API for Verify supports the parts needed for async completion
+  // but is currently expected to complete synchronously.
+  scoped_ptr<ClientCertVerifier::Request> ignore_async;
+  int res = verifier->Verify(client_cert.get(),
+                             base::Bind(&DoNothingOnCompletion), &ignore_async);
+  DCHECK(res != ERR_IO_PENDING);

[davidben, 2016/02/04 00:40:11]

Nit: DCHECK_NE

[ryanchung, 2016/02/05 01:56:13]

Done.

+
+  if (res != OK) {
+    X509_STORE_CTX_set_error(store_ctx, X509_V_ERR_CERT_REJECTED);
+    return 0;
+  }
+  return 1;
+}
+
 }  // namespace net