Fix sess_hits counter on the server (#1974)

While debugging the Ruby tests using an external session cache for TLS
1.2, there was an issue found where we were bumping up the
`SSL_CTX_sess_hits` counter even when there wasn't a session found in
the cache. We bump `sess_hits` in three places, due to it's broader
implied meaning in OpenSSL. There's a nice description of the counter in
this older commit: 4b18065

Apparently session could be a nullptr, which means a session wasn't
found. In the original code, this meant that we would be updating the
cache hit counter regardless of a session being found or not. This
should only be updated when a session was found. Changing it to check
for an existing pointer in session fixes this.

### Testing:
New check inserted in an external session cache check.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.

ssl/ssl_session.cc

@@ -667,7 +667,7 @@ static enum ssl_hs_wait_t ssl_lookup_session(
     // TODO(davidben): This should probably move it to the front of the list.
     if (session == nullptr) {
       ssl_update_counter(ssl->session_ctx.get(),
-                          ssl->session_ctx->stats.sess_miss, true);
+                         ssl->session_ctx->stats.sess_miss, true);
     }
   }
 
@@ -705,15 +705,17 @@ static enum ssl_hs_wait_t ssl_lookup_session(
   if (!ssl_session_is_time_valid(ssl, session.get())) {
     ssl_update_counter(ssl->session_ctx.get(),
                        ssl->session_ctx->stats.sess_timeout, true);
-    if(session) {
+    if (session) {
       // The session was from the cache, so remove it.
       SSL_CTX_remove_session(ssl->session_ctx.get(), session.get());
       session.reset();
     }
   }
 
-  ssl_update_counter(ssl->session_ctx.get(),
-                     ssl->session_ctx->stats.sess_hit, true);
+  if (session) {
+    ssl_update_counter(ssl->session_ctx.get(), ssl->session_ctx->stats.sess_hit,
+                       true);
+  }
   *out_session = std::move(session);
   return ssl_hs_ok;
 }

ssl/ssl_test.cc

@@ -8601,6 +8601,9 @@ TEST_P(SSLVersionTest, SessionMissCache) {
   // Subsequent connections will all be both timeouts and misses.
   EXPECT_EQ(SSL_CTX_sess_misses(server_ctx_.get()), kNumConnections - 1);
   EXPECT_EQ(SSL_CTX_sess_timeouts(server_ctx_.get()), kNumConnections);
+  // Check that |sess_hits| is not incorrectly incremented on either end.
+  EXPECT_EQ(SSL_CTX_sess_hits(client_ctx_.get()), 0);
+  EXPECT_EQ(SSL_CTX_sess_hits(server_ctx_.get()), 0);
 }
 
 // Callback function to force an external session cache counter update.