From 17e66863a5f9d6065544d0b3d655ba8f40081570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20S=C3=B6derqvist?= Date: Tue, 27 May 2025 20:31:12 +0200 Subject: [PATCH] Detect SSL_new() returning NULL in outgoing connections (#2140) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When creating an outgoing TLS connection, we don't check if `SSL_new()` returned NULL. Without this patch, the check was done only for incoming connections in `connCreateAcceptedTLS()`. This patch moves the check to `createTLSConnection()` which is used both for incoming and outgoing connections. This check makes sure we fail the connection before going any further, e.g. when `connCreate()` is followed by `connConnect()`, the latter returns `C_ERR` which is commonly detected where outgoing connections are established, such where a replica connects to a primary. ```c int connectWithPrimary(void) { server.repl_transfer_s = connCreate(connTypeOfReplication()); if (connConnect(server.repl_transfer_s, server.primary_host, server.primary_port, server.bind_source_addr, server.repl_mptcp, syncWithPrimary) == C_ERR) { serverLog(LL_WARNING, "Unable to connect to PRIMARY: %s", connGetLastError(server.repl_transfer_s)); connClose(server.repl_transfer_s); server.repl_transfer_s = NULL; return C_ERR; } ``` For a more thorough explanation, see https://github.com/valkey-io/valkey/issues/1939#issuecomment-2912177877. Might fix #1939. Signed-off-by: Viktor Söderqvist --- src/tls.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) Index: b/src/tls.c =================================================================== --- a/src/tls.c +++ b/src/tls.c @@ -456,6 +456,14 @@ typedef struct tls_connection { size_t last_failed_write_data_len; } tls_connection; +/* Fetch the latest OpenSSL error and store it in the connection */ +static void updateTLSError(tls_connection *conn) { + conn->c.last_errno = 0; + if (conn->ssl_error) zfree(conn->ssl_error); + conn->ssl_error = zmalloc(512); + ERR_error_string_n(ERR_get_error(), conn->ssl_error, 512); +} + static connection *createTLSConnection(int client_side) { SSL_CTX *ctx = valkey_tls_ctx; if (client_side && valkey_tls_client_ctx) ctx = valkey_tls_client_ctx; @@ -464,6 +472,10 @@ static connection *createTLSConnection(i conn->c.fd = -1; conn->c.iovcnt = IOV_MAX; conn->ssl = SSL_new(ctx); + if (!conn->ssl) { + updateTLSError(conn); + conn->c.state = CONN_STATE_ERROR; + } return (connection *)conn; } @@ -471,14 +483,6 @@ static connection *connCreateTLS(void) { return createTLSConnection(1); } -/* Fetch the latest OpenSSL error and store it in the connection */ -static void updateTLSError(tls_connection *conn) { - conn->c.last_errno = 0; - if (conn->ssl_error) zfree(conn->ssl_error); - conn->ssl_error = zmalloc(512); - ERR_error_string_n(ERR_get_error(), conn->ssl_error, 512); -} - /* Create a new TLS connection that is already associated with * an accepted underlying file descriptor. * @@ -492,14 +496,9 @@ static connection *connCreateAcceptedTLS int require_auth = *(int *)priv; tls_connection *conn = (tls_connection *)createTLSConnection(0); conn->c.fd = fd; + if (conn->c.state == CONN_STATE_ERROR) return (connection *)conn; conn->c.state = CONN_STATE_ACCEPTING; - if (!conn->ssl) { - updateTLSError(conn); - conn->c.state = CONN_STATE_ERROR; - return (connection *)conn; - } - switch (require_auth) { case TLS_CLIENT_AUTH_NO: SSL_set_verify(conn->ssl, SSL_VERIFY_NONE, NULL); break; case TLS_CLIENT_AUTH_OPTIONAL: SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, NULL); break;