dnslists: make valid lookup result addresses configurable. Bug 2631

nomis · nomis · commit ae2db5ee4dff · 2021-08-07T23:48:32.000+01:00
This allows the Spamhaus error addresses to be considered an error, e.g.:
  dnslist_valid_addresses = ${if match{$dnslist_domain}{\N.spamhaus.org$\N}{!127.255.255.0/24 : }}127.0.0.0/8
diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
@@ -15885,6 +15885,20 @@ means that DNSSEC will not work with Exim on that platform either, unless Exim
 is linked against an alternative DNS client library.
 
 
+.new
+.option dnslist_valid_addresses main "host list&!!" &`127.0.0.0/8`&
+.cindex "dnslists ACL condition" "valid addresses"
+This option specifies IP addresses that are acceptable in dnslist responses.
+
+Responses containing address records that do not match this list will be logged
+as an error and ignored. This helps identify list domains that have been taken
+over by a domain-parking registrar.
+
+The variable &$dnslist_domain$& contains the name of the overall domain that
+matched (for example, &`spamhaus.example`&).
+.wen
+
+
 .option drop_cr main boolean false
 This is an obsolete option that is now a no-op. It used to affect the way Exim
 handled CR and LF characters in incoming messages. What happens now is
@@ -32601,6 +32615,12 @@ connection (assuming long-enough TTL).
 Exim does not share information between multiple incoming
 connections (but your local name server cache should be active).
 
+.new
+DNS list responses are filtered using the &%dnslist_valid_addresses%& host list
+before use to protect against list domains that have been taken over by a
+domain-parking registrar and no longer return values in the 127.0.0.0/8 range.
+.wen
+
 There are a number of DNS lists to choose from, some commercial, some free,
 or free for small deployments.  An overview can be found at
 &url(https://en.wikipedia.org/wiki/Comparison_of_DNS_blacklists).
diff --git a/src/src/dnsbl.c b/src/src/dnsbl.c
@@ -203,6 +203,8 @@ if (cb->rc == DNS_SUCCEED)
   {
   dns_address * da = NULL;
   uschar *addlist = cb->rhs->address;
+  uschar *orig_dnslist_domain = NULL;
+  int filter_rc = FAIL;
 
   /* For A and AAAA records, there may be multiple addresses from multiple
   records. For A6 records (currently not expected to be used) there may be
@@ -214,6 +216,76 @@ if (cb->rc == DNS_SUCCEED)
   HDEBUG(D_dnsbl) debug_printf("DNS lookup for %s succeeded (yielding %s)\n",
     query, addlist);
 
+  /* Make dnslist_domain available to dnslist_valid_addresses expansion. */
+  orig_dnslist_domain = dnslist_domain;
+  dnslist_domain = domain_txt;
+
+  for (da = cb->rhs; da; da = da->next)
+    {
+    switch (verify_check_this_host(&dnslist_valid_addresses, NULL, US"", da->address, NULL))
+      {
+      case OK:
+        da->dnsbl_invalid = FALSE;
+
+        if (filter_rc != DEFER)
+          filter_rc = OK;
+        break;
+
+      case FAIL:
+        da->dnsbl_invalid = TRUE;
+        addlist = NULL;
+
+        log_write(0, LOG_MAIN,
+          "DNS list lookup for %s at %s returned %s;"
+          " invalid address discarded",
+          keydomain, domain, da->address);
+        break;
+
+      case DEFER:
+        log_write(0, LOG_MAIN,
+          "DNS list lookup for %s at %s returned %s;"
+          " unable to verify, returned DEFER",
+          keydomain, domain, da->address);
+
+        filter_rc = DEFER;
+        break;
+      }
+    }
+
+  dnslist_domain = orig_dnslist_domain;
+
+  if (filter_rc == FAIL)
+    {
+    HDEBUG(D_dnsbl)
+      {
+        debug_printf("=> all addresses are invalid\n");
+        debug_printf("=> that means %s is not listed at %s\n",
+          keydomain, domain);
+      }
+    }
+
+  if (filter_rc != OK) return filter_rc;
+
+  /* Need to recreate addlist without filtered addresses. */
+  if (addlist == NULL)
+    {
+    for (da = cb->rhs; da; da = da->next)
+      {
+      if (da->dnsbl_invalid)
+        continue;
+
+      if (addlist == NULL)
+        addlist = da->address;
+      else
+        addlist = string_sprintf("%s, %s", addlist, da->address);
+      }
+
+    HDEBUG(D_dnsbl)
+      {
+        debug_printf("=> updated address list: %s\n", addlist);
+      }
+    }
+
   /* Address list check; this can be either for equality, or via a bitmask.
   In the latter case, all the bits must match. */
 
@@ -225,6 +297,9 @@ if (cb->rc == DNS_SUCCEED)
       const uschar *ptr = iplist;
       uschar *res;
 
+      if (da->dnsbl_invalid)
+        continue;
+
       /* Handle exact matching */
 
       if (!bitmask)
@@ -249,14 +324,7 @@ if (cb->rc == DNS_SUCCEED)
         We change this only for IPv4 addresses in the list. */
 
         if (host_aton(da->address, address) == 1)
-	  if ((address[0] & 0xff000000) != 0x7f000000)    /* 127.0.0.0/8 */
-	    log_write(0, LOG_MAIN,
-	      "DNS list lookup for %s at %s returned %s;"
-	      " not in 127.0/8 and discarded",
-	      keydomain, domain, da->address);
-
-	  else
-	    mask = address[0];
+          mask = address[0];
 
         /* Scan the returned addresses, skipping any that are IPv6 */
 
@@ -311,33 +379,6 @@ if (cb->rc == DNS_SUCCEED)
       }
     }
 
-  /* No address list check; discard any illegal returns and give up if
-  none remain. */
-
-  else
-    {
-    BOOL ok = FALSE;
-    for (da = cb->rhs; da; da = da->next)
-      {
-      int address[4];
-
-      if (  host_aton(da->address, address) == 1		/* ipv4 */
-	 && (address[0] & 0xff000000) == 0x7f000000	/* 127.0.0.0/8 */
-	 )
-	ok = TRUE;
-      else
-	log_write(0, LOG_MAIN,
-	    "DNS list lookup for %s at %s returned %s;"
-	    " not in 127.0/8 and discarded",
-	    keydomain, domain, da->address);
-      }
-    if (!ok)
-      {
-      yield = FAIL;
-      goto out;
-      }
-    }
-
   /* Either there was no IP list, or the record matched, implying that the
   domain is on the list. We now want to find a corresponding TXT record. If an
   alternate domain is specified for the TXT record, call this function
diff --git a/src/src/globals.c b/src/src/globals.c
@@ -898,6 +898,7 @@ int     dns_use_edns0          = -1; /* <0 = not coerced */
 uschar *dnslist_domain         = NULL;
 uschar *dnslist_matched        = NULL;
 uschar *dnslist_text           = NULL;
+const uschar *dnslist_valid_addresses = US"127.0.0.0/8";
 uschar *dnslist_value          = NULL;
 tree_node *domainlist_anchor   = NULL;
 int     domainlist_count       = 0;
diff --git a/src/src/globals.h b/src/src/globals.h
@@ -567,6 +567,7 @@ extern int     dns_use_edns0;          /* Coerce EDNS0 support on/off in resolve
 extern uschar *dnslist_domain;         /* DNS (black) list domain */
 extern uschar *dnslist_matched;        /* DNS (black) list matched key */
 extern uschar *dnslist_text;           /* DNS (black) list text message */
+extern const uschar *dnslist_valid_addresses; /* DNS list IP addresses that are considered valid (127.0.0.0/8) */
 extern uschar *dnslist_value;          /* DNS (black) list IP address */
 extern tree_node *domainlist_anchor;   /* Tree of defined domain lists */
 extern int     domainlist_count;       /* Number defined */
diff --git a/src/src/readconf.c b/src/src/readconf.c
@@ -143,6 +143,7 @@ static optionlist optionlist_config[] = {
   { "dns_retry",                opt_int,         {&dns_retry} },
   { "dns_trust_aa",             opt_stringptr,   {&dns_trust_aa} },
   { "dns_use_edns0",            opt_int,         {&dns_use_edns0} },
+  { "dnslist_valid_addresses",  opt_stringptr,   {&dnslist_valid_addresses} },
  /* This option is now a no-op, retained for compatibility */
   { "drop_cr",                  opt_bool,        {&drop_cr} },
 /*********************************************************/
diff --git a/src/src/structs.h b/src/src/structs.h
@@ -796,6 +796,7 @@ address. */
 
 typedef struct dns_address {
   struct dns_address *next;
+  BOOL    dnsbl_invalid;
   uschar  address[1];
 } dns_address;
 
diff --git a/test/confs/0139 b/test/confs/0139
@@ -9,6 +9,16 @@ domainlist local_domains = exim.test.ex
 trusted_users = CALLER
 hosts_require_helo =
 
+.ifdef DNSBL_127_255
+# Split into two /9s so that it's visible in the debug output
+dnslist_valid_addresses = ${if eq{$dnslist_domain}{rbl.test.ex}{!127.255.255.0/24 : 127.0.0.0/9 : 127.128.0.0/9}{127.0.0.0/8}}
+.endif
+
+.ifdef DNSBL_DEFER
+# Expansion failure
+dnslist_valid_addresses = ${if eq{intentional_expansion_failure
+.endif
+
 acl_smtp_helo = check_helo
 acl_smtp_rcpt = check_recipient
 acl_smtp_mail = check_mail
diff --git a/test/dnszones-src/db.test.ex b/test/dnszones-src/db.test.ex
@@ -212,6 +212,15 @@ TTL=2 14.12.11.V4NET.rbl A 127.0.0.2
 105.13.13.V4NET.rbl    A   255.255.255.255
                        A   255.255.255.254
 
+; Configuration to consider 127.255.255.0/24 as invalid
+
+106.13.13.V4NET.rbl    A   127.255.255.255
+
+; Exact match along with invalid return value
+
+107.13.13.V4NET.rbl    A   127.0.0.1
+107.13.13.V4NET.rbl    A   128.0.0.0
+
 ; -------- Testing MX records --------
 
 mxcased      MX  5  ten-99.TEST.EX.
diff --git a/test/log/0509 b/test/log/0509
@@ -1,4 +1,4 @@
 1999-03-02 09:44:33 rbl.test.ex/<;1.2.3.4;V4NET.11.12.13
-1999-03-02 09:44:33 DNS list lookup for ten-1 at test.ex returned V4NET.0.0.1; not in 127.0/8 and discarded
+1999-03-02 09:44:33 DNS list lookup for ten-1 at test.ex returned V4NET.0.0.1; invalid address discarded
 1999-03-02 09:44:33 test.ex/a.b.c.d::ten-1::localhost
 1999-03-02 09:44:33 U=CALLER rejected connection in "connect" ACL
diff --git a/test/scripts/0000-Basic/0139 b/test/scripts/0000-Basic/0139
@@ -65,4 +65,24 @@ exim -bh V4NET.13.13.105
 vrfy a@b
 quit
 ****
+exim -bh V4NET.13.13.106
+vrfy a@b
+quit
+****
+exim -DDNSBL_127_255 -bh V4NET.13.13.2
+vrfy a@b
+quit
+****
+exim -DDNSBL_127_255 -bh V4NET.13.13.106
+vrfy a@b
+quit
+****
+exim -DDNSBL_DEFER -bh V4NET.13.13.2
+vrfy a@b
+quit
+****
+exim -bh V4NET.13.13.107
+vrfy a@b
+quit
+****
 no_msglog_check
diff --git a/test/stderr/0139 b/test/stderr/0139
diff --git a/test/stdout/0139 b/test/stdout/0139
