Implement referred Token Bindings

net/http/http_network_transaction.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/http/http_network_transaction.h"
 
 #include <set>
 #include <utility>
 #include <vector>
 
@@ skipping 958 matching lines @@
   if (rv == OK)
     next_state_ = STATE_GET_TOKEN_BINDING_KEY;
   return rv;
 }
 
 int HttpNetworkTransaction::DoGetTokenBindingKey() {
   next_state_ = STATE_GET_TOKEN_BINDING_KEY_COMPLETE;
   if (!IsTokenBindingEnabled())
     return OK;
 
+  scoped_ptr<crypto::ECPrivateKey>* key = &provided_token_binding_key_;
+  std::string host = request_->url.host();
+  if (provided_token_binding_key_) {
+    key = &referred_token_binding_key_;
+    host = request_->token_binding_referrer;
+  }
   net_log_.BeginEvent(NetLog::TYPE_HTTP_TRANSACTION_GET_TOKEN_BINDING_KEY);
   ChannelIDService* channel_id_service = session_->params().channel_id_service;
-  return channel_id_service->GetOrCreateChannelID(
-      request_->url.host(), &token_binding_key_, io_callback_,
-      &token_binding_request_);
+  return channel_id_service->GetOrCreateChannelID(host, key, io_callback_,
+                                                  &token_binding_request_);
 }
 
 int HttpNetworkTransaction::DoGetTokenBindingKeyComplete(int rv) {
   DCHECK_NE(ERR_IO_PENDING, rv);
   next_state_ = STATE_INIT_REQUEST_BODY;
   if (!IsTokenBindingEnabled())
     return OK;
 
+  if (!request_->token_binding_referrer.empty() &&
+      !referred_token_binding_key_ && rv == OK) {
+    next_state_ = STATE_GET_TOKEN_BINDING_KEY;

[davidben, 2016/03/15 22:49:56]

Do you think this would be better done as two separate states?

[nharper, 2016/03/16 17:49:22]

Two separate states would make the state transitions clearer (it would remove
the loop), but it would also result in code duplication. If you think the state
transition clarity is worth the duplication, then I'm fine changing it.

[davidben, 2016/03/24 20:53:51]

I think have a weak preference towards the state machine clarify because there's
already so little code in those states. Also state machines are scary. But if
you disagree with me or it turns out to be a hassle because my synchronous
analogy doesn't quite align (below), feel free to override the weak preference.

My thinking is since the state machine stuff roughly mirrors synchronous code,
if you were writing it synchronously, you wouldn't write it as:

  // Earlier stuff...

get_token_binding_key:

  scoped_ptr<crypto::ECPrivateKey>* key = &provided_token_binding_key_;
  std::string host = request_->url.host();
  if (provided_token_binding_key_) {
    key = &referred_token_binding_key_;
    host = request_->token_binding_referrer;
  }

  net_log_.BeginEvent(NetLog::TYPE_HTTP_TRANSACTION_GET_TOKEN_BINDING_KEY);
  ChannelIDService* channel_id_service =
      session_->params().channel_id_service;
  int rv = channel_id_service->GetOrCreateChannelID(host, key);
  net_log_.EndEventWithNetErrorCode(
       NetLog::TYPE_HTTP_TRANSACTION_GET_TOKEN_BINDING_KEY, rv);
  if (rv < 0)
    return rv;

  if (!request_->token_binding_referrer.empty() &&
      !referred_token_binding_key_ && rv == OK) {
    goto get_token_binding_key;
  }

  // DoInitRequestBody stuff...
  
You'd probably write this as:

  // Earlier stuff...

  // Get the main token 
  net_log_.BeginEvent(NetLog::TYPE_HTTP_TRANSACTION_GET_TOKEN_BINDING_KEY);
  ChannelIDService* channel_id_service =
      session_->params().channel_id_service;
  int rv = channel_id_service->GetOrCreateChannelID(
      request_->url.host(), &provided_token_binding_key_);
  net_log_.EndEventWithNetErrorCode(
       NetLog::TYPE_HTTP_TRANSACTION_GET_TOKEN_BINDING_KEY, rv);
  if (rv < 0)
    return rv;

  if (!request_->token_binding_referrer.empty()) {
    // Get the referred token.
    net_log_.BeginEvent(NetLog::TYPE_HTTP_TRANSACTION_GET_TOKEN_BINDING_KEY);
    ChannelIDService* channel_id_service =
        session_->params().channel_id_service;
    int rv = channel_id_service->GetOrCreateChannelID(
        request_->token_binding_referrer, &referred_token_binding_key_);
    net_log_.EndEventWithNetErrorCode(
         NetLog::TYPE_HTTP_TRANSACTION_GET_TOKEN_BINDING_KEY, rv);
    if (rv < 0)
      return rv;
  }

  // DoInitRequestBody stuff...


Of course, this analogy breaks down a little because conditionals are funny and
such. I could believe duplicating the state wouldn't actually be clearer because
of that? I dunno, your call.

[nharper, 2016/03/25 01:34:29]

I've rewritten it to have more states - it makes much more sense that way than
looping back to avoid code duplication.

+  }
   net_log_.EndEventWithNetErrorCode(
       NetLog::TYPE_HTTP_TRANSACTION_GET_TOKEN_BINDING_KEY, rv);
   return rv;
 }
 
 int HttpNetworkTransaction::BuildRequestHeaders(
     bool using_http_proxy_without_tunnel) {
   request_headers_.SetHeader(HttpRequestHeaders::kHost,
                              GetHostAndOptionalPort(request_->url));
 
@@ skipping 19 matching lines @@
     // An empty POST/PUT request still needs a content length.  As for HEAD,
     // IE and Safari also add a content length header.  Presumably it is to
     // support sending a HEAD request to an URL that only expects to be sent a
     // POST or some other method that normally would have a message body.
     // Firefox (40.0) does not send the header, and RFC 7230 & 7231
     // specify that it should not be sent due to undefined behavior.
     request_headers_.SetHeader(HttpRequestHeaders::kContentLength, "0");
   }
 
   RecordTokenBindingSupport();
-  if (token_binding_key_) {
+  if (provided_token_binding_key_) {
     std::string token_binding_header;
     int rv = BuildTokenBindingHeader(&token_binding_header);
     if (rv != OK)
       return rv;
     request_headers_.SetHeader(HttpRequestHeaders::kTokenBinding,
                                token_binding_header);
   }
 
   // Honor load flags that impact proxy caches.
   if (request_->load_flags & LOAD_BYPASS_CACHE) {
@@ skipping 17 matching lines @@
     before_proxy_headers_sent_callback_.Run(proxy_info_, &request_headers_);
 
   response_.did_use_http_auth =
       request_headers_.HasHeader(HttpRequestHeaders::kAuthorization) ||
       request_headers_.HasHeader(HttpRequestHeaders::kProxyAuthorization);
   return OK;
 }
 
 int HttpNetworkTransaction::BuildTokenBindingHeader(std::string* out) {
   std::vector<uint8_t> signed_ekm;
-  int rv = stream_->GetSignedEKMForTokenBinding(token_binding_key_.get(),
-                                                &signed_ekm);
+  int rv = stream_->GetSignedEKMForTokenBinding(
+      provided_token_binding_key_.get(), &signed_ekm);
   if (rv != OK)
     return rv;
   std::string provided_token_binding;
-  rv = BuildProvidedTokenBinding(token_binding_key_.get(), signed_ekm,
-                                 &provided_token_binding);
+  rv = BuildTokenBinding(TB_TYPE_PROVIDED, provided_token_binding_key_.get(),
+                         signed_ekm, &provided_token_binding);
   if (rv != OK)
     return rv;
+
   std::vector<base::StringPiece> token_bindings;
   token_bindings.push_back(provided_token_binding);
+
+  std::string referred_token_binding;
+  if (referred_token_binding_key_) {
+    std::vector<uint8_t> referred_signed_ekm;
+    int rv = stream_->GetSignedEKMForTokenBinding(
+        referred_token_binding_key_.get(), &referred_signed_ekm);
+    if (rv != OK)
+      return rv;
+    rv = BuildTokenBinding(TB_TYPE_REFERRED, referred_token_binding_key_.get(),
+                           referred_signed_ekm, &referred_token_binding);
+    if (rv != OK)
+      return rv;
+    token_bindings.push_back(referred_token_binding);
+  }
   std::string header;
   rv = BuildTokenBindingMessageFromTokenBindings(token_bindings, &header);

[davidben, 2016/03/15 22:49:56]

This seems sort of a nuisance of a serialization scheme. The syntax allows
multiple referred TBs or provided TBs with no way to select them.

[nharper, 2016/03/16 17:49:22]

Is this comment about how the spec defines the serialization, or how I chose to
implement it?

[davidben, 2016/03/24 20:53:51]

Just the spec. Sorry, that was unclear. Not a request for changing code.

   if (rv != OK)
     return rv;
   base::Base64UrlEncode(header, base::Base64UrlEncodePolicy::INCLUDE_PADDING,
                         out);
   return OK;
 }
 
 int HttpNetworkTransaction::DoInitRequestBody() {
   next_state_ = STATE_INIT_REQUEST_BODY_COMPLETE;
   int rv = OK;
@@ skipping 464 matching lines @@
   pending_auth_target_ = HttpAuth::AUTH_NONE;
   read_buf_ = NULL;
   read_buf_len_ = 0;
   headers_valid_ = false;
   request_headers_.Clear();
   response_ = HttpResponseInfo();
   establishing_tunnel_ = false;
   remote_endpoint_ = IPEndPoint();
   net_error_details_.quic_broken = false;
   net_error_details_.quic_connection_error = QUIC_NO_ERROR;
-  token_binding_key_.reset();
+  provided_token_binding_key_.reset();
+  referred_token_binding_key_.reset();
 }
 
 void HttpNetworkTransaction::CacheNetErrorDetailsAndResetStream() {
   if (stream_)
     stream_->PopulateNetErrorDetails(&net_error_details_);
   stream_.reset();
 }
 
 void HttpNetworkTransaction::RecordSSLFallbackMetrics(int result) {
   if (result != OK && result != ERR_SSL_INAPPROPRIATE_FALLBACK)
@@ skipping 210 matching lines @@
   DCHECK(stream_request_);
 
   // Since the transaction can restart with auth credentials, it may create a
   // stream more than once. Accumulate all of the connection attempts across
   // those streams by appending them to the vector:
   for (const auto& attempt : stream_request_->connection_attempts())
     connection_attempts_.push_back(attempt);
 }
 
 }  // namespace net