From 51ffcca6f1f6005c37c25144ed8b30f5e8a094e9 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 21 Jan 2021 17:34:55 +0000 Subject: [PATCH] Avoid bare TCP ACKs during TLS-on-connect startup. We can't get the QUICKACK turned off on the accepted socket fast enough to stop the ACK for the ClientHello - but we get the rest, under OpenSSL. --- src/src/smtp_in.c | 31 ++++++++++++++++++------------- src/src/smtp_out.c | 6 +++--- src/src/transports/smtp.c | 8 ++++---- test/scripts/1100-Basic-TLS/1160 | 25 +++++++++++++++++++++++-- test/src/client.c | 7 +++++++ 5 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 13cba2ec2..4cc619014 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -2893,7 +2893,7 @@ if (!f.sender_host_unknown) if (smtp_batched_input) return TRUE; /* If valid Proxy Protocol source is connecting, set up session. - * Failure will not allow any SMTP function other than QUIT. */ +Failure will not allow any SMTP function other than QUIT. */ #ifdef SUPPORT_PROXY proxy_session = FALSE; @@ -2902,16 +2902,21 @@ if (check_proxy_protocol_host()) setup_proxy_protocol_host(); #endif +#ifdef TCP_QUICKACK /* Avoid pure-ACKs while in tls protocol pingpong phase */ +(void) setsockopt(fileno(smtp_in), IPPROTO_TCP, TCP_QUICKACK, + US &off, sizeof(off)); +#endif + /* Start up TLS if tls_on_connect is set. This is for supporting the legacy smtps port for use with older style SSL MTAs. */ #ifndef DISABLE_TLS - if (tls_in.on_connect) - { - if (tls_server_start(&user_msg) != OK) - return smtp_log_tls_fail(user_msg); - cmd_list[CMD_LIST_TLS_AUTH].is_mail_cmd = TRUE; - } +if (tls_in.on_connect) + { + if (tls_server_start(&user_msg) != OK) + return smtp_log_tls_fail(user_msg); + cmd_list[CMD_LIST_TLS_AUTH].is_mail_cmd = TRUE; + } #endif /* Run the connect ACL if it exists */ @@ -3912,6 +3917,12 @@ os_non_restarting_signal(SIGTERM, command_sigterm_handler); if (smtp_batched_input) return smtp_setup_batch_msg(); +#ifdef TCP_QUICKACK +if (smtp_in) /* Avoid pure-ACKs while in cmd pingpong phase */ + (void) setsockopt(fileno(smtp_in), IPPROTO_TCP, TCP_QUICKACK, + US &off, sizeof(off)); +#endif + /* Deal with SMTP commands. This loop is exited by setting done to a POSITIVE value. The values are 2 larger than the required yield of the function. */ @@ -3969,12 +3980,6 @@ while (done <= 0) } #endif -#ifdef TCP_QUICKACK - if (smtp_in) /* Avoid pure-ACKs while in cmd pingpong phase */ - (void) setsockopt(fileno(smtp_in), IPPROTO_TCP, TCP_QUICKACK, - US &off, sizeof(off)); -#endif - switch(smtp_read_command( #ifndef DISABLE_PIPE_CONNECT !fl.pipe_connect_acceptable, diff --git a/src/src/smtp_out.c b/src/src/smtp_out.c index bb7a0e3b3..2d2fd2180 100644 --- a/src/src/smtp_out.c +++ b/src/src/smtp_out.c @@ -329,12 +329,12 @@ else HDEBUG(D_transport|D_acl|D_v) debug_printf("sending %ld nonTFO early-data\n", (long)early_data->len); -#ifdef TCP_QUICKACK - (void) setsockopt(sock, IPPROTO_TCP, TCP_QUICKACK, US &off, sizeof(off)); -#endif if (send(sock, early_data->data, early_data->len, 0) < 0) save_errno = errno; } +#ifdef TCP_QUICKACK + (void) setsockopt(sock, IPPROTO_TCP, TCP_QUICKACK, US &off, sizeof(off)); +#endif } /* Either bind() or connect() failed */ diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index 301d84c2e..ee5e49e57 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -2107,6 +2107,10 @@ PIPE_CONNECT_RETRY: sx->send_quit = FALSE; return DEFER; } +#ifdef TCP_QUICKACK + (void) setsockopt(sx->cctx.sock, IPPROTO_TCP, TCP_QUICKACK, US &off, + sizeof(off)); +#endif } /* Expand the greeting message while waiting for the initial response. (Makes sense if helo_data contains ${lookup dnsdb ...} stuff). The expansion is @@ -2152,10 +2156,6 @@ will be? Somehow I doubt it. */ else #endif { -#ifdef TCP_QUICKACK - (void) setsockopt(sx->cctx.sock, IPPROTO_TCP, TCP_QUICKACK, US &off, - sizeof(off)); -#endif if (!smtp_reap_banner(sx)) goto RESPONSE_FAILED; } diff --git a/test/scripts/1100-Basic-TLS/1160 b/test/scripts/1100-Basic-TLS/1160 index ce7298e47..e57867e1c 100644 --- a/test/scripts/1100-Basic-TLS/1160 +++ b/test/scripts/1100-Basic-TLS/1160 @@ -4,8 +4,29 @@ # For GnuTLS, additionally run the daemon under sudo. # Tell wireshark to use DIR/spool/sslkeys for Master Secret log, and decode TCP/1225 as TLS, TLS/1225 as SMTP # -# sudo exim -DSERVER=server -d+tls -bd -oX PORT_D -exim -DSERVER=server -bd -oX PORT_D +# We get (TLS1.3 , OpenSSL): +# SYN > +# < SYN,ACK +# ACK > +# Client Hello > +# < Server Hello, Change Ciph, Extensions, Cert, Cert Verify, Finished +# Change Ciph,Finsh > +# < Banner +# EHLO > +# < EHLO resp +# MAIL,RCPT,DATA > +# < ACK,ACK,DATA-go-ahead +# +# GnuTLS splits both the server records and the client response pair over two TCP segments: +# Client Hello > +# < Server Hello, Change Ciph +# Change Ciph > +# < Extensins, Cert, Cert Verify, Finished +# Finished > +# (otherwise the same). The extra segments are piplined and do not incur an extra roundtrip time. +# +# exim -DSERVER=server -bd -oX PORT_D +sudo exim -DSERVER=server -d+tls -bd -oX PORT_D **** exim CALLER@test.ex Test message. Contains FF: ΓΏ diff --git a/test/src/client.c b/test/src/client.c index 9190af068..9beaf25bb 100644 --- a/test/src/client.c +++ b/test/src/client.c @@ -1234,6 +1234,13 @@ if (rc < 0) exit(85); } +#ifdef TCP_QUICKACK + { + int off = 0; + (void) setsockopt(srv.sock, IPPROTO_TCP, TCP_QUICKACK, US &off, sizeof(off)); + } +#endif + printf("connected\n"); -- 2.30.2