opensourcemirror
/
redis
mirror of https://github.com/antirez/redis.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Improve TLS error handling. (#11557) 

* Remove duplicate code, propagating SSL errors into connection state.
* Add missing error handling in synchronous IO functions.
* Fix connection error reporting in some replication flows.
This commit is contained in:
Yossi Gottlieb 2022-11-30 22:23:00 +02:00 committed by GitHub
parent 7dfd7b9197
commit 6c98a97bf4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 57 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/replication.c
View File

@ -1837,7 +1837,7 @@ void readSyncBulkPayload(connection *conn) {
if (nread == -1) {
serverLog(LL_WARNING,
"I/O error reading bulk count from MASTER: %s",
strerror(errno));
connGetLastError(conn));
goto error;
} else {
/* nread here is returned by connSyncReadLine(), which calls syncReadLine() and
@ -1909,7 +1909,7 @@ void readSyncBulkPayload(connection *conn) {
return;
}
serverLog(LL_WARNING,"I/O error trying to sync with MASTER: %s",
(nread == -1) ? strerror(errno) : "connection lost");
(nread == -1) ? connGetLastError(conn) : "connection lost");
cancelReplicationHandshake(1);
return;
}
@ -2254,7 +2254,7 @@ char *receiveSynchronousResponse(connection *conn) {
/* Read the reply from the server. */
if (connSyncReadLine(conn,buf,sizeof(buf),server.repl_syncio_timeout*1000) == -1)
{
serverLog(LL_WARNING, "Failed to read response from the server: %s", strerror(errno));
serverLog(LL_WARNING, "Failed to read response from the server: %s", connGetLastError(conn));
return NULL;
}
server.repl_transfer_lastio = server.unixtime;
@ -2815,7 +2815,7 @@ void syncWithMaster(connection *conn) {
serverLog(LL_NOTICE,"Retrying with SYNC...");
if (connSyncWrite(conn,"SYNC\r\n",6,server.repl_syncio_timeout*1000) == -1) {
serverLog(LL_WARNING,"I/O error writing to MASTER: %s",
strerror(errno));
connGetLastError(conn));
goto error;
}
}

113
src/tls.c
View File

@ -517,6 +517,7 @@ static connection *connCreateAcceptedTLS(int fd, void *priv) {
}
static void tlsEventHandler(struct aeEventLoop *el, int fd, void *clientData, int mask);
static void updateSSLEvent(tls_connection *conn);
/* Process the return code received from OpenSSL>
* Update the want parameter with expected I/O.
@ -550,6 +551,47 @@ static int handleSSLReturnCode(tls_connection *conn, int ret_value, WantIOType *
return 0;
}
/* Handle OpenSSL return code following SSL_write() or SSL_read():
*
* - Updates conn state and last_errno.
* - If update_event is nonzero, calls updateSSLEvent() when necessary.
*
* Returns ret_value, or -1 on error or dropped connection.
*/
static int updateStateAfterSSLIO(tls_connection *conn, int ret_value, int update_event) {
/* If system call was interrupted, there's no need to go through the full
* OpenSSL error handling and just report this for the caller to retry the
* operation.
*/
if (errno == EINTR) {
conn->c.last_errno = EINTR;
return -1;
}
if (ret_value <= 0) {
WantIOType want = 0;
int ssl_err;
if (!(ssl_err = handleSSLReturnCode(conn, ret_value, &want))) {
if (want == WANT_READ) conn->flags |= TLS_CONN_FLAG_WRITE_WANT_READ;
if (want == WANT_WRITE) conn->flags |= TLS_CONN_FLAG_READ_WANT_WRITE;
if (update_event) updateSSLEvent(conn);
errno = EAGAIN;
return -1;
} else {
if (ssl_err == SSL_ERROR_ZERO_RETURN ||
((ssl_err == SSL_ERROR_SYSCALL && !errno))) {
conn->c.state = CONN_STATE_CLOSED;
return -1;
} else {
conn->c.state = CONN_STATE_ERROR;
return -1;
}
}
}
return ret_value;
}
static void registerSSLEvent(tls_connection *conn, WantIOType want) {
int mask = aeGetFileEvents(server.el, conn->c.fd);
@ -830,39 +872,12 @@ static int connTLSConnect(connection *conn_, const char *addr, int port, const c
static int connTLSWrite(connection *conn_, const void *data, size_t data_len) {
tls_connection *conn = (tls_connection *) conn_;
int ret, ssl_err;
int ret;
if (conn->c.state != CONN_STATE_CONNECTED) return -1;
ERR_clear_error();
ret = SSL_write(conn->ssl, data, data_len);
/* If system call was interrupted, there's no need to go through the full
* OpenSSL error handling and just report this for the caller to retry the
* operation.
*/
if (errno == EINTR) {
conn->c.last_errno = EINTR;
return -1;
}
if (ret <= 0) {
WantIOType want = 0;
if (!(ssl_err = handleSSLReturnCode(conn, ret, &want))) {
if (want == WANT_READ) conn->flags |= TLS_CONN_FLAG_WRITE_WANT_READ;
updateSSLEvent(conn);
errno = EAGAIN;
return -1;
} else {
if (ssl_err == SSL_ERROR_ZERO_RETURN ||
((ssl_err == SSL_ERROR_SYSCALL && !errno))) {
conn->c.state = CONN_STATE_CLOSED;
return -1;
} else {
conn->c.state = CONN_STATE_ERROR;
return -1;
}
}
}
return ret;
return updateStateAfterSSLIO(conn, ret, 1);
}
static int connTLSWritev(connection *conn_, const struct iovec *iov, int iovcnt) {
@ -905,40 +920,11 @@ static int connTLSWritev(connection *conn_, const struct iovec *iov, int iovcnt)
static int connTLSRead(connection *conn_, void *buf, size_t buf_len) {
tls_connection *conn = (tls_connection *) conn_;
int ret;
int ssl_err;
if (conn->c.state != CONN_STATE_CONNECTED) return -1;
ERR_clear_error();
ret = SSL_read(conn->ssl, buf, buf_len);
/* If system call was interrupted, there's no need to go through the full
* OpenSSL error handling and just report this for the caller to retry the
* operation.
*/
if (errno == EINTR) {
conn->c.last_errno = EINTR;
return -1;
}
if (ret <= 0) {
WantIOType want = 0;
if (!(ssl_err = handleSSLReturnCode(conn, ret, &want))) {
if (want == WANT_WRITE) conn->flags |= TLS_CONN_FLAG_READ_WANT_WRITE;
updateSSLEvent(conn);
errno = EAGAIN;
return -1;
} else {
if (ssl_err == SSL_ERROR_ZERO_RETURN ||
((ssl_err == SSL_ERROR_SYSCALL) && !errno)) {
conn->c.state = CONN_STATE_CLOSED;
return 0;
} else {
conn->c.state = CONN_STATE_ERROR;
return -1;
}
}
}
return ret;
return updateStateAfterSSLIO(conn, ret, 1);
}
static const char *connTLSGetLastError(connection *conn_) {
@ -1005,7 +991,9 @@ static ssize_t connTLSSyncWrite(connection *conn_, char *ptr, ssize_t size, long
setBlockingTimeout(conn, timeout);
SSL_clear_mode(conn->ssl, SSL_MODE_ENABLE_PARTIAL_WRITE);
ERR_clear_error();
int ret = SSL_write(conn->ssl, ptr, size);
ret = updateStateAfterSSLIO(conn, ret, 0);
SSL_set_mode(conn->ssl, SSL_MODE_ENABLE_PARTIAL_WRITE);
unsetBlockingTimeout(conn);
@ -1016,7 +1004,9 @@ static ssize_t connTLSSyncRead(connection *conn_, char *ptr, ssize_t size, long
tls_connection *conn = (tls_connection *) conn_;
setBlockingTimeout(conn, timeout);
ERR_clear_error();
int ret = SSL_read(conn->ssl, ptr, size);
ret = updateStateAfterSSLIO(conn, ret, 0);
unsetBlockingTimeout(conn);
return ret;
@ -1032,7 +1022,10 @@ static ssize_t connTLSSyncReadLine(connection *conn_, char *ptr, ssize_t size, l
while(size) {
char c;
if (SSL_read(conn->ssl,&c,1) <= 0) {
ERR_clear_error();
int ret = SSL_read(conn->ssl, &c, 1);
ret = updateStateAfterSSLIO(conn, ret, 0);
if (ret <= 0) {
nread = -1;
goto exit;
}