stream-ssl: Disable SSL session caching.

SSL session caching causes mysterious connection failure problems.

Bug #4448.
Bug #4501.

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index a93c00642171c1be4f60fc7c7cafeb2563b7aea0..4d7c7c4dd15395dfe214091403be9555667e4587 100644 (file)
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -49,9 +49,6 @@
 
 VLOG_DEFINE_THIS_MODULE(stream_ssl);
 
-COVERAGE_DEFINE(ssl_session);
-COVERAGE_DEFINE(ssl_session_reused);
-
 /* Active SSL. */
 
 enum ssl_state {
@@ -140,20 +137,6 @@ struct ssl_stream
 /* SSL context created by ssl_init(). */
 static SSL_CTX *ctx;
 
-/* Maps from stream target (e.g. "127.0.0.1:1234") to SSL_SESSION *.  The
- * sessions are those from the last SSL connection to the given target.
- * OpenSSL caches server-side sessions internally, so this cache is only used
- * for client connections.
- *
- * The stream_ssl module owns a reference to each of the sessions in this
- * table, so they must be freed with SSL_SESSION_free() when they are no
- * longer needed. */
-static struct shash client_sessions = SHASH_INITIALIZER(&client_sessions);
-
-/* Maximum number of client sessions to cache.  Ordinarily I'd expect that one
- * session would be sufficient but this should cover it. */
-#define MAX_CLIENT_SESSION_CACHE 16
-
 struct ssl_config_file {
     bool read;                  /* Whether the file was successfully read. */
     char *file_name;            /* Configured file name, if any. */
@@ -283,13 +266,6 @@ new_ssl_stream(const char *name, int fd, enum session_type type,
     if (!verify_peer_cert || (bootstrap_ca_cert && type == CLIENT)) {
         SSL_set_verify(ssl, SSL_VERIFY_NONE, NULL);
     }
-    if (type == CLIENT) {
-        /* Grab SSL session information from the cache. */
-        SSL_SESSION *session = shash_find_data(&client_sessions, name);
-        if (session && SSL_set_session(ssl, session) != 1) {
-            interpret_queued_ssl_error("SSL_set_session");
-        }
-    }
 
     /* Create and return the ssl_stream. */
     sslv = xmalloc(sizeof *sslv);
@@ -446,58 +422,6 @@ do_ca_cert_bootstrap(struct stream *stream)
     return EPROTO;
 }
 
-static void
-ssl_delete_session(struct shash_node *node)
-{
-    SSL_SESSION *session = node->data;
-    SSL_SESSION_free(session);
-    shash_delete(&client_sessions, node);
-}
-
-/* Find and free any previously cached session for 'stream''s target. */
-static void
-ssl_flush_session(struct stream *stream)
-{
-    struct shash_node *node;
-
-    node = shash_find(&client_sessions, stream_get_name(stream));
-    if (node) {
-        ssl_delete_session(node);
-    }
-}
-
-/* Add 'stream''s session to the cache for its target, so that it will be
- * reused for future SSL connections to the same target. */
-static void
-ssl_cache_session(struct stream *stream)
-{
-    struct ssl_stream *sslv = ssl_stream_cast(stream);
-    SSL_SESSION *session;
-
-    /* Get session from stream. */
-    session = SSL_get1_session(sslv->ssl);
-    if (session) {
-        SSL_SESSION *old_session;
-
-        old_session = shash_replace(&client_sessions, stream_get_name(stream),
-                                    session);
-        if (old_session) {
-            /* Free the session that we replaced.  (We might actually have
-             * session == old_session, but either way we have to free it to
-             * avoid leaking a reference.) */
-            SSL_SESSION_free(old_session);
-        } else if (shash_count(&client_sessions) > MAX_CLIENT_SESSION_CACHE) {
-            for (;;) {
-                struct shash_node *node = shash_random_node(&client_sessions);
-                if (node->data != session) {
-                    ssl_delete_session(node);
-                    break;
-                }
-            }
-        }
-    }
-}
-
 static int
 ssl_connect(struct stream *stream)
 {
@@ -530,17 +454,6 @@ ssl_connect(struct stream *stream)
             } else {
                 int unused;
 
-                if (sslv->type == CLIENT) {
-                    /* Delete any cached session for this stream's target.
-                     * Otherwise a single error causes recurring errors that
-                     * don't resolve until the SSL client or server is
-                     * restarted.  (It can take dozens of reused connections to
-                     * see this behavior, so this is difficult to test.)  If we
-                     * delete the session on the first error, though, the error
-                     * only occurs once and then resolves itself. */
-                    ssl_flush_session(stream);
-                }
-
                 interpret_ssl_error((sslv->type == CLIENT ? "SSL_connect"
                                      : "SSL_accept"), retval, error, &unused);
                 shutdown(sslv->fd, SHUT_RDWR);
@@ -565,11 +478,6 @@ ssl_connect(struct stream *stream)
             VLOG_ERR("rejecting SSL connection during bootstrap race window");
             return EPROTO;
         } else {
-            /* Statistics. */
-            COVERAGE_INC(ssl_session);
-            if (SSL_session_reused(sslv->ssl)) {
-                COVERAGE_INC(ssl_session_reused);
-            }
             return 0;
         }
     }
@@ -590,8 +498,6 @@ ssl_close(struct stream *stream)
      * background. */
     SSL_shutdown(sslv->ssl);
 
-    ssl_cache_session(stream);
-
     /* SSL_shutdown() might have signaled an error, in which case we need to
      * flush it out of the OpenSSL error queue or the next OpenSSL operation
      * will falsely signal an error. */
@@ -1009,17 +915,6 @@ do_ssl_init(void)
     SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
                        NULL);
 
-    /* We have to set a session context ID string in 'ctx' because OpenSSL
-     * otherwise refuses to use a cached session on the server side when
-     * SSL_VERIFY_PEER is set.  And it not only refuses to use the cached
-     * session, it actually generates an error and kills the connection.
-     * According to a comment in ssl_get_prev_session() in OpenSSL's
-     * ssl/ssl_sess.c, this is intentional behavior.
-     *
-     * Any context string is OK, as long as one is set. */
-    SSL_CTX_set_session_id_context(ctx, (const unsigned char *) PACKAGE,
-                                   strlen(PACKAGE));
-
     return 0;
 }