From 563b63fa3e09d67239f51483e5dcec5c91251522 Mon Sep 17 00:00:00 2001 From: Philip Hazel Date: Mon, 16 Apr 2007 10:31:58 +0000 Subject: [PATCH] Fix bug in previous patch: following data is permitted after '.' so it must not be diagnosed as an error. The check for vanished socket can only be applied when there is no data pending. --- doc/doc-txt/ChangeLog | 18 ++++--- src/src/receive.c | 89 ++++++++++++++++------------------ test/log/0559 | 2 - test/log/2029 | 2 - test/log/2150 | 2 - test/scripts/0000-Basic/0300 | 3 +- test/scripts/0000-Basic/0301 | 3 +- test/scripts/0000-Basic/0559 | 22 --------- test/scripts/2000-GnuTLS/2029 | 27 ----------- test/scripts/2100-OpenSSL/2150 | 27 ----------- test/stdout/0300 | 3 +- test/stdout/0301 | 3 +- test/stdout/0559 | 24 --------- test/stdout/2029 | 32 ------------ test/stdout/2150 | 32 ------------ 15 files changed, 57 insertions(+), 232 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 4a8d41d66..db68bdae1 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -1,4 +1,4 @@ -$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.502 2007/04/13 15:13:47 ph10 Exp $ +$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.503 2007/04/16 10:31:58 ph10 Exp $ Change log file for Exim from version 4.21 ------------------------------------------- @@ -203,14 +203,16 @@ SC/03 Eximstats - V1.58 Fix to get <> and blackhole to show in edomain tables. PH/43 Yet another patch from the Sieve maintainer. PH/44 I found a way to check for a TCP/IP connection going away before sending - the response to the final '.' that terminates a message. At this time, - there should not be any pending input - the client should be waiting for - the response. Therefore, a select() can be used: if it shows that the - input is "ready", there is either input waiting, or the socket has been - closed. Both cases are errors. (It's a bit more complicated than this - because of buffering, but that's the essence of it.) Previously, Exim + the response to the final '.' that terminates a message, but only in the + case where the client has not sent further data following the '.' + (unfortunately, this is allowed). However, in many cases there won't be + any further data because there won't be any more messages to send. A call + to select() can be used: if it shows that the input is "ready", there is + either input waiting, or the socket has been closed. An attempt to read + the next input character can distinguish the two cases. Previously, Exim would have sent an OK response which the client would never have see. - This could lead to message repetition. This fix should cure that. + This could lead to message repetition. This fix should cure that, at + least in a lot of common cases. Exim version 4.66 diff --git a/src/src/receive.c b/src/src/receive.c index 98a728b2f..e4c82d2fa 100644 --- a/src/src/receive.c +++ b/src/src/receive.c @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/receive.c,v 1.36 2007/04/13 15:13:47 ph10 Exp $ */ +/* $Cambridge: exim/src/src/receive.c,v 1.37 2007/04/16 10:31:58 ph10 Exp $ */ /************************************************* * Exim - an Internet mail transport agent * @@ -3436,21 +3436,26 @@ to cause a call to receive_bomb_out() if the log cannot be opened. */ receive_call_bombout = TRUE; -/* Before sending an SMTP response in a TCP/IP session, we check to see if -there is unconsumed input (which there shouldn't be) or if the connection has -gone away. This can be done because the end of a message is always a -synchronization point. If the connection is still present, but there is no -pending input, the result of a select() call will be zero. If, however, the -connection has gone away, or if there is pending input, the result of select() -will be non-zero. The two cases can be distinguished by trying to read the next -input character. Of course, since TCP/IP is asynchronous, there is always a -chance that the connection will vanish between the time of this test and the -sending of the response, but the chance of this happening should be small. +/* Before sending an SMTP response in a TCP/IP session, we check to see if the +connection has gone away. This can only be done if there is no unconsumed input +waiting in the local input buffer. We can test for this by calling +receive_smtp_buffered(). RFC 2920 (pipelining) explicitly allows for additional +input to be sent following the final dot, so the presence of following input is +not an error. -We also check for input that has already been received and is in the local -input buffer (plain SMTP or TLS) by calling receive_smtp_buffered(). */ +If the connection is still present, but there is no unread input for the +socket, the result of a select() call will be zero. If, however, the connection +has gone away, or if there is pending input, the result of select() will be +non-zero. The two cases can be distinguished by trying to read the next input +character. If we succeed, we can unread it so that it remains in the local +buffer for handling later. If not, the connection has been lost. -if (smtp_input && sender_host_address != NULL && !sender_host_notsocket) +Of course, since TCP/IP is asynchronous, there is always a chance that the +connection will vanish between the time of this test and the sending of the +response, but the chance of this happening should be small. */ + +if (smtp_input && sender_host_address != NULL && !sender_host_notsocket && + !receive_smtp_buffered()) { struct timeval tv; fd_set select_check; @@ -3459,47 +3464,39 @@ if (smtp_input && sender_host_address != NULL && !sender_host_notsocket) tv.tv_sec = 0; tv.tv_usec = 0; - if (select(fileno(smtp_in) + 1, &select_check, NULL, NULL, &tv) != 0 || - receive_smtp_buffered()) + if (select(fileno(smtp_in) + 1, &select_check, NULL, NULL, &tv) != 0) { - uschar *msg; - if ((RECEIVE_GETC)() == EOF) - { - msg = US"SMTP connection lost after final dot"; - smtp_reply = US""; /* No attempt to send a response */ - } - else + int c = (RECEIVE_GETC)(); + if (c != EOF) (RECEIVE_UNGETC)(c); else { - msg = US"Synchronization error (data after final dot)"; - smtp_reply = US"550 Synchronization error (data after final dot)"; - } - - /* Overwrite the log line workspace */ + uschar *msg = US"SMTP connection lost after final dot"; + smtp_reply = US""; /* No attempt to send a response */ + smtp_yield = FALSE; /* Nothing more on this connection */ - sptr = 0; - s = string_cat(s, &size, &sptr, msg, Ustrlen(msg)); - s = add_host_info_for_log(s, &size, &sptr); - s[sptr] = 0; - log_write(0, LOG_MAIN, "%s", s); + /* Re-use the log line workspace */ - /* We now have to delete the files for this aborted message. */ + sptr = 0; + s = string_cat(s, &size, &sptr, msg, Ustrlen(msg)); + s = add_host_info_for_log(s, &size, &sptr); + s[sptr] = 0; + log_write(0, LOG_MAIN, "%s", s); - sprintf(CS spool_name, "%s/input/%s/%s-D", spool_directory, message_subdir, - message_id); - Uunlink(spool_name); + /* Delete the files for this aborted message. */ - sprintf(CS spool_name, "%s/input/%s/%s-H", spool_directory, message_subdir, - message_id); - Uunlink(spool_name); + sprintf(CS spool_name, "%s/input/%s/%s-D", spool_directory, + message_subdir, message_id); + Uunlink(spool_name); - sprintf(CS spool_name, "%s/msglog/%s/%s", spool_directory, message_subdir, - message_id); - Uunlink(spool_name); + sprintf(CS spool_name, "%s/input/%s/%s-H", spool_directory, + message_subdir, message_id); + Uunlink(spool_name); - /* Do not accept any more messages on this connection. */ + sprintf(CS spool_name, "%s/msglog/%s/%s", spool_directory, + message_subdir, message_id); + Uunlink(spool_name); - smtp_yield = FALSE; - goto TIDYUP; + goto TIDYUP; + } } } diff --git a/test/log/0559 b/test/log/0559 index 077691e95..fcae1cf93 100644 --- a/test/log/0559 +++ b/test/log/0559 @@ -1,4 +1,2 @@ 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225 1999-03-02 09:44:33 10HmaX-0005vi-00 SMTP connection lost after final dot H=(abcd) [127.0.0.1] P=esmtp -1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225 -1999-03-02 09:44:33 10HmaY-0005vi-00 Synchronization error (data after final dot) H=(abcd) [127.0.0.1] P=esmtp diff --git a/test/log/2029 b/test/log/2029 index 737b6977b..0e16a7b24 100644 --- a/test/log/2029 +++ b/test/log/2029 @@ -1,5 +1,3 @@ 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225 1999-03-02 09:44:33 10HmaX-0005vi-00 TLS recv error on connection from [127.0.0.1]: A TLS packet with unexpected length was received. 1999-03-02 09:44:33 10HmaX-0005vi-00 SMTP connection lost after final dot H=[127.0.0.1] P=smtps -1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225 -1999-03-02 09:44:33 10HmaY-0005vi-00 Synchronization error (data after final dot) H=[127.0.0.1] P=smtps diff --git a/test/log/2150 b/test/log/2150 index 41ada4435..01c430781 100644 --- a/test/log/2150 +++ b/test/log/2150 @@ -1,4 +1,2 @@ 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225 1999-03-02 09:44:33 10HmaX-0005vi-00 SMTP connection lost after final dot H=[127.0.0.1] P=smtps -1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225 -1999-03-02 09:44:33 10HmaY-0005vi-00 Synchronization error (data after final dot) H=[127.0.0.1] P=smtps diff --git a/test/scripts/0000-Basic/0300 b/test/scripts/0000-Basic/0300 index de3be1095..053aec804 100644 --- a/test/scripts/0000-Basic/0300 +++ b/test/scripts/0000-Basic/0300 @@ -23,9 +23,8 @@ rset\r\nmail from:\r\nrcpt to:\r\ndata ??? 250 ??? 354 the message -. +.\r\nmail from: +++ 1 -mail from: rcpt to:\r\ndata\r\nthe message\r\nsecond line ??? 250 ??? 250 diff --git a/test/scripts/0000-Basic/0301 b/test/scripts/0000-Basic/0301 index f84244f05..4add8f42c 100644 --- a/test/scripts/0000-Basic/0301 +++ b/test/scripts/0000-Basic/0301 @@ -26,9 +26,8 @@ mail from:\r\nrcpt to:\r\ndata ??? 250 ??? 354 the message -. +.\r\nmail from:\r\nrcpt to:\r\ndata\r\nthe message ??? 250 -mail from:\r\nrcpt to:\r\ndata\r\nthe message ??? 250 ??? 250 ??? 354 diff --git a/test/scripts/0000-Basic/0559 b/test/scripts/0000-Basic/0559 index 01d7d99af..3729f25ab 100644 --- a/test/scripts/0000-Basic/0559 +++ b/test/scripts/0000-Basic/0559 @@ -20,25 +20,3 @@ This is a test message. **** sleep 1 killdaemon -# -# Also check for next input sent too soon -# -exim -DSERVER=server -bd -oX PORT_D -**** -client -t5 127.0.0.1 PORT_D -??? 220 -ehlo abcd -??? 250- -??? 250- -??? 250- -??? 250 -mail from:\r\nrcpt to:\r\ndata -??? 250 -??? 250 -??? 354 -This is a test message. -.\r\nrset -??? 550 -**** -sleep 1 -killdaemon diff --git a/test/scripts/2000-GnuTLS/2029 b/test/scripts/2000-GnuTLS/2029 index e371e68d7..c2b35497b 100644 --- a/test/scripts/2000-GnuTLS/2029 +++ b/test/scripts/2000-GnuTLS/2029 @@ -28,30 +28,3 @@ This is a test message. **** sleep 1 killdaemon -# -# Also check for next input sent too soon -# -exim -DSERVER=server -bd -oX PORT_D -**** -client-gnutls 127.0.0.1 PORT_D -??? 220 -ehlo abcd -??? 250- -??? 250- -??? 250- -??? 250- -??? 250 -starttls -??? 220 -mail from: -??? 250 -rcpt to: -??? 250 -data -??? 354 -This is a test message. -.\r\nrset -+++ 1 -**** -sleep 1 -killdaemon diff --git a/test/scripts/2100-OpenSSL/2150 b/test/scripts/2100-OpenSSL/2150 index cfc6a20d7..91cd38276 100644 --- a/test/scripts/2100-OpenSSL/2150 +++ b/test/scripts/2100-OpenSSL/2150 @@ -26,30 +26,3 @@ This is a test message. **** sleep 1 killdaemon -# -# Also check for next input sent too soon -# -exim -DSERVER=server -bd -oX PORT_D -**** -client-gnutls 127.0.0.1 PORT_D -??? 220 -ehlo abcd -??? 250- -??? 250- -??? 250- -??? 250- -??? 250 -starttls -??? 220 -mail from: -??? 250 -rcpt to: -??? 250 -data -??? 354 -This is a test message. -.\r\nrset -+++ 1 -**** -sleep 1 -killdaemon diff --git a/test/stdout/0300 b/test/stdout/0300 index 8dd5a3cc1..a41df39d3 100644 --- a/test/stdout/0300 +++ b/test/stdout/0300 @@ -27,9 +27,8 @@ Connecting to 127.0.0.1 port 1225 ... connected ??? 354 <<< 354 Enter message, ending with "." on a line by itself >>> the message ->>> . +>>> .\r\nmail from: +++ 1 ->>> mail from: >>> rcpt to:\r\ndata\r\nthe message\r\nsecond line ??? 250 <<< 250 OK id=10HmaX-0005vi-00 diff --git a/test/stdout/0301 b/test/stdout/0301 index 964f0c17b..198962d06 100644 --- a/test/stdout/0301 +++ b/test/stdout/0301 @@ -36,10 +36,9 @@ Connecting to 127.0.0.1 port 1225 ... connected ??? 354 <<< 354 Enter message, ending with "." on a line by itself >>> the message ->>> . +>>> .\r\nmail from:\r\nrcpt to:\r\ndata\r\nthe message ??? 250 <<< 250 OK id=10HmaX-0005vi-00 ->>> mail from:\r\nrcpt to:\r\ndata\r\nthe message ??? 250 <<< 250 OK ??? 250 diff --git a/test/stdout/0559 b/test/stdout/0559 index eee1b2e43..71f5c4541 100644 --- a/test/stdout/0559 +++ b/test/stdout/0559 @@ -21,27 +21,3 @@ Connecting to 127.0.0.1 port 1225 ... connected >>> . +++ 1 End of script -Connecting to 127.0.0.1 port 1225 ... connected -??? 220 -<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 ->>> ehlo abcd -??? 250- -<<< 250-myhost.test.ex Hello abcd [127.0.0.1] -??? 250- -<<< 250-SIZE 52428800 -??? 250- -<<< 250-PIPELINING -??? 250 -<<< 250 HELP ->>> mail from:\r\nrcpt to:\r\ndata -??? 250 -<<< 250 OK -??? 250 -<<< 250 Accepted -??? 354 -<<< 354 Enter message, ending with "." on a line by itself ->>> This is a test message. ->>> .\r\nrset -??? 550 -<<< 550 Synchronization error (data after final dot) -End of script diff --git a/test/stdout/2029 b/test/stdout/2029 index dd1bdaef8..1efa5cd7c 100644 --- a/test/stdout/2029 +++ b/test/stdout/2029 @@ -30,35 +30,3 @@ Succeeded in starting TLS >>> . +++ 1 End of script -Connecting to 127.0.0.1 port 1225 ... connected -??? 220 -<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 ->>> ehlo abcd -??? 250- -<<< 250-myhost.test.ex Hello abcd [127.0.0.1] -??? 250- -<<< 250-SIZE 52428800 -??? 250- -<<< 250-PIPELINING -??? 250- -<<< 250-STARTTLS -??? 250 -<<< 250 HELP ->>> starttls -??? 220 -<<< 220 TLS go ahead -Attempting to start TLS -Succeeded in starting TLS ->>> mail from: -??? 250 -<<< 250 OK ->>> rcpt to: -??? 250 -<<< 250 Accepted ->>> data -??? 354 -<<< 354 Enter message, ending with "." on a line by itself ->>> This is a test message. ->>> .\r\nrset -+++ 1 -End of script diff --git a/test/stdout/2150 b/test/stdout/2150 index dd1bdaef8..1efa5cd7c 100644 --- a/test/stdout/2150 +++ b/test/stdout/2150 @@ -30,35 +30,3 @@ Succeeded in starting TLS >>> . +++ 1 End of script -Connecting to 127.0.0.1 port 1225 ... connected -??? 220 -<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 ->>> ehlo abcd -??? 250- -<<< 250-myhost.test.ex Hello abcd [127.0.0.1] -??? 250- -<<< 250-SIZE 52428800 -??? 250- -<<< 250-PIPELINING -??? 250- -<<< 250-STARTTLS -??? 250 -<<< 250 HELP ->>> starttls -??? 220 -<<< 220 TLS go ahead -Attempting to start TLS -Succeeded in starting TLS ->>> mail from: -??? 250 -<<< 250 OK ->>> rcpt to: -??? 250 -<<< 250 Accepted ->>> data -??? 354 -<<< 354 Enter message, ending with "." on a line by itself ->>> This is a test message. ->>> .\r\nrset -+++ 1 -End of script -- 2.25.1