SSLClientSessionCache: Log number of times Lookup is called per Session.

net/socket/ssl_client_socket_impl.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_client_socket_impl.h"
 
 #include <errno.h>
 #include <string.h>
 
 #include <algorithm>
@@ skipping 926 matching lines @@
   // 6066, Section 3).
   //
   // TODO(rsleevi): Should this code allow hostnames that violate the LDH rule?
   // See https://crbug.com/496472 and https://crbug.com/496468 for discussion.
   IPAddress unused;
   if (!unused.AssignFromIPLiteral(host_and_port_.host()) &&
       !SSL_set_tlsext_host_name(ssl_.get(), host_and_port_.host().c_str())) {
     return ERR_UNEXPECTED;
   }
 
-  bssl::UniquePtr<SSL_SESSION> session =
-      context->session_cache()->Lookup(GetSessionCacheKey());
+  bssl::UniquePtr<SSL_SESSION> session = context->session_cache()->Lookup(
+      GetSessionCacheKey(), &ssl_session_cache_lookup_count_);
   if (session)
     SSL_set_session(ssl_.get(), session.get());
 
   transport_adapter_.reset(new SocketBIOAdapter(
       transport_->socket(), GetBufferSize("SSLBufferSizeRecv"),
       GetBufferSize("SSLBufferSizeSend"), this));
   BIO* transport_bio = transport_adapter_->bio();
 
   BIO_up_ref(transport_bio);  // SSL_set0_rbio takes ownership.
   SSL_set0_rbio(ssl_.get(), transport_bio);
@@ skipping 180 matching lines @@
   }
 
   next_handshake_state_ = STATE_HANDSHAKE_COMPLETE;
   return net_error;
 }
 
 int SSLClientSocketImpl::DoHandshakeComplete(int result) {
   if (result < 0)
     return result;
 
+  if (ssl_session_cache_lookup_count_) {
+    SSLContext::GetInstance()->session_cache()->ResetLookupCount(
+        GetSessionCacheKey(), SSL_get_session(ssl_.get()));

[davidben, 2017/01/19 20:29:34]

Doesn't this want to be reset regardless of whether
ssl_session_cache_lookup_count_ is set and regardless of whether the sessions
are the same? We expect to get a fresh session from the server in all cases.

(SSL_get_session is also really Weird(TM). It doesn't return the offered session
in TLS 1.3 because 1.3 resumption.)

[nharper, 2017/01/19 21:50:35]

Yes, that sounds right. We're modelling the server sending infinite
NewSessionTickets, so it doesn't matter whether this session was resumed, or
with which session.

+    if (negotiated_protocol_ == kProtoHTTP2 && SSL_session_reused(ssl_.get())) {
+      UMA_HISTOGRAM_EXACT_LINEAR("Net.SSLSessionConcurrentLookupCount",
+                                 ssl_session_cache_lookup_count_, 20);

[davidben, 2017/01/19 20:29:34]

This should probably have a comment for what's going on here.

[nharper, 2017/01/19 21:50:35]

Done.

+    }
+  }
+
   // DHE is offered on the deprecated cipher fallback and then rejected
   // afterwards. This is to aid in diagnosing connection failures because a
   // server requires DHE ciphers.
   //
   // TODO(davidben): A few releases after DHE's removal, remove this logic.
   if (!ssl_config_.dhe_enabled &&
       SSL_CIPHER_is_DHE(SSL_get_current_cipher(ssl_.get()))) {
     return ERR_SSL_OBSOLETE_CIPHER;
   }
 
@@ skipping 906 matching lines @@
     if (ERR_GET_REASON(info->error_code) == SSL_R_TLSV1_ALERT_ACCESS_DENIED &&
         !certificate_requested_) {
       net_error = ERR_SSL_PROTOCOL_ERROR;
     }
   }
 
   return net_error;
 }
 
 }  // namespace net