From 2ead369f8435918f3f15408b9394e580bcaf0910 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 10 Mar 2022 15:23:26 +0000 Subject: [PATCH] OpenSSL: track shutdown calls. Bug 2864 --- doc/doc-txt/ChangeLog | 5 +++++ src/src/macros.h | 7 ++++--- src/src/tls-gnu.c | 10 +++++++--- src/src/tls-openssl.c | 13 ++++++++----- src/src/transports/smtp.c | 19 +++++++++++++------ 5 files changed, 37 insertions(+), 17 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 5ba587b8e..1c799b664 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -95,6 +95,11 @@ JH/21 Remove the "allow_insecure_tainted_data" main config option and the JH/22 Fix static address-list lookups to properly return the matched item. Previously only the domain part was returned. +JH/23 Bug 2864: FreeBSD: fix transport hang after 4xx/5xx response. Previously + the call into OpenSSL to send a TLS Close was being repeated; this + resulted in the library waiting for the peer's Close. If that was never + sent we waited forever. Fix by tracking send calls. + Exim version 4.95 ----------------- diff --git a/src/src/macros.h b/src/src/macros.h index 92f2cc08f..659a70f48 100644 --- a/src/src/macros.h +++ b/src/src/macros.h @@ -1051,9 +1051,10 @@ enum { FILTER_UNSET, FILTER_FORWARD, FILTER_EXIM, FILTER_SIEVE }; /* Options on tls_close */ -#define TLS_NO_SHUTDOWN 0 -#define TLS_SHUTDOWN_NOWAIT 1 -#define TLS_SHUTDOWN_WAIT 2 +#define TLS_NO_SHUTDOWN 0 /* Just forget the context */ +#define TLS_SHUTDOWN_NOWAIT 1 /* Send alert; do not wait */ +#define TLS_SHUTDOWN_WAIT 2 /* Send alert & wait for peer's alert */ +#define TLS_SHUTDOWN_WONLY 3 /* only wait for peer's alert */ #ifdef COMPILE_UTILITY diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c index 1215f852e..622782369 100644 --- a/src/src/tls-gnu.c +++ b/src/src/tls-gnu.c @@ -3744,17 +3744,21 @@ if (!tlsp || tlsp->active.sock < 0) return; /* TLS was not active */ if (do_shutdown) { DEBUG(D_tls) debug_printf("tls_close(): shutting down TLS%s\n", - do_shutdown > 1 ? " (with response-wait)" : ""); + do_shutdown > TLS_SHUTDOWN_NOWAIT ? " (with response-wait)" : ""); tls_write(ct_ctx, NULL, 0, FALSE); /* flush write buffer */ #ifdef EXIM_TCP_CORK - if (do_shutdown > 1) + if (do_shutdown == TLS_SHUTDOWN_WAIT) (void) setsockopt(tlsp->active.sock, IPPROTO_TCP, EXIM_TCP_CORK, US &off, sizeof(off)); #endif + /* The library seems to have no way to only wait for a peer's + shutdown, so handle the same as TLS_SHUTDOWN_WAIT */ + ALARM(2); - gnutls_bye(state->session, do_shutdown > 1 ? GNUTLS_SHUT_RDWR : GNUTLS_SHUT_WR); + gnutls_bye(state->session, + do_shutdown > TLS_SHUTDOWN_NOWAIT ? GNUTLS_SHUT_RDWR : GNUTLS_SHUT_WR); ALARM_CLR(0); } diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c index d5c5778fc..7bf62f504 100644 --- a/src/src/tls-openssl.c +++ b/src/src/tls-openssl.c @@ -4519,22 +4519,25 @@ int * fdp = o_ctx ? &tls_out.active.sock : &tls_in.active.sock; if (*fdp < 0) return; /* TLS was not active */ -if (do_shutdown) +if (do_shutdown > TLS_NO_SHUTDOWN) { int rc; DEBUG(D_tls) debug_printf("tls_close(): shutting down TLS%s\n", - do_shutdown > 1 ? " (with response-wait)" : ""); + do_shutdown > TLS_SHUTDOWN_NOWAIT ? " (with response-wait)" : ""); tls_write(ct_ctx, NULL, 0, FALSE); /* flush write buffer */ - if ( (rc = SSL_shutdown(*sslp)) == 0 /* send "close notify" alert */ - && do_shutdown > 1) + if ( ( do_shutdown >= TLS_SHUTDOWN_WONLY + || (rc = SSL_shutdown(*sslp)) == 0 /* send "close notify" alert */ + ) + && do_shutdown > TLS_SHUTDOWN_NOWAIT + ) { #ifdef EXIM_TCP_CORK (void) setsockopt(*fdp, IPPROTO_TCP, EXIM_TCP_CORK, US &off, sizeof(off)); #endif ALARM(2); - rc = SSL_shutdown(*sslp); /* wait for response */ + rc = SSL_shutdown(*sslp); /* wait for response */ ALARM_CLR(0); } diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index e2c2680a0..524f18633 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -4085,7 +4085,7 @@ else sx->send_quit = FALSE; /* avoid sending it later */ #ifndef DISABLE_TLS - if (sx->cctx.tls_ctx) /* need to send TLS Close Notify */ + if (sx->cctx.tls_ctx && sx->send_tlsclose) /* need to send TLS Close Notify */ { # ifdef EXIM_TCP_CORK /* Use _CORK to get Close Notify in FIN segment */ (void) setsockopt(sx->cctx.sock, IPPROTO_TCP, EXIM_TCP_CORK, US &on, sizeof(on)); @@ -4429,7 +4429,8 @@ if (!sx->ok) # ifndef DISABLE_TLS if (sx->cctx.tls_ctx) { - tls_close(sx->cctx.tls_ctx, TLS_SHUTDOWN_WAIT); + tls_close(sx->cctx.tls_ctx, + sx->send_tlsclose ? TLS_SHUTDOWN_WAIT : TLS_SHUTDOWN_WONLY); sx->cctx.tls_ctx = NULL; } # endif @@ -4640,7 +4641,8 @@ if (sx->completed_addr && sx->ok && sx->send_quit) a new EHLO. If we don't get a good response, we don't attempt to pass the socket on. */ - tls_close(sx->cctx.tls_ctx, TLS_SHUTDOWN_WAIT); + tls_close(sx->cctx.tls_ctx, + sx->send_tlsclose ? TLS_SHUTDOWN_WAIT : TLS_SHUTDOWN_WONLY); sx->send_tlsclose = FALSE; sx->cctx.tls_ctx = NULL; tls_out.active.sock = -1; @@ -4742,7 +4744,7 @@ if (sx->send_quit) { /* Use _MORE to get QUIT in FIN segment */ (void)smtp_write_command(sx, SCMD_MORE, "QUIT\r\n"); #ifndef DISABLE_TLS - if (sx->cctx.tls_ctx) + if (sx->cctx.tls_ctx && sx->send_tlsclose) { # ifdef EXIM_TCP_CORK /* Use _CORK to get TLS Close Notify in FIN segment */ (void) setsockopt(sx->cctx.sock, IPPROTO_TCP, EXIM_TCP_CORK, US &on, sizeof(on)); @@ -4797,10 +4799,15 @@ if (sx->send_quit || tcw_done && !tcw) while (!sigalrm_seen && n > 0); ALARM_CLR(0); + if (sx->send_tlsclose) + { # ifdef EXIM_TCP_CORK - (void) setsockopt(sx->cctx.sock, IPPROTO_TCP, EXIM_TCP_CORK, US &on, sizeof(on)); + (void) setsockopt(sx->cctx.sock, IPPROTO_TCP, EXIM_TCP_CORK, US &on, sizeof(on)); # endif - tls_close(sx->cctx.tls_ctx, TLS_SHUTDOWN_WAIT); + tls_close(sx->cctx.tls_ctx, TLS_SHUTDOWN_WAIT); + } + else + tls_close(sx->cctx.tls_ctx, TLS_SHUTDOWN_WONLY); sx->cctx.tls_ctx = NULL; } #endif -- 2.30.2