MEDIUM: server/ssl: pick another thread's session when we have none yet

The per-thread SSL context in servers causes a burst of connection
renegotiations on startup, both for the forwarded traffic and for the
health checks. Health checks have been seen to continue to cause SSL
rekeying for several minutes after a restart on large thread-count
machines. The reason is that the context is exlusively per-thread
and that the more threads there are, the more likely it is for a new
connection to start on a thread that doesn't have such a context yet.

In order to improve this situation, this commit ensures that a thread
starting an SSL connection to a server without a session will first
look at the last session that was updated by another thread, and will
try to use it. In order to minimize the contention, we're using a read
lock here to protect the data, and the first-level index is an integer
containing the thread number, that is always valid and may always be
dereferenced. This way the session retrieval algorithm becomes quite
simple:

  - if the last thread index is valid, then try to use the same session
    under a read lock ;
  - if any error happens, then atomically nuke the index so that other
    threads don't use it and the next one to update a connection updates
    it again

And for the ssl_sess_new_srv_cb(), we have this:

  - update the entry under a write lock if the new session is valid,
    otherwise kill it if the session is not valid;
  - atomically update the index if it was 0 and the new one is valid,
    otherwise atomically nuke it if the session failed.

Note that even if only the pointer is destroyed, the element will be
re-allocated by the next thread during the sess_new_srv_sb().

Right now a session is picked even if the SNI doesn't match, because
we don't know the SNI yet during ssl_sock_init(), but that's essentially
a matter of API, since connect_server() figures the SNI very early, then
calls conn_prepare() which calls ssl_sock_init(). Thus in the future we
could easily imaging storing a number of SNI-based contexts instead of
storing contexts per thread.

It could be worth backporting this to one LTS version after some
observation, though this is not strictly necessary. the current commit
depends on the following ones:

  BUG/MINOR: ssl_sock: fix possible memory leak on OOM
  MINOR: ssl_sock: avoid iterating realloc(+1) on stored context
  DOC: ssl: add some comments about the non-obvious session allocation stuff
  CLEANUP: ssl: keep a pointer to the server in ssl_sock_init()
  MEDIUM: ssl_sock: always use the SSL's server name, not the one from the tid
  MEDIUM: server/ssl: place an rwlock in the per-thread ssl server session
  MINOR: server/ssl: maintain an index of the last known valid SSL session
  MINOR: server/ssl: clear the shared good session index on failure
  MEDIUM: server/ssl: pick another thread's session when we have none yet

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 90bc7be91fc685ddb023997daa39b5351a7e7430..f68d0e318f404cf50058c31728e7c05ea1724143 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -5770,6 +5770,35 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
                                        SSL_set_tlsext_host_name(ctx->ssl, srv->ssl_ctx.reused_sess[tid].sni);
                                HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[tid].sess_lock);
                        }
+               } else {
+                       /* No session available yet, let's see if we can pick one
+                        * from another thread. If old_tid is non-null, it designates
+                        * the index of a recently updated thread that might still have
+                        * a usable session. All threads are collectively responsible
+                        * for resetting the index if it fails.
+                        */
+                       const unsigned char *ptr;
+                       SSL_SESSION *sess;
+                       uint old_tid = HA_ATOMIC_LOAD(&srv->ssl_ctx.last_ssl_sess_tid); // 0=none, >0 = tid + 1
+
+                       if (old_tid) {
+                               HA_RWLOCK_RDLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[old_tid-1].sess_lock);
+
+                               ptr = srv->ssl_ctx.reused_sess[old_tid-1].ptr;
+                               if (ptr) {
+                                       sess = d2i_SSL_SESSION(NULL, &ptr, srv->ssl_ctx.reused_sess[old_tid-1].size);
+                                       if (sess) {
+                                               if (!SSL_set_session(ctx->ssl, sess))
+                                                       HA_ATOMIC_CAS(&srv->ssl_ctx.last_ssl_sess_tid, &old_tid, 0); // no more valid
+                                               SSL_SESSION_free(sess);
+                                       }
+                               }
+
+                               if (srv->ssl_ctx.reused_sess[old_tid-1].sni)
+                                       SSL_set_tlsext_host_name(ctx->ssl, srv->ssl_ctx.reused_sess[old_tid-1].sni);
+
+                               HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[old_tid-1].sess_lock);
+                       }
                }
                HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.lock);