106 lines
3.8 KiB
Diff
106 lines
3.8 KiB
Diff
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;
|