Fix negated dnslists item bug; add == and =& features, courtesy Brad

Jorsch.

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=<postmaster@exim.test.ex> rejected RCPT <userx@exim.te
 >>> 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 <a@b> Unrouteable address

+221 the.local.host.name closing connection