From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Thu, 21 Jan 2021 17:34:55 +0000 (+0000)
Subject:     Avoid bare TCP ACKs during TLS-on-connect startup.
X-Git-Url: https://git.exim.org/users/heiko/exim.git/commitdiff_plain/51ffcca6f1f6005c37c25144ed8b30f5e8a094e9?ds=sidebyside

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.
---

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");