Skip to content

Commit

Permalink
common.h: Use better message for early client EOF
Browse files Browse the repository at this point in the history
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
  • Loading branch information
donald committed May 23, 2021
1 parent 34a9bf2 commit eef7f8f
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
Expand All @@ -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)
Expand All @@ -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:
Expand Down

0 comments on commit eef7f8f

Please sign in to comment.