Skip to content

Commit 2cb896c

Browse files
committed
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
1 parent 777f62c commit 2cb896c

File tree

1 file changed

+29
-0
lines changed

1 file changed

+29
-0
lines changed

src/ssl_sock.c

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5770,6 +5770,35 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
57705770
SSL_set_tlsext_host_name(ctx->ssl, srv->ssl_ctx.reused_sess[tid].sni);
57715771
HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[tid].sess_lock);
57725772
}
5773+
} else {
5774+
/* No session available yet, let's see if we can pick one
5775+
* from another thread. If old_tid is non-null, it designates
5776+
* the index of a recently updated thread that might still have
5777+
* a usable session. All threads are collectively responsible
5778+
* for resetting the index if it fails.
5779+
*/
5780+
const unsigned char *ptr;
5781+
SSL_SESSION *sess;
5782+
uint old_tid = HA_ATOMIC_LOAD(&srv->ssl_ctx.last_ssl_sess_tid); // 0=none, >0 = tid + 1
5783+
5784+
if (old_tid) {
5785+
HA_RWLOCK_RDLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[old_tid-1].sess_lock);
5786+
5787+
ptr = srv->ssl_ctx.reused_sess[old_tid-1].ptr;
5788+
if (ptr) {
5789+
sess = d2i_SSL_SESSION(NULL, &ptr, srv->ssl_ctx.reused_sess[old_tid-1].size);
5790+
if (sess) {
5791+
if (!SSL_set_session(ctx->ssl, sess))
5792+
HA_ATOMIC_CAS(&srv->ssl_ctx.last_ssl_sess_tid, &old_tid, 0); // no more valid
5793+
SSL_SESSION_free(sess);
5794+
}
5795+
}
5796+
5797+
if (srv->ssl_ctx.reused_sess[old_tid-1].sni)
5798+
SSL_set_tlsext_host_name(ctx->ssl, srv->ssl_ctx.reused_sess[old_tid-1].sni);
5799+
5800+
HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[old_tid-1].sess_lock);
5801+
}
57735802
}
57745803
HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.lock);
57755804

0 commit comments

Comments
 (0)