Implement Token Binding negotiation TLS extension

net/socket/ssl_client_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.
 
 // OpenSSL binding for SSLClientSocket. The class layout and general principle
 // of operation is derived from SSLClientSocketNSS.
 
 #include "net/socket/ssl_client_socket_openssl.h"
 
 #include <errno.h>
 #include <openssl/bio.h>
+#include <openssl/bytestring.h>
 #include <openssl/err.h>
+#include <openssl/evp.h>
 #include <openssl/mem.h>
 #include <openssl/ssl.h>
 #include <string.h>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/environment.h"
 #include "base/lazy_instance.h"
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 230 matching lines @@
         !ssl_keylog_file.empty()) {
       crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
       BIO* bio = BIO_new_file(ssl_keylog_file.c_str(), "a");
       if (!bio) {
         LOG(ERROR) << "Failed to open " << ssl_keylog_file;
         ERR_print_errors_cb(&LogErrorCallback, NULL);
       } else {
         SSL_CTX_set_keylog_bio(ssl_ctx_.get(), bio);
       }
     }
+
+    if (!TokenBindingExtension::RegisterCallbacks(ssl_ctx_.get())) {
+      DLOG(ERROR) << "Failed to register calllbacks for token binding";

[mattm, 2015/09/24 22:13:12]

dcheck?

[nharper, 2015/09/28 21:43:39]

Done.

+    }
   }
 
   static int ClientCertRequestCallback(SSL* ssl, void* arg) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     DCHECK(socket);
     return socket->ClientCertRequestCallback(ssl);
   }
 
   static int CertVerifyCallback(X509_STORE_CTX *store_ctx, void *arg) {
     SSL* ssl = reinterpret_cast<SSL*>(X509_STORE_CTX_get_ex_data(
@@ skipping 58 matching lines @@
   int ssl_socket_data_index_;
 
   ScopedSSL_CTX ssl_ctx_;
 
   // TODO(davidben): Use a separate cache per URLRequestContext.
   // https://crbug.com/458365
   //
   // TODO(davidben): Sessions should be invalidated on fatal
   // alerts. https://crbug.com/466352
   SSLClientSessionCacheOpenSSL session_cache_;
+  TokenBindingExtension token_binding_extension_;

[mattm, 2015/09/24 22:13:13]

unused?

[nharper, 2015/09/28 21:43:39]

It's used in several places below.

[mattm, 2015/09/28 23:07:25]

Those all look like references to
SSLClientSocketOpenSSL::token_binding_extension_, does anything use
SSLClientSocketOpenSSL::SSLContext::token_binding_extension_?

[nharper, 2015/09/28 23:18:33]

Sorry, I didn't notice that this was in SSLContext. It is indeed unused here.

 };
 
 const SSL_PRIVATE_KEY_METHOD
     SSLClientSocketOpenSSL::SSLContext::kPrivateKeyMethod = {
         &SSLClientSocketOpenSSL::SSLContext::PrivateKeyTypeCallback,
         &SSLClientSocketOpenSSL::SSLContext::PrivateKeySupportsDigestCallback,
         &SSLClientSocketOpenSSL::SSLContext::PrivateKeyMaxSignatureLenCallback,
         &SSLClientSocketOpenSSL::SSLContext::PrivateKeySignCallback,
         &SSLClientSocketOpenSSL::SSLContext::PrivateKeySignCompleteCallback,
 };
@@ skipping 199 matching lines @@
 
   net_log_.BeginEvent(NetLog::TYPE_SSL_CONNECT);
 
   // Set up new ssl object.
   int rv = Init();
   if (rv != OK) {
     net_log_.EndEventWithNetErrorCode(NetLog::TYPE_SSL_CONNECT, rv);
     return rv;
   }
 
+  // Set params for TokenBindingExtension object.
+  if (IsTokenBindingEnabled(ssl_config_, channel_id_service_)) {
+    token_binding_extension_.SetParams(&ssl_config_.token_binding_params);
+  }
+
   // Set SSL to client mode. Handshake happens in the loop below.
   SSL_set_connect_state(ssl_);
 
   GotoState(STATE_HANDSHAKE);
   rv = DoHandshakeLoop(OK);
   if (rv == ERR_IO_PENDING) {
     user_connect_callback_ = callback;
   } else {
     net_log_.EndEventWithNetErrorCode(NetLog::TYPE_SSL_CONNECT, rv);
   }
@@ skipping 143 matching lines @@
   ssl_info->cert = server_cert_verify_result_.verified_cert;
   ssl_info->unverified_cert = server_cert_;
   ssl_info->cert_status = server_cert_verify_result_.cert_status;
   ssl_info->is_issued_by_known_root =
       server_cert_verify_result_.is_issued_by_known_root;
   ssl_info->public_key_hashes =
     server_cert_verify_result_.public_key_hashes;
   ssl_info->client_cert_sent =
       ssl_config_.send_client_cert && ssl_config_.client_cert.get();
   ssl_info->channel_id_sent = channel_id_sent_;
+  ssl_info->token_binding_negotiated = token_binding_extension_.WasNegotiated();
   ssl_info->pinning_failure_log = pinning_failure_log_;
 
   AddSCTInfoToSSLInfo(ssl_info);
 
   const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl_);
   CHECK(cipher);
   ssl_info->security_bits = SSL_CIPHER_get_bits(cipher, NULL);
   ssl_info->key_exchange_info =
       SSL_SESSION_get_key_exchange_info(SSL_get_session(ssl_));
 
@@ skipping 369 matching lines @@
 
 int SSLClientSocketOpenSSL::DoHandshakeComplete(int result) {
   if (result < 0)
     return result;
 
   if (ssl_config_.version_fallback &&
       ssl_config_.version_max < ssl_config_.version_fallback_min) {
     return ERR_SSL_FALLBACK_BEYOND_MINIMUM_VERSION;
   }
 
+  // Check that if token binding was negotiated, then extended master secret
+  // must also be negotiated.
+  if (token_binding_extension_.WasNegotiated() &&
+      !ssl_->session->extended_master_secret) {
+    return ERR_SSL_PROTOCOL_ERROR;
+  }
+
+  if (token_binding_extension_.WasNegotiated() && !channel_id_key_) {
+    GotoState(STATE_TOKEN_BINDING_LOOKUP);
+    return OK;
+  }
+
   // SSL handshake is completed. If NPN wasn't negotiated, see if ALPN was.
   if (npn_status_ == kNextProtoUnsupported) {
     const uint8_t* alpn_proto = NULL;
     unsigned alpn_len = 0;
     SSL_get0_alpn_selected(ssl_, &alpn_proto, &alpn_len);
     if (alpn_len > 0) {
       npn_proto_.assign(reinterpret_cast<const char*>(alpn_proto), alpn_len);
       npn_status_ = kNextProtoNegotiated;
       set_negotiation_extension(kExtensionALPN);
     }
@@ skipping 58 matching lines @@
     return MapOpenSSLError(err, err_tracer);
   }
 
   // Return to the handshake.
   channel_id_sent_ = true;
   net_log_.AddEvent(NetLog::TYPE_SSL_CHANNEL_ID_PROVIDED);
   GotoState(STATE_HANDSHAKE);
   return OK;
 }
 
+int SSLClientSocketOpenSSL::DoTokenBindingLookup() {
+  net_log_.AddEvent(NetLog::TYPE_SSL_CHANNEL_ID_REQUESTED);

[mattm, 2015/09/24 22:13:12]

(I'm not sure why the netlogging for channel id is different in the openssl and
nss files.)

I think the channel_id_service lookups should have begin event
TYPE_SSL_GET_DOMAIN_BOUND_CERT (and a matching end event when the lookup
finishes).

Then there should be a new TYPE_SSL_TOKEN_BINDING_REQUESTED which is done when
the extension is negotiated even if channel_id_key_ is already initialized, and
a TOKEN_BINDING_PROVIDED when it's used 

[nharper, 2015/09/28 21:43:38]

(Both nss and openssl use TYPE_SSL_CHANNEL_ID_REQUESTED/PROVIDED; nss also calls
NetLog::BeginEvent/EndEventWithNetErrorCode for TYPE_SSL_GET_DOMAIN_BOUND_CERT.)

I added a new event TYPE_SSL_GET_TOKEN_BINDING_KEY and call BeginEvent and
EndEventWithNetErrorCode for it when we do the key lookup.

+  GotoState(STATE_TOKEN_BINDING_LOOKUP_COMPLETE);
+  return channel_id_service_->GetOrCreateChannelID(
+      host_and_port_.host(), &channel_id_key_,
+      base::Bind(&SSLClientSocketOpenSSL::OnHandshakeIOComplete,
+                 base::Unretained(this)),
+      &channel_id_request_);
+}
+
+int SSLClientSocketOpenSSL::DoTokenBindingLookupComplete(int result) {
+  if (result < 0)
+    return result;
+
+  if (!channel_id_key_) {
+    LOG(ERROR) << "Failed to import Channel ID.";
+    return ERR_CHANNEL_ID_IMPORT_FAILED;
+  }
+
+  GotoState(STATE_HANDSHAKE_COMPLETE);
+  return OK;
+}
+
 int SSLClientSocketOpenSSL::DoVerifyCert(int result) {
   DCHECK(!server_cert_chain_->empty());
   DCHECK(start_cert_verification_time_.is_null());
 
   GotoState(STATE_VERIFY_CERT_COMPLETE);
 
   // OpenSSL decoded the certificate, but the platform certificate
   // implementation could not. This is treated as a fatal SSL-level protocol
   // error rather than a certificate error. See https://crbug.com/91341.
   if (!server_cert_.get())
@@ skipping 210 matching lines @@
       case STATE_HANDSHAKE_COMPLETE:
         rv = DoHandshakeComplete(rv);
         break;
       case STATE_CHANNEL_ID_LOOKUP:
         DCHECK_EQ(OK, rv);
         rv = DoChannelIDLookup();
        break;
       case STATE_CHANNEL_ID_LOOKUP_COMPLETE:
         rv = DoChannelIDLookupComplete(rv);
         break;
+      case STATE_TOKEN_BINDING_LOOKUP:
+        DCHECK_EQ(OK, rv);
+        rv = DoTokenBindingLookup();
+        break;
+      case STATE_TOKEN_BINDING_LOOKUP_COMPLETE:
+        rv = DoTokenBindingLookupComplete(rv);
+        break;
       case STATE_VERIFY_CERT:
         DCHECK_EQ(OK, rv);
         rv = DoVerifyCert(rv);
        break;
       case STATE_VERIFY_CERT_COMPLETE:
         rv = DoVerifyCertComplete(rv);
         break;
       case STATE_NONE:
       default:
         rv = ERR_UNEXPECTED;
@@ skipping 708 matching lines @@
   if (next_handshake_state_ == STATE_HANDSHAKE) {
     OnHandshakeIOComplete(signature_result_);
     return;
   }
 
   // During a renegotiation, either Read or Write calls may be blocked on an
   // asynchronous private key operation.
   PumpReadWriteEvents();
 }
 
+TokenBindingExtension::TokenBindingExtension()
+    : negotiated_(false), negotiated_param_(TokenBindingParam(0)) {}
+
+TokenBindingExtension::~TokenBindingExtension() {}
+
+bool TokenBindingExtension::WasNegotiated() const {
+  return negotiated_;
+}
+
+TokenBindingParam TokenBindingExtension::NegotiationResult() const {
+  return negotiated_param_;
+};
+
+void TokenBindingExtension::SetParams(std::vector<TokenBindingParam>* params) {

[mattm, 2015/09/24 22:13:12]

take a const ref

[nharper, 2015/09/28 21:43:38]

Done.

+  supported_params_.clear();
+  for (TokenBindingParam param : *params) {
+    supported_params_.push_back(param);
+  }
+};
+
+// static
+int TokenBindingExtension::RegisterCallbacks(SSL_CTX* ssl_ctx) {
+  return SSL_CTX_add_client_custom_ext(
+      ssl_ctx, kExtNum, &TokenBindingExtension::ClientAddCallback,
+      &TokenBindingExtension::ClientFreeCallback, nullptr,
+      &TokenBindingExtension::ClientParseCallback, nullptr);
+}
+
+int TokenBindingExtension::ClientAdd(SSL* s,
+                                     unsigned int ext_type,
+                                     const unsigned char** out,

[davidben, 2015/09/25 21:51:50]

Nit: uint8_t

[nharper, 2015/09/28 21:43:39]

Done. I've gone through and changed these signatures to match boringssl (I
originally copied the signatures, variable names included, from openssl).

+                                     size_t* outlen,
+                                     int* al) {

[mattm, 2015/09/24 22:13:12]

al?

[nharper, 2015/09/28 21:43:39]

out_alert_value. Renamed.

+  if (supported_params_.empty()) {
+    return 0;
+  }
+  if (ext_type != kExtNum) {

[mattm, 2015/09/24 22:13:12]

I'm not familiar enough with the boringssl api here, but should these ext_type
tests be DCHECKs?

[davidben, 2015/09/25 21:51:50]

Yeah, I think DCHECK is right. It shouldn't be called with the wrong one. (We
inherited the API from OpenSSL. I guess the idea is you might reuse the callback
for multiple xtns?)

[nharper, 2015/09/28 21:43:39]

Done.

+    return 0;
+  }
+  CBB output, parameters_list;
+  int retval = 0;
+  if (!CBB_init(&output, 7))
+    goto end;

[davidben, 2015/09/25 21:51:50]

Optional: Here's a scoper for you to save on the gotos.

class ScopedCBB {
 public:
  ScopedCBB() { CBB_zero(&cbb_); }
  ~ScopedCBB() { CBB_cleanup(&cbb_); }

  CBB* get() { return &cbb_; }

 private:
  CBB cbb_;

  DISALLOW_COPY_AND_ASSIGN(ScopedCBB);
};

[nharper, 2015/09/28 21:43:39]

Would it be appropriate to put this scoper in //crypto/scoped_openssl_types.h,
or should it go somewhere else?

+  if (!CBB_add_u8(&output, kProtocolVersionMajor) ||
+      !CBB_add_u8(&output, kProtocolVersionMinor) ||
+      !CBB_add_u8_length_prefixed(&output, &parameters_list)) {
+    goto end;
+  }
+  for (size_t i = 0; i < supported_params_.size(); ++i) {
+    if (!CBB_add_u8(&parameters_list, supported_params_[i])) {
+      goto end;
+    }
+  }
+  if (!CBB_finish(&output, const_cast<uint8_t**>(out), outlen))
+    goto end;
+
+  retval = 1;
+
+end:
+  CBB_cleanup(&output);
+  return retval;
+}
+
+int TokenBindingExtension::ClientParse(SSL* s,
+                                       unsigned int ext_type,
+                                       const unsigned char* in,

[davidben, 2015/09/25 21:51:50]

Nit: uint8_t

[nharper, 2015/09/28 21:43:39]

Done.

+                                       size_t inlen,
+                                       int* al) {
+  if (ext_type != kExtNum) {
+    return 0;
+  }
+  CBS in_ext;
+  CBS_init(&in_ext, in, inlen);
+
+  CBS parameters_list;
+  uint8_t version_major, version_minor, param;
+  if (!CBS_get_u8(&in_ext, &version_major) ||
+      !CBS_get_u8(&in_ext, &version_minor) ||
+      version_major != kProtocolVersionMajor ||
+      version_minor != kProtocolVersionMinor ||
+      !CBS_get_u8_length_prefixed(&in_ext, &parameters_list)) {

[davidben, 2015/09/25 21:51:50]

Also reject CBS_len(&in_ext) != 0 to reject extra bits.

[nharper, 2015/09/28 21:43:38]

Done.

+    return 0;

[davidben, 2015/09/25 21:51:50]

*al = SSL_AD_DECODE_ERROR;

(This is actually the default, but it's not documented as such, so being
explicit is probably good.)

[nharper, 2015/09/28 21:43:38]

Done.

+  }
+
+  if (!CBS_get_u8(&parameters_list, &param))
+    return 0;

[davidben, 2015/09/25 21:51:50]

Optional: I would probably fold this line and the CBS_len check into the pile of
||s above.

[nharper, 2015/09/28 21:43:39]

Done.

+  bool valid_param = false;
+  for (size_t i = 0; i < supported_params_.size(); ++i) {
+    if (param == supported_params_[i]) {
+      negotiated_param_ = TokenBindingParam(param);
+      valid_param = true;
+      break;
+    }
+  }
+  if (CBS_len(&parameters_list) > 0 || !valid_param)

[mattm, 2015/09/24 22:13:12]

Add a comment about why you don't need to check the rest of the parameters_list

[davidben, 2015/09/25 21:51:50]

If valid_param is false, we probably want to emit SSL_AD_ILLEGAL_PARAMETER
rather than SSL_AD_DECODE_ERROR. Not that it really matters at all.

[davidben, 2015/09/25 21:51:50]

parameters_list is only ever supposed to have one element for a client.

[mattm, 2015/09/25 22:01:30]

Right, I just meant there should be a comment stating that. Otherwise if you're
just reading over the code it looks rather weird.

[nharper, 2015/09/28 21:43:39]

Done.

+    return 0;

[mattm, 2015/09/24 22:13:12]

Is setting an appropriate value to *al (out_alert_value) when returning an error
required or optional for this function?

[davidben, 2015/09/25 21:51:50]

*al is only processed on failure, so it can be left alone.

[mattm, 2015/09/25 22:01:30]

I was asking about the failure cases specifically, but it looks like you
addressed that in comments above.

[davidben, 2015/09/25 22:04:25]

Right, sorry, misread that. :-)

+  negotiated_ = true;
+  return 1;
+}
+
+// static
+int TokenBindingExtension::ClientAddCallback(SSL* s,
+                                             unsigned int ext_type,
+                                             const unsigned char** out,
+                                             size_t* outlen,
+                                             int* al,
+                                             void* add_arg) {
+  SSLClientSocketOpenSSL* socket =
+      SSLClientSocketOpenSSL::SSLContext::GetInstance()->GetClientSocketFromSSL(
+          s);
+  return socket->token_binding_extension_.ClientAdd(s, ext_type, out, outlen,
+                                                    al);
+}
+
+// static
+void TokenBindingExtension::ClientFreeCallback(SSL* s,
+                                               unsigned ext_type,
+                                               const unsigned char* out,
+                                               void* add_arg) {
+  OPENSSL_free(const_cast<unsigned char*>(out));
+}
+
+// static
+int TokenBindingExtension::ClientParseCallback(SSL* s,
+                                               unsigned int ext_type,
+                                               const unsigned char* in,
+                                               size_t inlen,
+                                               int* al,
+                                               void* parse_arg) {
+  SSLClientSocketOpenSSL* socket =
+      SSLClientSocketOpenSSL::SSLContext::GetInstance()->GetClientSocketFromSSL(
+          s);
+  return socket->token_binding_extension_.ClientParse(s, ext_type, in, inlen,
+                                                      al);
+}
+
 }  // namespace net