Never use the internal session cache for a client.

The internal session cache is keyed on session ID, so this is completely
useless for clients (indeed we never look it up internally). Along the way,
tidy up ssl_update_cache to be more readable. The slight behavior change is
that SSL_CTX_add_session's return code no longer controls the external
callback. It's not clear to me what that could have accomplished. (It can only
fail on allocation error. We only call it for new sessions, so the duplicate
case is impossible.)

The one thing of value the internal cache might have provided is managing the
timeout. The SSL_CTX_flush_sessions logic would flip the not_resumable bit and
cause us not to offer expired sessions (modulo SSL_CTX_flush_sessions's delay
and any discrepancies between the two caches). Instead, just check expiration
when deciding whether or not to offer a session.

This way clients that set SSL_SESS_CACHE_CLIENT blindly don't accidentally
consume gobs of memory.

BUG=531194

Change-Id: If97485beab21874f37737edc44df24e61ce23705
Reviewed-on: https://boringssl-review.googlesource.com/6321
Reviewed-by: Adam Langley <alangley@gmail.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index b42e907..29f905c 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1456,11 +1456,8 @@
 /* SSL_SESS_CACHE_OFF disables all session caching. */
 #define SSL_SESS_CACHE_OFF 0x0000
 
-/* SSL_SESS_CACHE_CLIENT enables session caching for a client.
- *
- * TODO(davidben): The internal cache is useless on the client. Always act as if
- * SSL_SESS_CACHE_NO_INTERNAL is set. https://crbug.com/531194. Also see TODO
- * attached to |SSL_CTX_sess_set_new_cb|. */
+/* SSL_SESS_CACHE_CLIENT enables session caching for a client. The internal
+ * cache is never used on a client, so this only enables the callbacks. */
 #define SSL_SESS_CACHE_CLIENT 0x0001
 
 /* SSL_SESS_CACHE_SERVER enables session caching for a server. */
@@ -1473,15 +1470,16 @@
  * |SSL_CTX_flush_sessions| every 255 connections. */
 #define SSL_SESS_CACHE_NO_AUTO_CLEAR 0x0080
 
-/* SSL_SESS_CACHE_NO_INTERNAL_LOOKUP disables looking up a session from the
- * internal session cache. */
+/* SSL_SESS_CACHE_NO_INTERNAL_LOOKUP, on a server, disables looking up a session
+ * from the internal session cache. */
 #define SSL_SESS_CACHE_NO_INTERNAL_LOOKUP 0x0100
 
-/* SSL_SESS_CACHE_NO_INTERNAL_STORE disables storing sessions in the internal
- * session cache. */
+/* SSL_SESS_CACHE_NO_INTERNAL_STORE, on a server, disables storing sessions in
+ * the internal session cache. */
 #define SSL_SESS_CACHE_NO_INTERNAL_STORE 0x0200
 
-/* SSL_SESS_CACHE_NO_INTERNAL disables the internal session cache. */
+/* SSL_SESS_CACHE_NO_INTERNAL, on a server, disables the internal session
+ * cache. */
 #define SSL_SESS_CACHE_NO_INTERNAL \
     (SSL_SESS_CACHE_NO_INTERNAL_LOOKUP | SSL_SESS_CACHE_NO_INTERNAL_STORE)