Enforce minimum recommended SMTP timeouts, apply to EHLO/LHLO as well.

Short timeouts could cause fetchmail to not wait long enough for the
"250 Ok" after shipping a long message, particularly with synchronous
mailers and extensive spam filtering. This caused fetchmail to re-fetch
long messages.

While the actual fix is making sure that the timeout is no shorter than
the time the SMTP server takes to process the message, we now enforce
the minimum RFC-5321 recommended timeouts even if the user configures a
lower timeout.

This is to fix Berlios Bug #10972, reported by Viktor Binzberger.

NOTE: it is untested whether we will properly delete the message from
the POP3/IMAP server or mark it as seen, as the upstream server may
close the connection sooner.

svn path=/branches/BRANCH_6-3/; revision=5338

7 files changed, 55 insertions, 27 deletions

diff --git a/NEWS b/NEWS
index 00d36e17..e7c007a4 100644
--- a/NEWS
+++ b/NEWS
@@ -81,6 +81,18 @@ fetchmail 6.3.10 (not yet released):
 * Non-delivery notice ("bounce mail") now mentions the original reason again,
   before the address list. This fixes a regression introduced in 6.3.0.
 * Several compiler warnings were fixed.
+* The minimum recommended SMTP (RFC-5321) timeouts are enforced to leave
+  sufficient time for the listener to respond. Some synchronous listeners,
+  particularly when used with spam filtering and other policy enforcement
+  services, take extended amounts of time to process messages after the sender,
+  recipient, or data block and EOM line. This can cause fetchmail to not wait
+  long enough for the "250 Ok" and make fetchmail believe the message wasn't
+  properly delivered when in fact it was; fetchmail would then retry the
+  download next time and never make progress.
+  Fixes Berlios Bug #10972, reported by Viktor Binzberger.
+* The ESMTP/LMTP client will now apply an application-specific timeout while
+  waiting for the EHLO/LHLO response, rather than wait for the server or TCP
+  connection timeout.
 
 # CHANGES
 * Make the comparison of the SSL fingerprints case insensitive, to
diff --git a/TODO.txt b/TODO.txt
index e92cc4d8..d54798c8 100644
--- a/TODO.txt
+++ b/TODO.txt
@@ -1,8 +1,6 @@
 Note that there is a separate todo.html with different content than this.
 
 soon - MUST:
-- BerliOS Bug #10972, Fetchmail refetches big messages repeatedly (SMTP listener
-  issue)
 - Debian Bug #475239, MIME decoder may break up words (need to quote results)
 - Replace sscanf/fscanf by functions that do range checking (strtol/strtoul) :-/
   See http://sourceware.org/bugzilla/show_bug.cgi?id=6453
diff --git a/etrn.c b/etrn.c
index 27441604..ac416c86 100644
--- a/etrn.c
+++ b/etrn.c
@@ -26,7 +26,7 @@ static int etrn_ok (int sock, char *argbuf)
     int ok;
 
     (void)argbuf;
-    ok = SMTP_ok(sock, SMTP_MODE);
+    ok = SMTP_ok(sock, SMTP_MODE, TIMEOUT_DEFAULT);
     if (ok == SM_UNRECOVERABLE)
 	return(PS_PROTOCOL);
     else
diff --git a/odmr.c b/odmr.c
index cb0d559a..a20d89f1 100644
--- a/odmr.c
+++ b/odmr.c
@@ -36,7 +36,7 @@ static int odmr_ok (int sock, char *argbuf)
     int ok;
 
     (void)argbuf;
-    ok = SMTP_ok(sock, SMTP_MODE);
+    ok = SMTP_ok(sock, SMTP_MODE, TIMEOUT_DEFAULT);
     if (ok == SM_UNRECOVERABLE)
 	return(PS_PROTOCOL);
     else
diff --git a/sink.c b/sink.c
index d6e4f430..78630a02 100644
--- a/sink.c
+++ b/sink.c
@@ -158,7 +158,7 @@ int smtp_open(struct query *ctl)
 	    }
 
 	    /* first, probe for ESMTP */
-	    if (SMTP_ok(ctl->smtp_socket, ctl->smtphostmode) == SM_OK &&
+	    if (SMTP_ok(ctl->smtp_socket, ctl->smtphostmode, TIMEOUT_STARTSMTP) == SM_OK &&
 		    SMTP_ehlo(ctl->smtp_socket, ctl->smtphostmode, id_me,
 			ctl->server.esmtp_name, ctl->server.esmtp_password,
 			&ctl->server.esmtp_options) == SM_OK)
@@ -186,7 +186,7 @@ int smtp_open(struct query *ctl)
 		}
 	    }
 
-	    if (SMTP_ok(ctl->smtp_socket, ctl->smtphostmode) == SM_OK &&
+	    if (SMTP_ok(ctl->smtp_socket, ctl->smtphostmode, TIMEOUT_STARTSMTP) == SM_OK &&
 		    SMTP_helo(ctl->smtp_socket, ctl->smtphostmode, id_me) == SM_OK)
 		break;  /* success */
 
@@ -282,7 +282,7 @@ static int send_bouncemail(struct query *ctl, struct msgblk *msg,
     if ((sock = SockOpen("localhost", SMTP_PORT, NULL, &ai1)) == -1)
 	return(FALSE);
 
-    if (SMTP_ok(sock, SMTP_MODE) != SM_OK)
+    if (SMTP_ok(sock, SMTP_MODE, TIMEOUT_STARTSMTP) != SM_OK)
     {
 	SockClose(sock);
 	return FALSE;
@@ -1422,7 +1422,7 @@ int close_sink(struct query *ctl, struct msgblk *msg, flag forward)
 	{
 	    if (lmtp_responses == 0)
 	    {
-		SMTP_ok(ctl->smtp_socket, ctl->smtphostmode);
+		SMTP_ok(ctl->smtp_socket, ctl->smtphostmode, TIMEOUT_EOM);
 
 		/*
 		 * According to RFC2033, 503 is the only legal response
@@ -1455,7 +1455,7 @@ int close_sink(struct query *ctl, struct msgblk *msg, flag forward)
 		responses = (char **)xmalloc(sizeof(char *) * lmtp_responses);
 		for (errors = i = 0; i < lmtp_responses; i++)
 		{
-		    if ((smtp_err = SMTP_ok(ctl->smtp_socket, ctl->smtphostmode))
+		    if ((smtp_err = SMTP_ok(ctl->smtp_socket, ctl->smtphostmode, TIMEOUT_EOM))
 			    == SM_UNRECOVERABLE)
 		    {
 			smtp_close(ctl, 0);
diff --git a/smtp.c b/smtp.c
index 96a708e3..61a6d93f 100644
--- a/smtp.c
+++ b/smtp.c
@@ -39,6 +39,7 @@ static struct opt extensions[] =
 
 char smtp_response[MSGBUFSIZE];
 
+/* XXX: this must not be used for LMTP! */
 int SMTP_helo(int sock, char smtp_mode, const char *host)
 /* send a "HELO" message to the SMTP listener */
 {
@@ -47,7 +48,7 @@ int SMTP_helo(int sock, char smtp_mode, const char *host)
   SockPrintf(sock,"HELO %s\r\n", host);
   if (outlevel >= O_MONITOR)
       report(stdout, "%cMTP> HELO %s\n", smtp_mode, host);
-  ok = SMTP_ok(sock, smtp_mode);
+  ok = SMTP_ok(sock, smtp_mode, TIMEOUT_HELO);
   return ok;
 }
 
@@ -107,7 +108,7 @@ static void SMTP_auth(int sock, char smtp_mode, char *username, char *password,
 
 		to64frombits(b64buf, tmp, strlen(tmp));
 		SockPrintf(sock, "%s\r\n", b64buf);
-		SMTP_ok(sock, smtp_mode);
+		SMTP_ok(sock, smtp_mode, TIMEOUT_DEFAULT);
 	}
 	else if (strstr(buf, "PLAIN")) {
 		int len;
@@ -123,7 +124,7 @@ static void SMTP_auth(int sock, char smtp_mode, char *username, char *password,
 		}
 		to64frombits(b64buf, tmp, len);
 		SockPrintf(sock, "AUTH PLAIN %s\r\n", b64buf);
-		SMTP_ok(sock, smtp_mode);
+		SMTP_ok(sock, smtp_mode, TIMEOUT_DEFAULT);
 	}
 	else if (strstr(buf, "LOGIN")) {
 		if (outlevel >= O_MONITOR)
@@ -162,7 +163,7 @@ static void SMTP_auth(int sock, char smtp_mode, char *username, char *password,
 		}
 		to64frombits(b64buf, password, strlen(password));
 		SockPrintf(sock, "%s\r\n", b64buf);
-		SMTP_ok(sock, smtp_mode);
+		SMTP_ok(sock, smtp_mode, TIMEOUT_DEFAULT);
 	}
 	return;
 }
@@ -172,17 +173,25 @@ int SMTP_ehlo(int sock, char smtp_mode, const char *host, char *name, char *pass
 {
   struct opt *hp;
   char auth_response[511];
+  SIGHANDLERTYPE alrmsave;
+  const int tmout = (mytimeout >= TIMEOUT_HELO ? mytimeout : TIMEOUT_HELO);
 
   SockPrintf(sock,"%cHLO %s\r\n", (smtp_mode == 'S') ? 'E' : smtp_mode, host);
   if (outlevel >= O_MONITOR)
       report(stdout, "%cMTP> %cHLO %s\n", 
 	    smtp_mode, (smtp_mode == 'S') ? 'E' : smtp_mode, host);
 
+  alrmsave = set_signal_handler(SIGALRM, null_signal_handler);
+  set_timeout(tmout);
+
   *opt = 0;
   while ((SockRead(sock, smtp_response, sizeof(smtp_response)-1)) != -1)
   {
       int  n = strlen(smtp_response);
 
+      set_timeout(0);
+      (void)set_signal_handler(SIGALRM, alrmsave);
+
       if (smtp_response[strlen(smtp_response)-1] == '\n')
 	  smtp_response[strlen(smtp_response)-1] = '\0';
       if (smtp_response[strlen(smtp_response)-1] == '\r')
@@ -206,6 +215,9 @@ int SMTP_ehlo(int sock, char smtp_mode, const char *host, char *name, char *pass
       }
       else if (smtp_response[3] != '-')
 	  return SM_ERROR;
+
+      alrmsave = set_signal_handler(SIGALRM, null_signal_handler);
+      set_timeout(tmout);
   }
   return SM_UNRECOVERABLE;
 }
@@ -225,7 +237,7 @@ int SMTP_from(int sock, char smtp_mode, const char *from, const char *opts)
     SockPrintf(sock,"%s\r\n", buf);
     if (outlevel >= O_MONITOR)
 	report(stdout, "%cMTP> %s\n", smtp_mode, buf);
-    ok = SMTP_ok(sock, smtp_mode);
+    ok = SMTP_ok(sock, smtp_mode, TIMEOUT_MAIL);
     return ok;
 }
 
@@ -237,7 +249,7 @@ int SMTP_rcpt(int sock, char smtp_mode, const char *to)
   SockPrintf(sock,"RCPT TO:<%s>\r\n", to);
   if (outlevel >= O_MONITOR)
       report(stdout, "%cMTP> RCPT TO:<%s>\n", smtp_mode, to);
-  ok = SMTP_ok(sock, smtp_mode);
+  ok = SMTP_ok(sock, smtp_mode, TIMEOUT_RCPT);
   return ok;
 }
 
@@ -249,7 +261,7 @@ int SMTP_data(int sock, char smtp_mode)
   SockPrintf(sock,"DATA\r\n");
   if (outlevel >= O_MONITOR)
       report(stdout, "%cMTP> DATA\n", smtp_mode);
-  ok = SMTP_ok(sock, smtp_mode);
+  ok = SMTP_ok(sock, smtp_mode, TIMEOUT_DATA);
   return ok;
 }
 
@@ -261,7 +273,7 @@ int SMTP_rset(int sock, char smtp_mode)
   SockPrintf(sock,"RSET\r\n");
   if (outlevel >= O_MONITOR)
       report(stdout, "%cMTP> RSET\n", smtp_mode);
-  ok = SMTP_ok(sock, smtp_mode);
+  ok = SMTP_ok(sock, smtp_mode, TIMEOUT_DEFAULT);
   return ok;
 }
 
@@ -273,15 +285,13 @@ int SMTP_quit(int sock, char smtp_mode)
   SockPrintf(sock,"QUIT\r\n");
   if (outlevel >= O_MONITOR)
       report(stdout, "%cMTP> QUIT\n", smtp_mode);
-  ok = SMTP_ok(sock, smtp_mode);
+  ok = SMTP_ok(sock, smtp_mode, TIMEOUT_DEFAULT);
   return ok;
 }
 
 int SMTP_eom(int sock, char smtp_mode)
 /* send a message data terminator to the SMTP listener */
 {
-  int ok;
-
   SockPrintf(sock,".\r\n");
   if (outlevel >= O_MONITOR)
       report(stdout, "%cMTP>. (EOM)\n", smtp_mode);
@@ -290,23 +300,21 @@ int SMTP_eom(int sock, char smtp_mode)
    * When doing LMTP, must process many of these at the outer level. 
    */
   if (smtp_mode == 'S')
-      ok = SMTP_ok(sock, smtp_mode);
+      return SMTP_ok(sock, smtp_mode, TIMEOUT_EOM);
   else
-      ok = SM_OK;
-
-  return ok;
+      return SM_OK;
 }
 
 time_t last_smtp_ok = 0;
 
-int SMTP_ok(int sock, char smtp_mode)
+int SMTP_ok(int sock, char smtp_mode, int mintimeout)
 /* returns status of SMTP connection */
 {
     SIGHANDLERTYPE alrmsave;
 
     /* set an alarm for smtp ok */
     alrmsave = set_signal_handler(SIGALRM, null_signal_handler);
-    set_timeout(mytimeout);
+    set_timeout(mytimeout >= mintimeout ? mytimeout : mintimeout);
 
     while ((SockRead(sock, smtp_response, sizeof(smtp_response)-1)) != -1)
     {
diff --git a/smtp.h b/smtp.h
index b65196a7..2f3f71fa 100644
--- a/smtp.h
+++ b/smtp.h
@@ -23,6 +23,16 @@
 #define ESMTP_ATRN	0x08		/* used with ODMR, RFC 2645 */
 #define ESMTP_AUTH	0x10
 
+/* SMTP timeouts (seconds) - SHOULD clauses from RFC-5321 sec. 4.5.3.2. */
+#define TIMEOUT_STARTSMTP	300
+#define TIMEOUT_HELO		300 /* not in RFC-5321, also for EHLO/LHLO */
+#define TIMEOUT_MAIL		300
+#define TIMEOUT_RCPT		300
+#define TIMEOUT_DATA		120
+#define TIMEOUT_DATALINE	180
+#define TIMEOUT_EOM		600
+#define TIMEOUT_DEFAULT		300 /* not in RFC-5321, all other client timeouts */
+
 extern time_t last_smtp_ok;
 
 int SMTP_helo(int socket, char smtp_mode, const char *host);
@@ -33,7 +43,7 @@ int SMTP_data(int socket, char smtp_mode);
 int SMTP_eom(int socket, char smtp_mode);
 int SMTP_rset(int socket, char smtp_mode);
 int SMTP_quit(int socket, char smtp_mode);
-int SMTP_ok(int socket, char smtp_mode);
+int SMTP_ok(int socket, char smtp_mode, int mintimeout);
 
 extern char smtp_response[MSGBUFSIZE];