BUG/MINOR: ssl: use a thread-safe sslconns increment

Each time a new SSL context is allocated, global.sslconns is
incremented. If global.maxsslconn is reached, the allocation is
cancelled.

This procedure was not entirely thread-safe due to the check and
increment operations conducted at different stage. This could lead to
global.maxsslconn slightly exceeded when several threads allocate SSL
context while sslconns is near the limit.

To fix this, use a CAS operation in a do/while loop. This code is
similar to the actconn/maxconn increment for connection.

A new function increment_sslconn() is defined for this operation. For
the moment, only SSL code is using it. However, it is expected that QUIC
will also use it to count QUIC connections as SSL ones.

This should be backported to all stable releases. Note that prior to the
2.6, sslconns was outside of global struct, so this commit should be
slightly adjusted.
This commit is contained in:
Amaury Denoyelle 2023-10-25 15:38:04 +02:00
parent fffd435bbd
commit 350f8b0c07
2 changed files with 32 additions and 7 deletions

View File

@ -113,6 +113,7 @@ int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *arg);
int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *priv);
#endif
int increment_sslconn();
SSL_CTX *ssl_sock_assign_generated_cert(unsigned int key, struct bind_conf *bind_conf, SSL *ssl);
SSL_CTX *ssl_sock_get_generated_cert(unsigned int key, struct bind_conf *bind_conf);
int ssl_sock_set_generated_cert(SSL_CTX *ctx, unsigned int key, struct bind_conf *bind_conf);

View File

@ -5698,6 +5698,27 @@ static int ssl_sock_start(struct connection *conn, void *xprt_ctx)
return 0;
}
/* Similar to increment_actconn() but for SSL connections. */
int increment_sslconn()
{
unsigned int count, next_sslconn;
do {
count = global.sslconns;
if (global.maxsslconn && count >= global.maxsslconn) {
/* maxconn reached */
next_sslconn = 0;
goto end;
}
/* try to increment sslconns */
next_sslconn = count + 1;
} while (!_HA_ATOMIC_CAS(&global.sslconns, &count, next_sslconn) && __ha_cpu_relax());
end:
return next_sslconn;
}
/*
* This function is called if SSL * context is not yet allocated. The function
* is designed to be called before any other data-layer operation and sets the
@ -5707,6 +5728,8 @@ static int ssl_sock_start(struct connection *conn, void *xprt_ctx)
static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
{
struct ssl_sock_ctx *ctx;
int next_sslconn = 0;
/* already initialized */
if (*xprt_ctx)
return 0;
@ -5734,6 +5757,12 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
ctx->xprt_ctx = NULL;
ctx->error_code = 0;
next_sslconn = increment_sslconn();
if (!next_sslconn) {
conn->err_code = CO_ER_SSL_TOO_MANY;
goto err;
}
/* Only work with sockets for now, this should be adapted when we'll
* add QUIC support.
*/
@ -5743,11 +5772,6 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
goto err;
}
if (global.maxsslconn && global.sslconns >= global.maxsslconn) {
conn->err_code = CO_ER_SSL_TOO_MANY;
goto err;
}
/* If it is in client mode initiate SSL session
in connect state otherwise accept state */
if (objt_server(conn->target)) {
@ -5823,7 +5847,6 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
/* leave init state and start handshake */
conn->flags |= CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN;
_HA_ATOMIC_INC(&global.sslconns);
_HA_ATOMIC_INC(&global.totalsslconns);
*xprt_ctx = ctx;
return 0;
@ -5856,7 +5879,6 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
conn->flags |= CO_FL_EARLY_SSL_HS;
#endif
_HA_ATOMIC_INC(&global.sslconns);
_HA_ATOMIC_INC(&global.totalsslconns);
*xprt_ctx = ctx;
return 0;
@ -5864,6 +5886,8 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
/* don't know how to handle such a target */
conn->err_code = CO_ER_SSL_NO_TARGET;
err:
if (next_sslconn)
_HA_ATOMIC_DEC(&global.sslconns);
if (ctx && ctx->wait_event.tasklet)
tasklet_free(ctx->wait_event.tasklet);
pool_free(ssl_sock_ctx_pool, ctx);