From 97cfe3942f67200f77f6ae9b302409075e4e5792 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Fri, 22 Jan 2021 19:58:54 +0000 Subject: [PATCH] Fix getting non-TLS QUIT in FIN segment Linux was behaving oddly with the TCP_CORK method, and using MSG_MORE is one fewer syscall. --- src/src/tls-gnu.c | 2 ++ src/src/tls-openssl.c | 2 ++ src/src/transports/smtp.c | 40 ++++++++++++++++++--------------------- test/runtest | 2 +- test/stderr/0143 | 2 +- test/stderr/0388 | 2 +- test/stderr/1157 | 6 +++--- test/stderr/2035 | 2 +- test/stderr/2135 | 2 +- test/stderr/4052 | 2 +- test/stderr/4520 | 2 +- 11 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c index b14bca483..46547d732 100644 --- a/src/src/tls-gnu.c +++ b/src/src/tls-gnu.c @@ -3513,6 +3513,8 @@ tls_support * tlsp = state->tlsp; if (!tlsp || tlsp->active.sock < 0) return; /* TLS was not active */ +tls_write(ct_ctx, NULL, 0, FALSE); /* flush write buffer */ + if (shutdown) { DEBUG(D_tls) debug_printf("tls_close(): shutting down TLS%s\n", diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c index 0dfd8e01a..0d653a445 100644 --- a/src/src/tls-openssl.c +++ b/src/src/tls-openssl.c @@ -4147,6 +4147,8 @@ int *fdp = o_ctx ? &tls_out.active.sock : &tls_in.active.sock; if (*fdp < 0) return; /* TLS was not active */ +tls_write(ct_ctx, NULL, 0, FALSE); /* flush write buffer */ + if (shutdown) { int rc; diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index eb6b77416..0a6bfde18 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -4380,35 +4380,21 @@ propagate it from the initial } /* End off tidily with QUIT unless the connection has died or the socket has -been passed to another process. There has been discussion on the net about what -to do after sending QUIT. The wording of the RFC suggests that it is necessary -to wait for a response, but on the other hand, there isn't anything one can do -with an error response, other than log it. Exim used to do that. However, -further discussion suggested that it is positively advantageous not to wait for -the response, but to close the session immediately. This is supposed to move -the TCP/IP TIME_WAIT state from the server to the client, thereby removing some -load from the server. (Hosts that are both servers and clients may not see much -difference, of course.) Further discussion indicated that this was safe to do -on Unix systems which have decent implementations of TCP/IP that leave the -connection around for a while (TIME_WAIT) after the application has gone away. -This enables the response sent by the server to be properly ACKed rather than -timed out, as can happen on broken TCP/IP implementations on other OS. - -This change is being made on 31-Jul-98. After over a year of trouble-free -operation, the old commented-out code was removed on 17-Sep-99. */ +been passed to another process. */ SEND_QUIT: if (sx->send_quit) - { -#ifdef EXIM_TCP_CORK - (void) setsockopt(sx->cctx.sock, IPPROTO_TCP, EXIM_TCP_CORK, US &on, sizeof(on)); -#endif - (void)smtp_write_command(sx, SCMD_FLUSH, "QUIT\r\n"); - } + /* Use _MORE to get QUIT in FIN segment */ + (void)smtp_write_command(sx, SCMD_MORE, "QUIT\r\n"); END_OFF: #ifndef DISABLE_TLS +# ifdef EXIM_TCP_CORK +if (sx->cctx.tls_ctx) /* Use _CORK to get TLS Close Notify in FIN segment */ + (void) setsockopt(sx->cctx.sock, IPPROTO_TCP, EXIM_TCP_CORK, US &on, sizeof(on)); +# endif + tls_close(sx->cctx.tls_ctx, TLS_SHUTDOWN_NOWAIT); sx->cctx.tls_ctx = NULL; #endif @@ -4426,7 +4412,17 @@ case continue_more won't get set. */ HDEBUG(D_transport|D_acl|D_v) debug_printf_indent(" SMTP(close)>>\n"); if (sx->send_quit) { + /* This flushes data queued in the socket, being the QUIT and any TLS Close, + sending them along with the client FIN flag. Us (we hope) sending FIN first + means we (client) take the TIME_WAIT state, so the server (which likely has a + higher connection rate) does no have to. */ + shutdown(sx->cctx.sock, SHUT_WR); + + /* Wait for (we hope) ack of our QUIT, and a server FIN. Discard any data + received, then discard the socket. Any packet received after then, or receive + data still in the socket, will get a RST - hence the pause/drain. */ + millisleep(20); testharness_pause_ms(200); if (fcntl(sx->cctx.sock, F_SETFL, O_NONBLOCK) == 0) diff --git a/test/runtest b/test/runtest index ba2450c45..c52afceaa 100755 --- a/test/runtest +++ b/test/runtest @@ -1289,7 +1289,7 @@ RESET_AFTER_EXTRA_LINE_READ: next if /\w+ in keep_environment\? (yes|no)/; # Sizes vary with test hostname - s/^cmd buf flush \d+ bytes$/cmd buf flush ddd bytes/; + s/^cmd buf flush \d+ bytes/cmd buf flush ddd bytes/; # Spool filesystem free space changes on different systems. s/^((?:spool|log) directory space =) -?\d+K (inodes =)\s*-?\d+/$1 nnnnnK $2 nnnnn/; diff --git a/test/stderr/0143 b/test/stderr/0143 index e91e97a52..f7f14f6ec 100644 --- a/test/stderr/0143 +++ b/test/stderr/0143 @@ -50,7 +50,7 @@ transport_check_waiting entered no messages waiting for 127.0.0.1 transport_check_waiting: FALSE SMTP>> QUIT -cmd buf flush ddd bytes +cmd buf flush ddd bytes (more expected) SMTP(close)>> Leaving my_smtp transport LOG: MAIN diff --git a/test/stderr/0388 b/test/stderr/0388 index 0b66b639c..e23c903f1 100644 --- a/test/stderr/0388 +++ b/test/stderr/0388 @@ -99,7 +99,7 @@ LOG: MAIN H=127.0.0.1 [127.0.0.1]: SMTP error from remote mail server after RCPT TO:: 451 Temporary error added retry item for R:x@y: errno=-44 more_errno=dd,A flags=0 SMTP>> QUIT -cmd buf flush ddd bytes +cmd buf flush ddd bytes (more expected) SMTP(close)>> set_process_info: pppp delivering 10HmaX-0005vi-00: just tried 127.0.0.1 [127.0.0.1]:PORT_S for x@y: result OK address match test: subject=*@127.0.0.1 pattern=* diff --git a/test/stderr/1157 b/test/stderr/1157 index 368ccc07d..333527603 100644 --- a/test/stderr/1157 +++ b/test/stderr/1157 @@ -73,7 +73,7 @@ cmd buf flush ddd bytes SMTP<< 354 Enter message, ending with "." on a line by itself SMTP<< 250 OK id=10HmbC-0005vi-00 SMTP>> QUIT -cmd buf flush ddd bytes +cmd buf flush ddd bytes (more expected) SMTP(close)>> >>>>>>>>>>>>>>>> Exim pid=pppp (tls-proxy) terminating with rc=0 >>>>>>>>>>>>>>>> LOG: MAIN @@ -159,7 +159,7 @@ cmd buf flush ddd bytes SMTP<< 354 Enter message, ending with "." on a line by itself SMTP<< 250 OK id=10HmbI-0005vi-00 SMTP>> QUIT -cmd buf flush ddd bytes +cmd buf flush ddd bytes (more expected) SMTP(close)>> >>>>>>>>>>>>>>>> Exim pid=pppp (tls-proxy) terminating with rc=0 >>>>>>>>>>>>>>>> LOG: MAIN @@ -282,7 +282,7 @@ cmd buf flush ddd bytes SMTP<< 354 Enter message, ending with "." on a line by itself SMTP<< 250 OK id=10HmbO-0005vi-00 SMTP>> QUIT -cmd buf flush ddd bytes +cmd buf flush ddd bytes (more expected) SMTP(close)>> LOG: MAIN => usery@test.ex R=client T=send_to_server H=127.0.0.1 [127.0.0.1]* X=TLS1.x:ke-RSA-AES256-SHAnnn:xxx CV=yes C="250 OK id=10HmbO-0005vi-00" diff --git a/test/stderr/2035 b/test/stderr/2035 index 5a69f577f..75c7a82f8 100644 --- a/test/stderr/2035 +++ b/test/stderr/2035 @@ -63,7 +63,7 @@ writing data block fd=dddd size=sss timeout=300 SMTP<< 250 OK id=10HmaY-0005vi-00 ok=1 send_quit=1 send_rset=0 continue_more=0 yield=0 first_address is NULL SMTP>> QUIT -cmd buf flush ddd bytes +cmd buf flush ddd bytes (more expected) SMTP(close)>> >>>>>>>>>>>>>>>> Exim pid=pppp (tls-proxy) terminating with rc=0 >>>>>>>>>>>>>>>> Leaving t1 transport diff --git a/test/stderr/2135 b/test/stderr/2135 index 5a69f577f..75c7a82f8 100644 --- a/test/stderr/2135 +++ b/test/stderr/2135 @@ -63,7 +63,7 @@ writing data block fd=dddd size=sss timeout=300 SMTP<< 250 OK id=10HmaY-0005vi-00 ok=1 send_quit=1 send_rset=0 continue_more=0 yield=0 first_address is NULL SMTP>> QUIT -cmd buf flush ddd bytes +cmd buf flush ddd bytes (more expected) SMTP(close)>> >>>>>>>>>>>>>>>> Exim pid=pppp (tls-proxy) terminating with rc=0 >>>>>>>>>>>>>>>> Leaving t1 transport diff --git a/test/stderr/4052 b/test/stderr/4052 index d4866ed88..22f05589b 100644 --- a/test/stderr/4052 +++ b/test/stderr/4052 @@ -57,7 +57,7 @@ transport_check_waiting entered no messages waiting for 127.0.0.1 transport_check_waiting: FALSE SMTP>> QUIT -cmd buf flush ddd bytes +cmd buf flush ddd bytes (more expected) SMTP(close)>> Leaving smtp transport LOG: MAIN diff --git a/test/stderr/4520 b/test/stderr/4520 index 6087e66e6..d88fc98da 100644 --- a/test/stderr/4520 +++ b/test/stderr/4520 @@ -49,7 +49,7 @@ dkim-signature:v=1;{SP}a=rsa-sha256;{SP}q=dns/txt;{SP}c=relaxed/relaxed;{SP}d=te DKIM [test.ex] Header sha256 computed: 241e16230df5723d899cfae9474c6b376a2ab1f81d1094e358f50ffd0e0067b3 SMTP<< 250 OK id=10HmbL-0005vi-00 SMTP>> QUIT -cmd buf flush ddd bytes +cmd buf flush ddd bytes (more expected) SMTP(close)>> LOG: MAIN => d@test.ex R=client T=send_to_server H=ip4.ip4.ip4.ip4 [ip4.ip4.ip4.ip4] C="250 OK id=10HmbL-0005vi-00" -- 2.30.2