From 431b736177e2cdfd0b4da4c8545d8b732286abe1 Mon Sep 17 00:00:00 2001 From: Philip Hazel Date: Wed, 17 Jan 2007 11:17:58 +0000 Subject: [PATCH] Fix negated dnslists item bug; add == and =& features, courtesy Brad Jorsch. --- doc/doc-txt/ChangeLog | 15 ++++- doc/doc-txt/NewStuff | 70 +++++++++++++++++++++- src/ACKNOWLEDGMENTS | 5 +- src/src/verify.c | 113 +++++++++++++++++++++++++---------- test/confs/0139 | 17 ++++++ test/scripts/0000-Basic/0139 | 8 +++ test/stderr/0139 | 101 +++++++++++++++++++++++++++++++ test/stdout/0139 | 16 +++++ 8 files changed, 308 insertions(+), 37 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 83bba99ee..240c815eb 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -1,4 +1,4 @@ -$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.453 2007/01/16 21:00:29 magnus Exp $ +$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.454 2007/01/17 11:17:58 ph10 Exp $ Change log file for Exim from version 4.21 ------------------------------------------- @@ -13,6 +13,19 @@ MH/01 Fix for bug #448, segfault in Dovecot authenticator when interface_address PH/01 Added a new log selector smtp_no_mail, to log SMTP sessions that do not issue a MAIL command. +PH/02 In an ACL statement such as + + deny dnslists = X!=127.0.0.2 : X=127.0.0.2 + + if a client was not listed at all, or was listed with a value other than + 127.0.0.2, in the X list, but was listed with 127.0.0.2 in the Y list, + the condition was not true (as it should be), so access was not denied. + The bug was that the ! inversion was incorrectly passed on to the second + item. This has been fixed. + +PH/03 Added additional dnslists conditions == and =& which are different from + = and & when the dns lookup returns more than one IP address. + Exim version 4.66 ----------------- diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff index a24a21226..960f93ce8 100644 --- a/doc/doc-txt/NewStuff +++ b/doc/doc-txt/NewStuff @@ -1,4 +1,4 @@ -$Cambridge: exim/doc/doc-txt/NewStuff,v 1.126 2007/01/15 15:59:22 ph10 Exp $ +$Cambridge: exim/doc/doc-txt/NewStuff,v 1.127 2007/01/17 11:17:58 ph10 Exp $ New Features in Exim -------------------- @@ -38,6 +38,74 @@ Version 4.67 setting of 10 for smtp_accep_max_nonmail, the connection will in any case be aborted before 20 non-mail commands are processed. + 2. When an item in a dnslists list is followed by = and & and a list of IP + addresses, in order to restrict the match to specific results from the DNS + lookup, the behaviour was not clear when the lookup returned more than one + IP address. For example, consider the condition + + dnslists = a.b.c=127.0.0.1 + + What happens if the DNS lookup for the incoming IP address yields both + 127.0.0.1 and 127.0.0.2 by means of two separate DNS records? Is the + condition true because at least one given value was found, or is it false + because at least one of the found values was not listed? And how does this + affect negated conditions? + + The behaviour of = and & has not been changed; however, the text below + documents it more clearly. In addition, two new additional conditions (== + and =&) have been added, to permit the "other" behaviour to be configured. + + A DNS lookup may yield more than one record. Thus, the result of the lookup + for a dnslists check may yield more than one IP address. The question then + arises as to whether all the looked up addresses must be listed, or whether + just one is good enough. Both possibilities are provided for: + + . If = or & is used, the condition is true if any one of the looked up + IP addresses matches one of the listed addresses. Consider: + + dnslists = a.b.c=127.0.0.1 + + If the DNS lookup yields both 127.0.0.1 and 127.0.0.2, the condition is + true because 127.0.0.1 matches. + + . If == or =& is used, the condition is true only if every one of the + looked up IP addresses matches one of the listed addresses. Consider: + + dnslists = a.b.c==127.0.0.1 + + If the DNS lookup yields both 127.0.0.1 and 127.0.0.2, the condition is + false because 127.0.0.2 is not listed. You would need to have + + dnslists = a.b.c==127.0.0.1,127.0.0.2 + + for the condition to be true. + + When ! is used to negate IP address matching, it inverts the result, giving + the precise opposite of the behaviour above. Thus: + + . If != or !& is used, the condition is true if none of the looked up IP + addresses matches one of the listed addresses. Consider: + + dnslists = a.b.c!&0.0.0.1 + + If the DNS lookup yields both 127.0.0.1 and 127.0.0.2, the condition is + false because 127.0.0.1 matches. + + . If !== or !=& is used, the condition is true there is at least one looked + up IP address that does not match. Consider: + + dnslists = a.b.c!=&0.0.0.1 + + If the DNS lookup yields both 127.0.0.1 and 127.0.0.2, the condition is + true, because 127.0.0.2 does not match. You would need to have + + dnslists = a.b.c!=&0.0.0.1,0.0.0.2 + + for the condition to be false. + + When the DNS lookup yields only a single IP address, there is no difference + between = and == and between & and =&. + Version 4.66 ------------ diff --git a/src/ACKNOWLEDGMENTS b/src/ACKNOWLEDGMENTS index 08440283a..a4b601652 100644 --- a/src/ACKNOWLEDGMENTS +++ b/src/ACKNOWLEDGMENTS @@ -1,4 +1,4 @@ -$Cambridge: exim/src/ACKNOWLEDGMENTS,v 1.67 2006/12/19 14:51:34 ph10 Exp $ +$Cambridge: exim/src/ACKNOWLEDGMENTS,v 1.68 2007/01/17 11:17:58 ph10 Exp $ EXIM ACKNOWLEDGEMENTS @@ -20,7 +20,7 @@ relatively small patches. Philip Hazel Lists created: 20 November 2002 -Last updated: 19 December 2006 +Last updated: 17 January 2007 THE OLD LIST @@ -178,6 +178,7 @@ Bob Johannessen Patch for Sieve envelope tests bug Patch for negative uid/gid bug Brad Jorsch Patch for bitwise logical operators Patch for using "message" on acceptance + Patch to add == and =& to dnslists Christian Kellner Patch for LDAP dereferencing Alex Kiernan Patches for libradius Diagnosis of milliwait clock-backwards bug diff --git a/src/src/verify.c b/src/src/verify.c index 141446aa7..3b0983919 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/verify.c,v 1.45 2007/01/08 10:50:18 ph10 Exp $ */ +/* $Cambridge: exim/src/src/verify.c,v 1.46 2007/01/17 11:17:58 ph10 Exp $ */ /************************************************* * Exim - an Internet mail transport agent * @@ -29,6 +29,12 @@ typedef struct dnsbl_cache_block { static tree_node *dnsbl_cache = NULL; +/* Bits for match_type in one_check_dnsbl() */ + +#define MT_NOT 1 +#define MT_ALL 2 + + /************************************************* * Retrieve a callout cache record * @@ -2540,7 +2546,12 @@ Arguments: reversed if IP address) iplist the list of matching IP addresses, or NULL for "any" bitmask true if bitmask matching is wanted - invert_result true if result to be inverted + match_type condition for 'succeed' result + 0 => Any RR in iplist (=) + 1 => No RR in iplist (!=) + 2 => All RRs in iplist (==) + 3 => Some RRs not in iplist (!==) + the two bits are defined as MT_NOT and MT_ALL defer_return what to return for a defer Returns: OK if lookup succeeded @@ -2549,7 +2560,7 @@ Returns: OK if lookup succeeded static int one_check_dnsbl(uschar *domain, uschar *domain_txt, uschar *keydomain, - uschar *prepend, uschar *iplist, BOOL bitmask, BOOL invert_result, + uschar *prepend, uschar *iplist, BOOL bitmask, int match_type, int defer_return) { dns_answer dnsa; @@ -2668,21 +2679,25 @@ if (cb->rc == DNS_SUCCEED) if (iplist != NULL) { - int ipsep = ','; - uschar ip[46]; - uschar *ptr = iplist; - - while (string_nextinlist(&ptr, &ipsep, ip, sizeof(ip)) != NULL) + for (da = cb->rhs; da != NULL; da = da->next) { + int ipsep = ','; + uschar ip[46]; + uschar *ptr = iplist; + uschar *res; + /* Handle exact matching */ + if (!bitmask) { - for (da = cb->rhs; da != NULL; da = da->next) + while ((res = string_nextinlist(&ptr, &ipsep, ip, sizeof(ip))) != NULL) { if (Ustrcmp(CS da->address, ip) == 0) break; } } + /* Handle bitmask matching */ + else { int address[4]; @@ -2695,37 +2710,60 @@ if (cb->rc == DNS_SUCCEED) ignore IPv6 addresses. The default mask is 0, which always matches. We change this only for IPv4 addresses in the list. */ - if (host_aton(ip, address) == 1) mask = address[0]; + if (host_aton(da->address, address) == 1) mask = address[0]; /* Scan the returned addresses, skipping any that are IPv6 */ - for (da = cb->rhs; da != NULL; da = da->next) + while ((res = string_nextinlist(&ptr, &ipsep, ip, sizeof(ip))) != NULL) { - if (host_aton(da->address, address) != 1) continue; - if ((address[0] & mask) == mask) break; + if (host_aton(ip, address) != 1) continue; + if ((address[0] & mask) == address[0]) break; } } - /* Break out if a match has been found */ + /* If either + + (a) An IP address in an any ('=') list matched, or + (b) No IP address in an all ('==') list matched - if (da != NULL) break; + then we're done searching. */ + + if (((match_type & MT_ALL) != 0) == (res == NULL)) break; } - /* If either + /* If da == NULL, either - (a) No IP address in a positive list matched, or - (b) An IP address in a negative list did match + (a) No IP address in an any ('=') list matched, or + (b) An IP address in an all ('==') list didn't match - then behave as if the DNSBL lookup had not succeeded, i.e. the host is - not on the list. */ + so behave as if the DNSBL lookup had not succeeded, i.e. the host is not on + the list. */ - if (invert_result != (da == NULL)) + if ((match_type == MT_NOT || match_type == MT_ALL) != (da == NULL)) { HDEBUG(D_dnsbl) { + uschar *res = NULL; + switch(match_type) + { + case 0: + res = US"was no match"; + break; + case MT_NOT: + res = US"was an exclude match"; + break; + case MT_ALL: + res = US"was an IP address that did not match"; + break; + case MT_NOT|MT_ALL: + res = US"were no IP addresses that did not match"; + break; + } debug_printf("=> but we are not accepting this block class because\n"); - debug_printf("=> there was %s match for %c%s\n", - invert_result? "an exclude":"no", bitmask? '&' : '=', iplist); + debug_printf("=> there %s for %s%c%s\n", + res, + ((match_type & MT_ALL) == 0)? "" : "=", + bitmask? '&' : '=', iplist); } return FAIL; } @@ -2739,7 +2777,7 @@ if (cb->rc == DNS_SUCCEED) if (domain_txt != domain) return one_check_dnsbl(domain_txt, domain_txt, keydomain, prepend, NULL, - FALSE, invert_result, defer_return); + FALSE, match_type, defer_return); /* If there is no alternate domain, look up a TXT record in the main domain if it has not previously been cached. */ @@ -2852,7 +2890,6 @@ verify_check_dnsbl(uschar **listptr) { int sep = 0; int defer_return = FAIL; -BOOL invert_result = FALSE; uschar *list = *listptr; uschar *domain; uschar *s; @@ -2873,6 +2910,7 @@ while ((domain = string_nextinlist(&list, &sep, buffer, sizeof(buffer))) != NULL { int rc; BOOL bitmask = FALSE; + int match_type = 0; uschar *domain_txt; uschar *comma; uschar *iplist; @@ -2899,8 +2937,8 @@ while ((domain = string_nextinlist(&list, &sep, buffer, sizeof(buffer))) != NULL if (key != NULL) *key++ = 0; /* See if there's a list of addresses supplied after the domain name. This is - introduced by an = or a & character; if preceded by ! we invert the result. - */ + introduced by an = or a & character; if preceded by = we require all matches + and if preceded by ! we invert the result. */ iplist = Ustrchr(domain, '='); if (iplist == NULL) @@ -2909,14 +2947,23 @@ while ((domain = string_nextinlist(&list, &sep, buffer, sizeof(buffer))) != NULL iplist = Ustrchr(domain, '&'); } - if (iplist != NULL) + if (iplist != NULL) /* Found either = or & */ { - if (iplist > domain && iplist[-1] == '!') + if (iplist > domain && iplist[-1] == '!') /* Handle preceding ! */ { - invert_result = TRUE; + match_type |= MT_NOT; iplist[-1] = 0; } - *iplist++ = 0; + + *iplist++ = 0; /* Terminate domain, move on */ + + /* If we found = (bitmask == FALSE), check for == or =& */ + + if (!bitmask && (*iplist == '=' || *iplist == '&')) + { + bitmask = *iplist++ == '&'; + match_type |= MT_ALL; + } } /* If there is a comma in the domain, it indicates that a second domain for @@ -2967,7 +3014,7 @@ while ((domain = string_nextinlist(&list, &sep, buffer, sizeof(buffer))) != NULL if (sender_host_address == NULL) return FAIL; /* can never match */ if (revadd[0] == 0) invert_address(revadd, sender_host_address); rc = one_check_dnsbl(domain, domain_txt, sender_host_address, revadd, - iplist, bitmask, invert_result, defer_return); + iplist, bitmask, match_type, defer_return); if (rc == OK) { dnslist_domain = string_copy(domain_txt); @@ -3000,7 +3047,7 @@ while ((domain = string_nextinlist(&list, &sep, buffer, sizeof(buffer))) != NULL } rc = one_check_dnsbl(domain, domain_txt, keydomain, prepend, iplist, - bitmask, invert_result, defer_return); + bitmask, match_type, defer_return); if (rc == OK) { diff --git a/test/confs/0139 b/test/confs/0139 index cd9433a92..2f844b5f0 100644 --- a/test/confs/0139 +++ b/test/confs/0139 @@ -13,13 +13,30 @@ gecos_name = CALLER_NAME domainlist local_domains = exim.test.ex trusted_users = CALLER +acl_smtp_helo = check_helo acl_smtp_rcpt = check_recipient acl_smtp_mail = check_mail +acl_smtp_vrfy = check_vrfy # ------ ACL ------ begin acl +check_helo: + warn dnslists = rbl2.test.ex!=127.0.0.3 : rbl3.test.ex=127.0.0.3 + accept + +check_vrfy: + warn dnslists = rbl.test.ex=127.0.0.1 + warn dnslists = rbl.test.ex!=127.0.0.1 + warn dnslists = rbl.test.ex!=127.0.0.3 + warn dnslists = rbl.test.ex==127.0.0.1 + warn dnslists = rbl.test.ex==127.0.0.1,127.0.0.2 + warn dnslists = rbl.test.ex!==127.0.0.1 + warn dnslists = rbl.test.ex!==127.0.0.3 + warn dnslists = rbl.test.ex!==127.0.0.1,127.0.0.2 + accept + check_mail: warn dnslists = rbl4.test.ex&0.0.0.6 warn dnslists = rbl4.test.ex&127.0.0.3 diff --git a/test/scripts/0000-Basic/0139 b/test/scripts/0000-Basic/0139 index 4cd168e18..0224bd9d0 100644 --- a/test/scripts/0000-Basic/0139 +++ b/test/scripts/0000-Basic/0139 @@ -32,4 +32,12 @@ test data . quit **** +exim -bh V4NET.11.12.15 +helo a.b +quit +**** +exim -bh V4NET.13.13.2 +vrfy a@b +quit +**** no_msglog_check diff --git a/test/stderr/0139 b/test/stderr/0139 index 14cdd0647..42ebd686d 100644 --- a/test/stderr/0139 +++ b/test/stderr/0139 @@ -259,3 +259,104 @@ LOG: H=[V4NET.11.12.15] F= rejected RCPT >> warn: condition test failed >>> processing "accept" >>> accept: condition test succeeded +>>> host in hosts_connection_nolog? no (option unset) +>>> host in host_lookup? no (option unset) +>>> host in host_reject_connection? no (option unset) +>>> host in sender_unqualified_hosts? no (option unset) +>>> host in recipient_unqualified_hosts? no (option unset) +>>> host in helo_verify_hosts? no (option unset) +>>> host in helo_try_verify_hosts? no (option unset) +>>> host in helo_accept_junk_hosts? no (option unset) +>>> a.b in helo_lookup_domains? no (end of list) +>>> using ACL "check_helo" +>>> processing "warn" +>>> check dnslists = rbl2.test.ex!=127.0.0.3 : rbl3.test.ex=127.0.0.3 +>>> DNS list check: rbl2.test.ex!=127.0.0.3 +>>> new DNS lookup for 15.12.11.V4NET.rbl2.test.ex +>>> DNS lookup for 15.12.11.V4NET.rbl2.test.ex failed +>>> => that means V4NET.11.12.15 is not listed at rbl2.test.ex +>>> DNS list check: rbl3.test.ex=127.0.0.3 +>>> new DNS lookup for 15.12.11.V4NET.rbl3.test.ex +>>> DNS lookup for 15.12.11.V4NET.rbl3.test.ex succeeded (yielding 127.0.0.3) +>>> => that means V4NET.11.12.15 is listed at rbl3.test.ex +>>> warn: condition test succeeded +>>> processing "accept" +>>> accept: condition test succeeded +>>> host in hosts_connection_nolog? no (option unset) +>>> host in host_lookup? no (option unset) +>>> host in host_reject_connection? no (option unset) +>>> host in sender_unqualified_hosts? no (option unset) +>>> host in recipient_unqualified_hosts? no (option unset) +>>> host in helo_verify_hosts? no (option unset) +>>> host in helo_try_verify_hosts? no (option unset) +>>> host in helo_accept_junk_hosts? no (option unset) +>>> host in smtp_accept_max_nonmail_hosts? yes (matched "*") +>>> using ACL "check_vrfy" +>>> processing "warn" +>>> check dnslists = rbl.test.ex=127.0.0.1 +>>> DNS list check: rbl.test.ex=127.0.0.1 +>>> new DNS lookup for 2.13.13.V4NET.rbl.test.ex +>>> DNS lookup for 2.13.13.V4NET.rbl.test.ex succeeded (yielding 127.0.0.1, 127.0.0.2) +>>> => that means V4NET.13.13.2 is listed at rbl.test.ex +>>> warn: condition test succeeded +>>> processing "warn" +>>> check dnslists = rbl.test.ex!=127.0.0.1 +>>> DNS list check: rbl.test.ex!=127.0.0.1 +>>> using result of previous DNS lookup +>>> DNS lookup for 2.13.13.V4NET.rbl.test.ex succeeded (yielding 127.0.0.1, 127.0.0.2) +>>> => but we are not accepting this block class because +>>> => there was an exclude match for =127.0.0.1 +>>> warn: condition test failed +>>> processing "warn" +>>> check dnslists = rbl.test.ex!=127.0.0.3 +>>> DNS list check: rbl.test.ex!=127.0.0.3 +>>> using result of previous DNS lookup +>>> DNS lookup for 2.13.13.V4NET.rbl.test.ex succeeded (yielding 127.0.0.1, 127.0.0.2) +>>> => that means V4NET.13.13.2 is listed at rbl.test.ex +>>> warn: condition test succeeded +>>> processing "warn" +>>> check dnslists = rbl.test.ex==127.0.0.1 +>>> DNS list check: rbl.test.ex==127.0.0.1 +>>> using result of previous DNS lookup +>>> DNS lookup for 2.13.13.V4NET.rbl.test.ex succeeded (yielding 127.0.0.1, 127.0.0.2) +>>> => but we are not accepting this block class because +>>> => there was an IP address that did not match for ==127.0.0.1 +>>> warn: condition test failed +>>> processing "warn" +>>> check dnslists = rbl.test.ex==127.0.0.1,127.0.0.2 +>>> DNS list check: rbl.test.ex==127.0.0.1,127.0.0.2 +>>> using result of previous DNS lookup +>>> DNS lookup for 2.13.13.V4NET.rbl.test.ex succeeded (yielding 127.0.0.1, 127.0.0.2) +>>> => that means V4NET.13.13.2 is listed at rbl.test.ex +>>> warn: condition test succeeded +>>> processing "warn" +>>> check dnslists = rbl.test.ex!==127.0.0.1 +>>> DNS list check: rbl.test.ex!==127.0.0.1 +>>> using result of previous DNS lookup +>>> DNS lookup for 2.13.13.V4NET.rbl.test.ex succeeded (yielding 127.0.0.1, 127.0.0.2) +>>> => that means V4NET.13.13.2 is listed at rbl.test.ex +>>> warn: condition test succeeded +>>> processing "warn" +>>> check dnslists = rbl.test.ex!==127.0.0.3 +>>> DNS list check: rbl.test.ex!==127.0.0.3 +>>> using result of previous DNS lookup +>>> DNS lookup for 2.13.13.V4NET.rbl.test.ex succeeded (yielding 127.0.0.1, 127.0.0.2) +>>> => that means V4NET.13.13.2 is listed at rbl.test.ex +>>> warn: condition test succeeded +>>> processing "warn" +>>> check dnslists = rbl.test.ex!==127.0.0.1,127.0.0.2 +>>> DNS list check: rbl.test.ex!==127.0.0.1,127.0.0.2 +>>> using result of previous DNS lookup +>>> DNS lookup for 2.13.13.V4NET.rbl.test.ex succeeded (yielding 127.0.0.1, 127.0.0.2) +>>> => but we are not accepting this block class because +>>> => there were no IP addresses that did not match for ==127.0.0.1,127.0.0.2 +>>> warn: condition test failed +>>> processing "accept" +>>> accept: condition test succeeded +>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> +>>> routing a@b +>>> calling system_aliases router +>>> system_aliases router declined for a@b +>>> a in "userx"? no (end of list) +>>> no more routers +LOG: VRFY failed for a@b H=[V4NET.13.13.2] diff --git a/test/stdout/0139 b/test/stdout/0139 index 011f2d4fb..3736dd4fd 100644 --- a/test/stdout/0139 +++ b/test/stdout/0139 @@ -50,3 +50,19 @@ 354 Enter message, ending with "." on a line by itself 250 OK id=10HmaX-0005vi-00 221 the.local.host.name closing connection + +**** SMTP testing session as if from host V4NET.11.12.15 +**** but without any ident (RFC 1413) callback. +**** This is not for real! + +220 the.local.host.name ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 +250 the.local.host.name Hello a.b [V4NET.11.12.15] +221 the.local.host.name closing connection + +**** SMTP testing session as if from host V4NET.13.13.2 +**** but without any ident (RFC 1413) callback. +**** This is not for real! + +220 the.local.host.name ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 +550 Unrouteable address +221 the.local.host.name closing connection -- 2.30.2