[Fix] spf: use the enclosing element for exists

`parse_spf_exists` picked the current record as the last element of the
resolved array, which only holds while nothing else has appended an
element of its own. An `include` earlier in the same record does exactly
that, so from then on every `exists` in that record was attached to the
element belonging to the include.

Pass the element down from `spf_process_element` as the other mechanisms
already do. The domain spec of an `exists` is expanded before this
point, hence the visible effect so far was limited to a wrong domain in
the diagnostics of an unresolvable `exists`.

Cover the mechanism itself as well, it had no functional tests at all.

diff --git a/src/libserver/spf.c b/src/libserver/spf.c
index 67976d1e5cc439cb3e2c10ff33a946c1aea1a73e..f9e2843ee02247e30e04fb867785bac47898ba65 100644 (file)
--- a/src/libserver/spf.c
+++ b/src/libserver/spf.c
@@ -1889,14 +1889,13 @@ parse_spf_redirect(struct spf_record *rec,
 }
 
 static gboolean
-parse_spf_exists(struct spf_record *rec, struct spf_addr *addr)
+parse_spf_exists(struct spf_record *rec,
+                                struct spf_resolved_element *resolved, struct spf_addr *addr)
 {
        struct spf_dns_cb *cb;
        const char *host;
        struct rspamd_task *task = rec->task;
-       struct spf_resolved_element *resolved;
 
-       resolved = g_ptr_array_index(rec->resolved, rec->resolved->len - 1);
        CHECK_REC(rec);
 
        /* Check if element has unresolved macros */
@@ -2537,7 +2536,7 @@ spf_process_element(struct spf_record *rec,
                }
                else if (g_ascii_strncasecmp(begin, SPF_EXISTS,
                                                                         sizeof(SPF_EXISTS) - 1) == 0) {
-                       res = parse_spf_exists(rec, addr);
+                       res = parse_spf_exists(rec, resolved, addr);
                }
                else {
                        msg_notice_spf("spf error for domain %s: bad spf command %s",

diff --git a/test/functional/cases/001_merged/117_spf.robot b/test/functional/cases/001_merged/117_spf.robot
index 789f4eef07bb0c8c0feab78dfb4e44ea03310fc3..e0a49bfafbb95cdec940b02de65356e6c3074444 100644 (file)
--- a/test/functional/cases/001_merged/117_spf.robot
+++ b/test/functional/cases/001_merged/117_spf.robot
@@ -200,6 +200,24 @@ SPF NESTING LIMIT
   Expect Symbol  R_SPF_FAIL
   Do Not Expect Symbol  R_SPF_ALLOW
 
+SPF ALLOW EXISTS
+  [Documentation]  RFC 7208 5.7: an exists element whose name resolves matches
+  ...  any sender, here it follows an include that has appended an element of
+  ...  its own to the record
+  Scan File  ${RSPAMD_TESTDIR}/messages/dmarc/bad_dkim1.eml
+  ...  IP=8.8.8.8  From=x@exists.org.org.za
+  ...  Settings=${SETTINGS_SPF}
+  Expect Symbol  R_SPF_ALLOW
+
+SPF FAIL UNRESOLVEABLE EXISTS
+  [Documentation]  An exists element whose name does not resolve matches
+  ...  nothing, so the trailing -all applies
+  Scan File  ${RSPAMD_TESTDIR}/messages/dmarc/bad_dkim1.eml
+  ...  IP=8.8.8.8  From=x@noexists.org.org.za
+  ...  Settings=${SETTINGS_SPF}
+  Expect Symbol  R_SPF_FAIL
+  Do Not Expect Symbol  R_SPF_ALLOW
+
 SPF DNS REQUESTS LIMIT
   [Documentation]  Exactly max_dns_requests DNS elements (30 by default) are
   ...  evaluated, the 31st one is not, so only the shorter record authorises

diff --git a/test/functional/configs/merged-local.conf b/test/functional/configs/merged-local.conf
index 670aea3c20851deacaca51309aefec0211204870..285232857d819131bd724dd9b4abb07b68764e38 100644 (file)
--- a/test/functional/configs/merged-local.conf
+++ b/test/functional/configs/merged-local.conf
@@ -399,6 +399,16 @@ options = {
         {name = "pad.req.org.org.za", type = "aaaa", rcode = "norec"},
         {name = "hit.req.org.org.za", type = "a", replies = ["8.8.8.8"]},
         {name = "hit.req.org.org.za", type = "aaaa", rcode = "norec"},
+        # 117_spf: exists elements preceded by an include, so that the include
+        # has already appended an element of its own to the record
+        {name = "exists.org.org.za", type = "txt",
+          replies = ["v=spf1 include:inc.exists.org.org.za exists:probe.exists.org.org.za -all"]},
+        {name = "noexists.org.org.za", type = "txt",
+          replies = ["v=spf1 include:inc.exists.org.org.za exists:absent.exists.org.org.za -all"]},
+        {name = "inc.exists.org.org.za", type = "txt",
+          replies = ["v=spf1 ip4:192.0.2.40 -all"]},
+        {name = "probe.exists.org.org.za", type = "a", replies = ["127.0.0.2"]},
+        {name = "absent.exists.org.org.za", type = "a", rcode = "nxdomain"},
         {
           name = "fail7.org.org.za",
           type = "aaaa";