valkey/valkey-ssl_new-null-return.patch

From 17e66863a5f9d6065544d0b3d655ba8f40081570 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Viktor=20S=C3=B6derqvist?= <viktor.soderqvist@est.tech>
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 <viktor.soderqvist@est.tech>
---
 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;