diff options
author | Philip Hazel <ph10@hermes.cam.ac.uk> | 2007-02-20 15:58:02 +0000 |
---|---|---|
committer | Philip Hazel <ph10@hermes.cam.ac.uk> | 2007-02-20 15:58:02 +0000 |
commit | a14e56367e5ef12d43aee57e3f8565be8d468845 (patch) | |
tree | 710fe75066b5479f036ac005cb77f14fa46c17e1 | |
parent | ca86f471bf30f4630e96e24f6c13de269f380f41 (diff) |
Add extra sync checks after ACLs that might delay.
-rw-r--r-- | doc/doc-txt/ChangeLog | 10 | ||||
-rw-r--r-- | src/src/smtp_in.c | 122 | ||||
-rw-r--r-- | test/README | 13 | ||||
-rw-r--r-- | test/confs/0556 | 39 | ||||
-rw-r--r-- | test/log/0556 | 5 | ||||
-rw-r--r-- | test/rejectlog/0556 | 3 | ||||
-rw-r--r-- | test/scripts/0000-Basic/0556 | 55 | ||||
-rw-r--r-- | test/stdout/0556 | 59 |
8 files changed, 268 insertions, 38 deletions
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 96832f604..6e61fa9b6 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -1,4 +1,4 @@ -$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.483 2007/02/20 11:37:16 ph10 Exp $ +$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.484 2007/02/20 15:58:02 ph10 Exp $ Change log file for Exim from version 4.21 ------------------------------------------- @@ -123,6 +123,14 @@ PH/28 The $smtp_command and $smtp_command_argument variables were not correct address, for example: MAIL FROM:<foo@bar> SIZE=1234. The option settings were accidentally chopped off. +PH/29 SMTP synchronization checks are implemented when a command is read - + there is a check that no more input is waiting when there shouldn't be + any. However, for some commands, a delay in an ACL can mean that it is + some time before the response is written. In this time, more input might + arrive, invalidly. So now there are extra checks after an ACL has run for + HELO/EHLO and after the predata ACL, and likewise for MAIL and RCPT when + pipelining has not been advertised. + Exim version 4.66 ----------------- diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index c9c5842b1..d30d28280 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/smtp_in.c,v 1.54 2007/02/20 11:37:16 ph10 Exp $ */ +/* $Cambridge: exim/src/src/smtp_in.c,v 1.55 2007/02/20 15:58:02 ph10 Exp $ */ /************************************************* * Exim - an Internet mail transport agent * @@ -469,6 +469,7 @@ exim_exit(EXIT_FAILURE); + /************************************************* * Read one command line * *************************************************/ @@ -604,6 +605,60 @@ return OTHER_CMD; /************************************************* +* Recheck synchronization * +*************************************************/ + +/* Synchronization checks can never be perfect because a packet may be on its +way but not arrived when the check is done. Such checks can in any case only be +done when TLS is not in use. Normally, the checks happen when commands are +read: Exim ensures that there is no more input in the input buffer. In normal +cases, the response to the command will be fast, and there is no further check. + +However, for some commands an ACL is run, and that can include delays. In those +cases, it is useful to do another check on the input just before sending the +response. This also applies at the start of a connection. This function does +that check by means of the select() function, as long as the facility is not +disabled or inappropriate. A failure of select() is ignored. + +When there is unwanted input, we read it so that it appears in the log of the +error. + +Arguments: none +Returns: TRUE if all is well; FALSE if there is input pending +*/ + +static BOOL +check_sync(void) +{ +int fd, rc; +fd_set fds; +struct timeval tzero; + +if (!smtp_enforce_sync || sender_host_address == NULL || + sender_host_notsocket || tls_active >= 0) + return TRUE; + +fd = fileno(smtp_in); +FD_ZERO(&fds); +FD_SET(fd, &fds); +tzero.tv_sec = 0; +tzero.tv_usec = 0; +rc = select(fd + 1, (SELECT_ARG2_TYPE *)&fds, NULL, NULL, &tzero); + +if (rc <= 0) return TRUE; /* Not ready to read */ +rc = smtp_getc(); +if (rc < 0) return TRUE; /* End of file or error */ + +smtp_ungetc(rc); +rc = smtp_inend - smtp_inptr; +if (rc > 150) rc = 150; +smtp_inptr[rc] = 0; +return FALSE; +} + + + +/************************************************* * Forced closedown of call * *************************************************/ @@ -1760,30 +1815,14 @@ ss[ptr] = 0; /* string_cat leaves room for this */ /* Before we write the banner, check that there is no input pending, unless this synchronisation check is disabled. */ -if (smtp_enforce_sync && sender_host_address != NULL && !sender_host_notsocket) +if (!check_sync()) { - fd_set fds; - struct timeval tzero; - tzero.tv_sec = 0; - tzero.tv_usec = 0; - FD_ZERO(&fds); - FD_SET(fileno(smtp_in), &fds); - if (select(fileno(smtp_in) + 1, (SELECT_ARG2_TYPE *)&fds, NULL, NULL, - &tzero) > 0) - { - int rc = read(fileno(smtp_in), smtp_inbuffer, in_buffer_size); - if (rc > 0) - { - if (rc > 150) rc = 150; - smtp_inbuffer[rc] = 0; - log_write(0, LOG_MAIN|LOG_REJECT, "SMTP protocol " - "synchronization error (input sent without waiting for greeting): " - "rejected connection from %s input=\"%s\"", host_and_ident(TRUE), - string_printing(smtp_inbuffer)); - smtp_printf("554 SMTP synchronization error\r\n"); - return FALSE; - } - } + log_write(0, LOG_MAIN|LOG_REJECT, "SMTP protocol " + "synchronization error (input sent without waiting for greeting): " + "rejected connection from %s input=\"%s\"", host_and_ident(TRUE), + string_printing(smtp_inptr)); + smtp_printf("554 SMTP synchronization error\r\n"); + return FALSE; } /* Now output the banner */ @@ -2731,7 +2770,8 @@ while (done <= 0) spf_init(sender_helo_name, sender_host_address); #endif - /* Apply an ACL check if one is defined */ + /* Apply an ACL check if one is defined; afterwards, recheck + synchronization in case the client started sending in a delay. */ if (acl_smtp_helo != NULL) { @@ -2743,6 +2783,7 @@ while (done <= 0) host_build_sender_fullhost(); /* Rebuild */ break; } + else if (!check_sync()) goto SYNC_FAILURE; } /* Generate an OK reply. The default string includes the ident if present, @@ -3236,10 +3277,16 @@ while (done <= 0) } } - /* Apply an ACL check if one is defined, before responding */ + /* Apply an ACL check if one is defined, before responding. Afterwards, + when pipelining is not advertised, do another sync check in case the ACL + delayed and the client started sending in the meantime. */ - rc = (acl_smtp_mail == NULL)? OK : - acl_check(ACL_WHERE_MAIL, NULL, acl_smtp_mail, &user_msg, &log_msg); + if (acl_smtp_mail == NULL) rc = OK; else + { + rc = acl_check(ACL_WHERE_MAIL, NULL, acl_smtp_mail, &user_msg, &log_msg); + if (rc == OK && !pipelining_advertised && !check_sync()) + goto SYNC_FAILURE; + } if (rc == OK || rc == DISCARD) { @@ -3395,10 +3442,17 @@ while (done <= 0) } /* If the MAIL ACL discarded all the recipients, we bypass ACL checking - for them. Otherwise, check the access control list for this recipient. */ + for them. Otherwise, check the access control list for this recipient. As + there may be a delay in this, re-check for a synchronization error + afterwards, unless pipelining was advertised. */ - rc = recipients_discarded? DISCARD : - acl_check(ACL_WHERE_RCPT, recipient, acl_smtp_rcpt, &user_msg, &log_msg); + if (recipients_discarded) rc = DISCARD; else + { + rc = acl_check(ACL_WHERE_RCPT, recipient, acl_smtp_rcpt, &user_msg, + &log_msg); + if (rc == OK && !pipelining_advertised && !check_sync()) + goto SYNC_FAILURE; + } /* The ACL was happy */ @@ -3472,12 +3526,16 @@ while (done <= 0) break; } + /* If there is an ACL, re-check the synchronization afterwards, since the + ACL may have delayed. */ + if (acl_smtp_predata == NULL) rc = OK; else { enable_dollar_recipients = TRUE; rc = acl_check(ACL_WHERE_PREDATA, NULL, acl_smtp_predata, &user_msg, &log_msg); enable_dollar_recipients = FALSE; + if (rc == OK && !check_sync()) goto SYNC_FAILURE; } if (rc == OK) @@ -3493,7 +3551,6 @@ while (done <= 0) else done = smtp_handle_acl_fail(ACL_WHERE_PREDATA, rc, user_msg, log_msg); - break; @@ -3942,6 +3999,7 @@ while (done <= 0) case BADSYN_CMD: + SYNC_FAILURE: if (smtp_inend >= smtp_inbuffer + in_buffer_size) smtp_inend = smtp_inbuffer + in_buffer_size - 1; c = smtp_inend - smtp_inptr; diff --git a/test/README b/test/README index 5c060974c..c1398329d 100644 --- a/test/README +++ b/test/README @@ -1,4 +1,4 @@ -$Cambridge: exim/test/README,v 1.7 2007/01/31 16:52:12 ph10 Exp $ +$Cambridge: exim/test/README,v 1.8 2007/02/20 15:58:02 ph10 Exp $ EXPORTABLE EXIM TEST SUITE -------------------------- @@ -6,7 +6,7 @@ EXPORTABLE EXIM TEST SUITE This document last updated for: Test Suite Version: 4.67 -Date: 31 January 2007 +Date: 20 February 2007 BACKGROUND @@ -765,9 +765,12 @@ as well as to the named file. This command runs the auxiliary "client" program that simulates an SMTP client. It is controlled by a script read from its standard input, details of which are -given below. The only option is -t, which must be followed by a number, to -specify the command timeout in seconds. The program connects to the given IP -address and port, using the specified interface, if one is given. +given below. There are two options. One is -t, which must be followed directly +by a number, to specify the command timeout in seconds (e.g. -t5). The default +timeout is 1 second. The other option is -tls-on-connect, which causes the +client to try to start up a TLS session as soon as it has connected, without +using the STARTTLS command. The client program connects to the given IP address +and port, using the specified interface, if one is given. client-ssl [<options>] <ip address> <port> [<outgoing interface>] \ diff --git a/test/confs/0556 b/test/confs/0556 new file mode 100644 index 000000000..7ed0105ed --- /dev/null +++ b/test/confs/0556 @@ -0,0 +1,39 @@ +# Exim test configuration 0556 + +ACL_MAIL=accept +ACL_RCPT=accept +ACL_PREDATA=accept +PAH=127.0.0.1 + +exim_path = EXIM_PATH +host_lookup_order = bydns +primary_hostname = myhost.test.ex +rfc1413_query_timeout = 0s +spool_directory = DIR/spool +log_file_path = DIR/spool/log/%slog +gecos_pattern = "" +gecos_name = CALLER_NAME + +# ----- Main settings ----- + +acl_smtp_predata = ACL_PREDATA +acl_smtp_mail = ACL_MAIL +acl_smtp_rcpt = ACL_RCPT +pipelining_advertise_hosts = PAH + +queue_only + +# ----- ACL ----- + +begin ACL + +check_predata: + accept delay = 2s + +check_mail: + accept delay = 2s + +check_rcpt: + accept delay = 2s + +# End diff --git a/test/log/0556 b/test/log/0556 new file mode 100644 index 000000000..5f1631c5a --- /dev/null +++ b/test/log/0556 @@ -0,0 +1,5 @@ +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 SMTP protocol synchronization error (next input sent too soon: pipelining was advertised): rejected "data" H=(abcd) [127.0.0.1] next input="Start: sent early ...\r\n" +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 SMTP protocol synchronization error (next input sent too soon: pipelining was not advertised): rejected "mail from:<userx@test.ex>" H=(abcd) [127.0.0.1] next input="rcpt to:<userx@test.ex>\r\n" +1999-03-02 09:44:33 SMTP protocol synchronization error (next input sent too soon: pipelining was not advertised): rejected "rcpt to:<userx@test.ex>" H=(abcd) [127.0.0.1] next input="data\r\n" diff --git a/test/rejectlog/0556 b/test/rejectlog/0556 new file mode 100644 index 000000000..2f0d87f1b --- /dev/null +++ b/test/rejectlog/0556 @@ -0,0 +1,3 @@ +1999-03-02 09:44:33 SMTP protocol synchronization error (next input sent too soon: pipelining was advertised): rejected "data" H=(abcd) [127.0.0.1] next input="Start: sent early ...\r\n" +1999-03-02 09:44:33 SMTP protocol synchronization error (next input sent too soon: pipelining was not advertised): rejected "mail from:<userx@test.ex>" H=(abcd) [127.0.0.1] next input="rcpt to:<userx@test.ex>\r\n" +1999-03-02 09:44:33 SMTP protocol synchronization error (next input sent too soon: pipelining was not advertised): rejected "rcpt to:<userx@test.ex>" H=(abcd) [127.0.0.1] next input="data\r\n" diff --git a/test/scripts/0000-Basic/0556 b/test/scripts/0000-Basic/0556 new file mode 100644 index 000000000..54c3a86a5 --- /dev/null +++ b/test/scripts/0000-Basic/0556 @@ -0,0 +1,55 @@ +# SMTP synchronization checks before sending responses +need_ipv4 +# +exim -DSERVER=server -DACL_PREDATA=check_predata -bd -oX PORT_D +**** +# The pause (+++ 1) in the middle of this is so that there is no pending +# input when DATA is received, but we start sending the data itself too +# early (the server will be waiting 2 seconds in the predata ACL). +# +client 127.0.0.1 PORT_D +??? 220 +ehlo abcd +??? 250- +??? 250- +??? 250- +??? 250 +rset\r\nmail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata ++++ 1 +Start: sent early ... +??? 250 +??? 250 +??? 250 +??? 554 +**** +sleep 1 +killdaemon +# This time turn off pipelining to check MAIL and RCPT +exim -DSERVER=server -DACL_MAIL=check_mail -DACL_RCPT=check_rcpt -DPAH= \ + -bd -oX PORT_D +**** +client -t5 127.0.0.1 PORT_D +??? 220 +ehlo abcd +??? 250- +??? 250- +??? 250 +mail from:<userx@test.ex> ++++ 1 +rcpt to:<userx@test.ex> +??? 554 +**** +client -t5 127.0.0.1 PORT_D +??? 220 +ehlo abcd +??? 250- +??? 250- +??? 250 +mail from:<userx@test.ex> +??? 250 +rcpt to:<userx@test.ex> ++++ 1 +data +??? 554 +**** +killdaemon diff --git a/test/stdout/0556 b/test/stdout/0556 new file mode 100644 index 000000000..d8d60ab9a --- /dev/null +++ b/test/stdout/0556 @@ -0,0 +1,59 @@ +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 +>>> rset\r\nmail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata ++++ 1 +>>> Start: sent early ... +??? 250 +<<< 250 Reset OK +??? 250 +<<< 250 OK +??? 250 +<<< 250 Accepted +??? 554 +<<< 554 SMTP synchronization error +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 HELP +>>> mail from:<userx@test.ex> ++++ 1 +>>> rcpt to:<userx@test.ex> +??? 554 +<<< 554 SMTP synchronization error +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 HELP +>>> mail from:<userx@test.ex> +??? 250 +<<< 250 OK +>>> rcpt to:<userx@test.ex> ++++ 1 +>>> data +??? 554 +<<< 554 SMTP synchronization error +End of script |