From eef7f8f26728569026f2011cfc699c5487d7da98 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Sun, 23 May 2021 12:18:32 +0200 Subject: [PATCH] common.h: Use better message for early client EOF When SSL sees a client hangup, SSL_get_error currently returns SSL_ERROR_SYSCALL. The page at [1] says in section BUGS, that errno would be 0 in this case: The SSL_ERROR_SYSCALL with errno value of 0 indicates unexpected EOF from the peer. To my experimentation, this is not true. errno is left unchanged instead. The page at [2] says, that ERR_get_error could be used to distinguish between EOF and some errno indicated failure: SSL_ERROR_SYSCALL Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details). But to my experimentation, this is not true either. In both cases, ERR_get_error doesn't have any further information and ret (from SSL_accept) is -1 in both cases. This might be fixed in OpenSLL 3.0 [3]: On an unexpected EOF, versions before OpenSSL 3.0 returned SSL_ERROR_SYSCALL, nothing was added to the error stack, and errno was 0. Since OpenSSL 3.0 the returned error is SSL_ERROR_SSL with a meaningful error on the error stack. Although it is ugly, we only have the option to set errno to zero before the operation. do this in ssl_read_with_timeout and ssl_accept_with_timeout. [1]: https://www.openssl.org/docs/man1.1.1/man3/SSL_get_error.html [2]: https://linux.die.net/man/3/ssl_get_error [3]: https://www.openssl.org/docs/manmaster/man3/SSL_get_error.html --- common.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/common.h b/common.h index 4bdb577..fd365e3 100644 --- a/common.h +++ b/common.h @@ -118,6 +118,7 @@ static int __attribute__((unused)) ssl_write_with_timeout(SSL *ssl, int fd, char } static int __attribute__((unused)) ssl_read_with_timeout(SSL *ssl, int fd, void *buf, size_t num, int timeout){ + errno = 0; /* see commit message */ while (1) { int status = SSL_read(ssl, buf, num); if (status > 0) @@ -130,7 +131,7 @@ static int __attribute__((unused)) ssl_read_with_timeout(SSL *ssl, int fd, void return -1; continue; case SSL_ERROR_SYSCALL: - if (errno==0) { + if (errno == 0) { COMMON_LOG(LOG_ERR, "%s: unexpected EOF from peer", __func__); errno = ECONNABORTED; return -1; @@ -148,6 +149,7 @@ static int __attribute__((unused)) ssl_read_with_timeout(SSL *ssl, int fd, void } static int __attribute__((unused)) ssl_accept_with_timeout(SSL *ssl, int fd, int timeout) { + errno = 0; /* see commit message */ while (1) { int status = SSL_accept(ssl); if (status == 1) @@ -160,6 +162,11 @@ static int __attribute__((unused)) ssl_accept_with_timeout(SSL *ssl, int fd, in return -1; continue; case SSL_ERROR_SYSCALL: + if (errno == 0) { + COMMON_LOG(LOG_ERR, "%s: unexpected EOF from peer", __func__); + errno = ECONNABORTED; + return -1; + } COMMON_LOG(LOG_ERR, "%s: %m", __func__); return -1; case SSL_ERROR_SSL: