From 59c3fee2c760ebd4dacb3db2cddf7fea4220d506 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 3 Aug 2018 06:26:58 +0900 Subject: [PATCH 1/4] resolve: clear error queue before calling SSL_*() --- src/resolve/resolved-dnstls-openssl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/resolve/resolved-dnstls-openssl.c b/src/resolve/resolved-dnstls-openssl.c index 5dd7737337..92a171f565 100644 --- a/src/resolve/resolved-dnstls-openssl.c +++ b/src/resolve/resolved-dnstls-openssl.c @@ -73,6 +73,7 @@ int dnstls_stream_connect_tls(DnsStream *stream, DnsServer *server) { SSL_set_session(s, server->dnstls_data.session); SSL_set_bio(s, TAKE_PTR(rb), TAKE_PTR(wb)); + ERR_clear_error(); stream->dnstls_data.handshake = SSL_do_handshake(s); if (stream->dnstls_data.handshake <= 0) { error = SSL_get_error(s, stream->dnstls_data.handshake); @@ -120,6 +121,7 @@ int dnstls_stream_on_io(DnsStream *stream, uint32_t revents) { } if (stream->dnstls_data.shutdown) { + ERR_clear_error(); r = SSL_shutdown(stream->dnstls_data.ssl); if (r <= 0) { error = SSL_get_error(stream->dnstls_data.ssl, r); @@ -149,6 +151,7 @@ int dnstls_stream_on_io(DnsStream *stream, uint32_t revents) { dns_stream_unref(stream); return DNSTLS_STREAM_CLOSED; } else if (stream->dnstls_data.handshake <= 0) { + ERR_clear_error(); stream->dnstls_data.handshake = SSL_do_handshake(stream->dnstls_data.ssl); if (stream->dnstls_data.handshake <= 0) { error = SSL_get_error(stream->dnstls_data.ssl, stream->dnstls_data.handshake); @@ -197,6 +200,7 @@ int dnstls_stream_shutdown(DnsStream *stream, int error) { } if (error == ETIMEDOUT) { + ERR_clear_error(); r = SSL_shutdown(stream->dnstls_data.ssl); if (r == 0) { if (!stream->dnstls_data.shutdown) { @@ -249,6 +253,7 @@ ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count) { assert(stream->dnstls_data.ssl); assert(buf); + ERR_clear_error(); ss = r = SSL_write(stream->dnstls_data.ssl, buf, count); if (r <= 0) { error = SSL_get_error(stream->dnstls_data.ssl, ss); @@ -286,6 +291,7 @@ ssize_t dnstls_stream_read(DnsStream *stream, void *buf, size_t count) { assert(stream->dnstls_data.ssl); assert(buf); + ERR_clear_error(); ss = r = SSL_read(stream->dnstls_data.ssl, buf, count); if (r <= 0) { error = SSL_get_error(stream->dnstls_data.ssl, ss); From 36f1946c737ee126cb4ab6a2f9bf31daeb3bf0d1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 3 Aug 2018 06:29:38 +0900 Subject: [PATCH 2/4] resolve: fix typo and coding style cleanups --- src/resolve/resolved-dnstls-openssl.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/resolve/resolved-dnstls-openssl.c b/src/resolve/resolved-dnstls-openssl.c index 92a171f565..6b765b58ec 100644 --- a/src/resolve/resolved-dnstls-openssl.c +++ b/src/resolve/resolved-dnstls-openssl.c @@ -46,11 +46,9 @@ static int dnstls_flush_write_buffer(DnsStream *stream) { } int dnstls_stream_connect_tls(DnsStream *stream, DnsServer *server) { + _cleanup_(BIO_freep) BIO *rb = NULL, *wb = NULL; _cleanup_(SSL_freep) SSL *s = NULL; - _cleanup_(BIO_freep) BIO *rb = NULL; - _cleanup_(BIO_freep) BIO *wb = NULL; - int r; - int error; + int error, r; assert(stream); assert(server); @@ -106,14 +104,13 @@ void dnstls_stream_free(DnsStream *stream) { } int dnstls_stream_on_io(DnsStream *stream, uint32_t revents) { - int r; - int error; + int error, r; assert(stream); assert(stream->encrypted); assert(stream->dnstls_data.ssl); - /* Flush write buffer when requested by OpenSSL ss*/ + /* Flush write buffer when requested by OpenSSL */ if ((revents & EPOLLOUT) && (stream->dnstls_events & EPOLLOUT)) { r = dnstls_flush_write_buffer(stream); if (r < 0) @@ -181,8 +178,7 @@ int dnstls_stream_on_io(DnsStream *stream, uint32_t revents) { } int dnstls_stream_shutdown(DnsStream *stream, int error) { - int r; - int ssl_error; + int ssl_error, r; SSL_SESSION *s; assert(stream); @@ -244,8 +240,7 @@ int dnstls_stream_shutdown(DnsStream *stream, int error) { } ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count) { - int r; - int error; + int error, r; ssize_t ss; assert(stream); @@ -268,7 +263,7 @@ ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count) { char errbuf[256]; ERR_error_string_n(error, errbuf, sizeof(errbuf)); - log_debug("Failed to invoke SSL_read: %s", errbuf); + log_debug("Failed to invoke SSL_write: %s", errbuf); ss = -EPIPE; } } @@ -282,8 +277,7 @@ ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count) { } ssize_t dnstls_stream_read(DnsStream *stream, void *buf, size_t count) { - int r; - int error; + int error, r; ssize_t ss; assert(stream); From 8eadd2918395ea7fb7b4fbcee4723c9d60e09d04 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 3 Aug 2018 06:34:19 +0900 Subject: [PATCH 3/4] resolve: fix error handling of SSL_shutdown() --- src/resolve/resolved-dnstls-openssl.c | 32 ++++++++++++++++++++------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/resolve/resolved-dnstls-openssl.c b/src/resolve/resolved-dnstls-openssl.c index 6b765b58ec..c23211404b 100644 --- a/src/resolve/resolved-dnstls-openssl.c +++ b/src/resolve/resolved-dnstls-openssl.c @@ -120,31 +120,42 @@ int dnstls_stream_on_io(DnsStream *stream, uint32_t revents) { if (stream->dnstls_data.shutdown) { ERR_clear_error(); r = SSL_shutdown(stream->dnstls_data.ssl); - if (r <= 0) { + if (r == 0) { + stream->dnstls_events = 0; + + r = dnstls_flush_write_buffer(stream); + if (r < 0) + return r; + + return -EAGAIN; + } else if (r < 0) { error = SSL_get_error(stream->dnstls_data.ssl, r); - if (r == 0 || IN_SET(error, SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE)) { - if (r < 0) - stream->dnstls_events = error == SSL_ERROR_WANT_READ ? EPOLLIN : EPOLLOUT; + if (IN_SET(error, SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE)) { + stream->dnstls_events = error == SSL_ERROR_WANT_READ ? EPOLLIN : EPOLLOUT; r = dnstls_flush_write_buffer(stream); if (r < 0) return r; return -EAGAIN; + } else if (error == SSL_ERROR_SYSCALL) { + if (errno > 0) + log_debug_errno(errno, "Failed to invoke SSL_shutdown, ignoring: %m"); } else { char errbuf[256]; ERR_error_string_n(error, errbuf, sizeof(errbuf)); - log_debug("Failed to invoke SSL_shutdown: %s", errbuf); + log_debug("Failed to invoke SSL_shutdown, ignoring: %s", errbuf); } } + stream->dnstls_events = 0; + stream->dnstls_data.shutdown = false; + r = dnstls_flush_write_buffer(stream); if (r < 0) return r; - stream->dnstls_events = 0; - stream->dnstls_data.shutdown = false; dns_stream_unref(stream); return DNSTLS_STREAM_CLOSED; } else if (stream->dnstls_data.handshake <= 0) { @@ -204,6 +215,8 @@ int dnstls_stream_shutdown(DnsStream *stream, int error) { dns_stream_ref(stream); } + stream->dnstls_events = 0; + r = dnstls_flush_write_buffer(stream); if (r < 0) return r; @@ -222,11 +235,14 @@ int dnstls_stream_shutdown(DnsStream *stream, int error) { dns_stream_ref(stream); } return -EAGAIN; + } else if (ssl_error == SSL_ERROR_SYSCALL) { + if (errno > 0) + log_debug_errno(errno, "Failed to invoke SSL_shutdown, ignoring: %m"); } else { char errbuf[256]; ERR_error_string_n(ssl_error, errbuf, sizeof(errbuf)); - log_debug("Failed to invoke SSL_shutdown: %s", errbuf); + log_debug("Failed to invoke SSL_shutdown, ignoring: %s", errbuf); } } From 8e740110df8ca50ba442f995ca605ee16a0926c3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 3 Aug 2018 07:18:43 +0900 Subject: [PATCH 4/4] resolve: openssl: make dnstls_stream_{write,read}() may return zero --- src/resolve/resolved-dnstls-openssl.c | 31 ++++++++++++--------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/resolve/resolved-dnstls-openssl.c b/src/resolve/resolved-dnstls-openssl.c index c23211404b..a7a8b1152a 100644 --- a/src/resolve/resolved-dnstls-openssl.c +++ b/src/resolve/resolved-dnstls-openssl.c @@ -267,24 +267,24 @@ ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count) { ERR_clear_error(); ss = r = SSL_write(stream->dnstls_data.ssl, buf, count); if (r <= 0) { - error = SSL_get_error(stream->dnstls_data.ssl, ss); + error = SSL_get_error(stream->dnstls_data.ssl, r); if (IN_SET(error, SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE)) { stream->dnstls_events = error == SSL_ERROR_WANT_READ ? EPOLLIN : EPOLLOUT; - r = dnstls_flush_write_buffer(stream); - if (r < 0) - return r; - ss = -EAGAIN; + } else if (error == SSL_ERROR_ZERO_RETURN) { + stream->dnstls_events = 0; + ss = 0; } else { char errbuf[256]; ERR_error_string_n(error, errbuf, sizeof(errbuf)); log_debug("Failed to invoke SSL_write: %s", errbuf); + stream->dnstls_events = 0; ss = -EPIPE; } - } + } else + stream->dnstls_events = 0; - stream->dnstls_events = 0; r = dnstls_flush_write_buffer(stream); if (r < 0) return r; @@ -304,26 +304,23 @@ ssize_t dnstls_stream_read(DnsStream *stream, void *buf, size_t count) { ERR_clear_error(); ss = r = SSL_read(stream->dnstls_data.ssl, buf, count); if (r <= 0) { - error = SSL_get_error(stream->dnstls_data.ssl, ss); + error = SSL_get_error(stream->dnstls_data.ssl, r); if (IN_SET(error, SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE)) { stream->dnstls_events = error == SSL_ERROR_WANT_READ ? EPOLLIN : EPOLLOUT; - - /* flush write buffer in cache of renegotiation */ - r = dnstls_flush_write_buffer(stream); - if (r < 0) - return r; - ss = -EAGAIN; + } else if (error == SSL_ERROR_ZERO_RETURN) { + stream->dnstls_events = 0; + ss = 0; } else { char errbuf[256]; ERR_error_string_n(error, errbuf, sizeof(errbuf)); log_debug("Failed to invoke SSL_read: %s", errbuf); + stream->dnstls_events = 0; ss = -EPIPE; } - } - - stream->dnstls_events = 0; + } else + stream->dnstls_events = 0; /* flush write buffer in cache of renegotiation */ r = dnstls_flush_write_buffer(stream);