Skip to content

Commit 190e9f6

Browse files
zuiderkwasthpatro
authored andcommitted
Detect SSL_new() returning NULL in outgoing connections (#2140)
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 #1939 (comment). Might fix #1939. Signed-off-by: Viktor Söderqvist <[email protected]> Signed-off-by: Harkrishn Patro <[email protected]>
1 parent b0528fe commit 190e9f6

File tree

1 file changed

+13
-14
lines changed

1 file changed

+13
-14
lines changed

src/tls.c

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,14 @@ typedef struct tls_connection {
452452
size_t last_failed_write_data_len;
453453
} tls_connection;
454454

455+
/* Fetch the latest OpenSSL error and store it in the connection */
456+
static void updateTLSError(tls_connection *conn) {
457+
conn->c.last_errno = 0;
458+
if (conn->ssl_error) zfree(conn->ssl_error);
459+
conn->ssl_error = zmalloc(512);
460+
ERR_error_string_n(ERR_get_error(), conn->ssl_error, 512);
461+
}
462+
455463
static connection *createTLSConnection(int client_side) {
456464
SSL_CTX *ctx = valkey_tls_ctx;
457465
if (client_side && valkey_tls_client_ctx) ctx = valkey_tls_client_ctx;
@@ -460,21 +468,17 @@ static connection *createTLSConnection(int client_side) {
460468
conn->c.fd = -1;
461469
conn->c.iovcnt = IOV_MAX;
462470
conn->ssl = SSL_new(ctx);
471+
if (!conn->ssl) {
472+
updateTLSError(conn);
473+
conn->c.state = CONN_STATE_ERROR;
474+
}
463475
return (connection *)conn;
464476
}
465477

466478
static connection *connCreateTLS(void) {
467479
return createTLSConnection(1);
468480
}
469481

470-
/* Fetch the latest OpenSSL error and store it in the connection */
471-
static void updateTLSError(tls_connection *conn) {
472-
conn->c.last_errno = 0;
473-
if (conn->ssl_error) zfree(conn->ssl_error);
474-
conn->ssl_error = zmalloc(512);
475-
ERR_error_string_n(ERR_get_error(), conn->ssl_error, 512);
476-
}
477-
478482
/* Create a new TLS connection that is already associated with
479483
* an accepted underlying file descriptor.
480484
*
@@ -488,14 +492,9 @@ static connection *connCreateAcceptedTLS(int fd, void *priv) {
488492
int require_auth = *(int *)priv;
489493
tls_connection *conn = (tls_connection *)createTLSConnection(0);
490494
conn->c.fd = fd;
495+
if (conn->c.state == CONN_STATE_ERROR) return (connection *)conn;
491496
conn->c.state = CONN_STATE_ACCEPTING;
492497

493-
if (!conn->ssl) {
494-
updateTLSError(conn);
495-
conn->c.state = CONN_STATE_ERROR;
496-
return (connection *)conn;
497-
}
498-
499498
switch (require_auth) {
500499
case TLS_CLIENT_AUTH_NO: SSL_set_verify(conn->ssl, SSL_VERIFY_NONE, NULL); break;
501500
case TLS_CLIENT_AUTH_OPTIONAL: SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, NULL); break;

0 commit comments

Comments
 (0)