Enforce STARTTLS sync point, client side
authorJeremy Harris <jgh146exb@wizmail.org>
Thu, 30 Jul 2020 19:16:01 +0000 (20:16 +0100)
committerJeremy Harris <jgh146exb@wizmail.org>
Sun, 2 Aug 2020 10:10:35 +0000 (11:10 +0100)
Tested by appending to the "220 TLS go ahead\r\n" at src/tls-gnu.c line 2500
Testcase 2008, string "synch error before connect" becomes visible in log.

To get the debug output:
  Testcase 2008, initial block; add -d+all to the exi -qf

src/src/transports/smtp.c

index 46663b0523dda55c1c0adea503eef8e2d32f7731..341acde2d6ab4275fbd997bb8b2ca3d14522be37 100644 (file)
@@ -2461,9 +2461,7 @@ if (  smtp_peer_options & OPTION_TLS
       /* TLS negotiation failed; give an error. From outside, this function may
       be called again to try in clear on a new connection, if the options permit
       it for this host. */
       /* TLS negotiation failed; give an error. From outside, this function may
       be called again to try in clear on a new connection, if the options permit
       it for this host. */
-#ifdef USE_GNUTLS
-  GNUTLS_CONN_FAILED:
-#endif
+  TLS_CONN_FAILED:
       DEBUG(D_tls) debug_printf("TLS session fail: %s\n", tls_errstr);
 
 # ifdef SUPPORT_DANE
       DEBUG(D_tls) debug_printf("TLS session fail: %s\n", tls_errstr);
 
 # ifdef SUPPORT_DANE
@@ -2485,7 +2483,23 @@ if (  smtp_peer_options & OPTION_TLS
       goto TLS_FAILED;
       }
 
       goto TLS_FAILED;
       }
 
-    /* TLS session is set up */
+    /* TLS session is set up.  Check the inblock fill level.  If there is
+    content then as we have not yet done a tls read it must have arrived before
+    the TLS handshake, in-clear.  That violates the sync requirement of the
+    STARTTLS RFC, so fail. */
+
+    if (sx->inblock.ptr != sx->inblock.ptrend)
+      {
+      DEBUG(D_tls)
+       {
+       int i = sx->inblock.ptrend - sx->inblock.ptr;
+       debug_printf("unused data in input buffer after ack for STARTTLS:\n"
+         "'%.*s'%s\n",
+         i > 100 ? 100 : i, sx->inblock.ptr, i > 100 ? "..." : "");
+       }
+      tls_errstr = US"synch error before connect";
+      goto TLS_CONN_FAILED;
+      }
 
     smtp_peer_options_wrap = smtp_peer_options;
     for (address_item * addr = sx->addrlist; addr; addr = addr->next)
 
     smtp_peer_options_wrap = smtp_peer_options;
     for (address_item * addr = sx->addrlist; addr; addr = addr->next)
@@ -2590,7 +2604,7 @@ if (tls_out.active.sock >= 0)
       Can it do that, with all the flexibility we need? */
 
       tls_errstr = US"error on first read";
       Can it do that, with all the flexibility we need? */
 
       tls_errstr = US"error on first read";
-      goto GNUTLS_CONN_FAILED;
+      goto TLS_CONN_FAILED;
       }
 #else
       goto RESPONSE_FAILED;
       }
 #else
       goto RESPONSE_FAILED;