[Feature] Add R_SPF_PERMFAIL symbol

 - Also yield R_SPF_DNSFAIL on bogus redirect
 - Also grow SPF tests

4 files changed, 108 insertions, 27 deletions

diff --git a/src/libserver/spf.c b/src/libserver/spf.c
index 95bcc7628..dd7843eae 100644
--- a/src/libserver/spf.c
+++ b/src/libserver/spf.c
@@ -314,28 +314,33 @@ rspamd_spf_process_reference (struct spf_resolved *target,
 		if (!(cur->flags & RSPAMD_SPF_FLAG_PARSED)) {
 			/* Unresolved redirect */
 			msg_info_spf ("redirect to %s cannot be resolved", cur->spf_string);
-			return;
+			cur->flags |= RSPAMD_SPF_FLAG_TEMPFAIL;
+		}
+		else {
+			g_assert (cur->flags & RSPAMD_SPF_FLAG_REFRENCE);
+			g_assert (cur->m.idx < rec->resolved->len);
+			relt = g_ptr_array_index (rec->resolved, cur->m.idx);
+			msg_debug_spf ("domain %s is redirected to %s", elt->cur_domain,
+					relt->cur_domain);
 		}
-
-		g_assert (cur->flags & RSPAMD_SPF_FLAG_REFRENCE);
-		g_assert (cur->m.idx < rec->resolved->len);
-		relt = g_ptr_array_index (rec->resolved, cur->m.idx);
-		msg_debug_spf ("domain %s is redirected to %s", elt->cur_domain,
-				relt->cur_domain);
 	}
 
 	for (i = 0; i < elt->elts->len; i++) {
 		cur = g_ptr_array_index (elt->elts, i);
 
 		if (cur->flags & RSPAMD_SPF_FLAG_TEMPFAIL) {
-			target->failed = TRUE;
+			target->temp_failed = TRUE;
 			continue;
 		}
-		else if (!(cur->flags & RSPAMD_SPF_FLAG_PARSED)) {
+		if (cur->flags & RSPAMD_SPF_FLAG_NA) {
+			target->na = TRUE;
+			continue;
+		}
+		if (!(cur->flags & RSPAMD_SPF_FLAG_PARSED)) {
 			/* Ignore unparsed addrs */
 			continue;
 		}
-		else if (cur->flags & RSPAMD_SPF_FLAG_REFRENCE) {
+		if (cur->flags & RSPAMD_SPF_FLAG_REFRENCE) {
 			/* Process reference */
 			if (cur->flags & RSPAMD_SPF_FLAG_REDIRECT) {
 				/* Stop on redirected domain */
@@ -371,15 +376,20 @@ rspamd_spf_record_flatten (struct spf_record *rec)
 
 	g_assert (rec != NULL);
 
-	res = g_slice_alloc0 (sizeof (*res));
-	res->elts = g_array_sized_new (FALSE, FALSE, sizeof (struct spf_addr),
-			rec->resolved->len);
-	res->domain = g_strdup (rec->sender_domain);
-	res->ttl = rec->ttl;
-	REF_INIT_RETAIN (res, rspamd_flatten_record_dtor);
+	if (rec->resolved) {
+		res = g_slice_alloc0 (sizeof (*res));
+		res->elts = g_array_sized_new (FALSE, FALSE, sizeof (struct spf_addr),
+				rec->resolved->len);
+		res->domain = g_strdup (rec->sender_domain);
+		res->ttl = rec->ttl;
+		REF_INIT_RETAIN (res, rspamd_flatten_record_dtor);
 
-	if (rec->resolved->len > 0) {
-		rspamd_spf_process_reference (res, NULL, rec, TRUE);
+		if (rec->resolved->len > 0) {
+			rspamd_spf_process_reference (res, NULL, rec, TRUE);
+		}
+	}
+	else {
+		return rec;
 	}
 
 	return res;
@@ -1787,8 +1797,26 @@ spf_dns_callback (struct rdns_reply *reply, gpointer arg)
 			/* Top level resolved element */
 			rec->ttl = reply->entries->ttl;
 		}
-
-		spf_process_txt_record (rec, resolved, reply);
+	}
+	else if ((reply->code == RDNS_RC_NOREC || reply->code == RDNS_RC_NXDOMAIN)
+			&& rec->dns_requests == 0) {
+		resolved = rspamd_spf_new_addr_list (rec, rec->sender_domain);
+		struct spf_addr *addr;
+		addr = g_slice_alloc0 (sizeof (*addr));
+		addr->flags = 0;
+		addr->flags |= RSPAMD_SPF_FLAG_NA;
+		g_ptr_array_insert(resolved->elts, 0, addr);
+	}
+ 
+	if (!spf_process_txt_record (rec, resolved, reply)) {
+		if (rec->dns_requests == 0) {
+			resolved = g_ptr_array_index (rec->resolved, 0);
+			struct spf_addr *addr;
+			addr = g_slice_alloc0 (sizeof (*addr));
+			addr->flags = 0;
+			addr->flags |= RSPAMD_SPF_FLAG_NA;
+			g_ptr_array_insert(resolved->elts, 0, addr);
+		}
 	}
 
 	rspamd_spf_maybe_return (rec);
diff --git a/src/libserver/spf.h b/src/libserver/spf.h
index 001d3775a..1e2bcfe45 100644
--- a/src/libserver/spf.h
+++ b/src/libserver/spf.h
@@ -38,6 +38,7 @@ typedef enum spf_action_e {
 #define RSPAMD_SPF_FLAG_REFRENCE (1 << 6)
 #define RSPAMD_SPF_FLAG_REDIRECT (1 << 7)
 #define RSPAMD_SPF_FLAG_TEMPFAIL (1 << 8)
+#define RSPAMD_SPF_FLAG_NA (1 << 9)
 
 struct spf_addr {
 	guchar addr6[sizeof (struct in6_addr)];
@@ -58,7 +59,8 @@ struct spf_addr {
 struct spf_resolved {
 	gchar *domain;
 	guint ttl;
-	gboolean failed;
+	gboolean temp_failed;
+	gboolean na;
 	GArray *elts; /* Flat list of struct spf_addr */
 	ref_entry_t ref; /* Refcounting */
 };
diff --git a/src/plugins/spf.c b/src/plugins/spf.c
index 6d5517db9..33df85262 100644
--- a/src/plugins/spf.c
+++ b/src/plugins/spf.c
@@ -22,6 +22,7 @@
  * - symbol_softfail (string): symbol to insert (default: 'R_SPF_SOFTFAIL')
  * - symbol_na (string): symbol to insert (default: 'R_SPF_NA')
  * - symbol_dnsfail (string): symbol to insert (default: 'R_SPF_DNSFAIL')
+ * - symbol_permfail (string): symbol to insert (default: 'R_SPF_PERMFAIL')
  * - whitelist (map): map of whitelisted networks
  */
 
@@ -38,6 +39,7 @@
 #define DEFAULT_SYMBOL_NEUTRAL "R_SPF_NEUTRAL"
 #define DEFAULT_SYMBOL_ALLOW "R_SPF_ALLOW"
 #define DEFAULT_SYMBOL_DNSFAIL "R_SPF_DNSFAIL"
+#define DEFAULT_SYMBOL_PERMFAIL "R_SPF_PERMFAIL"
 #define DEFAULT_SYMBOL_NA "R_SPF_NA"
 #define DEFAULT_CACHE_SIZE 2048
 #define DEFAULT_CACHE_MAXAGE 86400
@@ -50,6 +52,7 @@ struct spf_ctx {
 	const gchar *symbol_allow;
 	const gchar *symbol_dnsfail;
 	const gchar *symbol_na;
+	const gchar *symbol_permfail;
 
 	rspamd_mempool_t *spf_pool;
 	radix_compressed_t *whitelist_ip;
@@ -158,6 +161,15 @@ spf_module_init (struct rspamd_config *cfg, struct module_ctx **ctx)
 			0);
 	rspamd_rcl_add_doc_by_path (cfg,
 			"spf",
+			"Symbol that is added if SPF policy is invalid",
+			"symbol_permfail",
+			UCL_STRING,
+			NULL,
+			0,
+			NULL,
+			0);
+	rspamd_rcl_add_doc_by_path (cfg,
+			"spf",
 			"Size of SPF parsed records cache",
 			"spf_cache_size",
 			UCL_INT,
@@ -226,6 +238,13 @@ spf_module_config (struct rspamd_config *cfg)
 		spf_module_ctx->symbol_na = DEFAULT_SYMBOL_NA;
 	}
 	if ((value =
+		rspamd_config_get_module_opt (cfg, "spf", "symbol_permfail")) != NULL) {
+		spf_module_ctx->symbol_permfail = ucl_obj_tostring (value);
+	}
+	else {
+		spf_module_ctx->symbol_permfail = DEFAULT_SYMBOL_PERMFAIL;
+	}
+	if ((value =
 		rspamd_config_get_module_opt (cfg, "spf", "spf_cache_size")) != NULL) {
 		cache_size = ucl_obj_toint (value);
 	}
@@ -252,6 +271,11 @@ spf_module_config (struct rspamd_config *cfg)
 			SYMBOL_TYPE_VIRTUAL,
 			cb_id);
 	rspamd_symbols_cache_add_symbol (cfg->cache,
+			spf_module_ctx->symbol_permfail, 0,
+			NULL, NULL,
+			SYMBOL_TYPE_VIRTUAL,
+			cb_id);
+	rspamd_symbols_cache_add_symbol (cfg->cache,
 			spf_module_ctx->symbol_na, 0,
 			NULL, NULL,
 			SYMBOL_TYPE_VIRTUAL,
@@ -373,7 +397,7 @@ spf_check_element (struct spf_resolved *rec, struct spf_addr *addr,
 			spf_result[0] = '-';
 			spf_message = "(SPF): spf fail";
 			if (addr->flags & RSPAMD_SPF_FLAG_ANY) {
-				if (rec->failed) {
+				if (rec->temp_failed) {
 					msg_info_task ("do not apply SPF failed policy, as we have "
 							"some addresses unresolved");
 					spf_symbol = spf_module_ctx->symbol_dnsfail;
@@ -387,7 +411,7 @@ spf_check_element (struct spf_resolved *rec, struct spf_addr *addr,
 			spf_result[0] = '~';
 
 			if (addr->flags & RSPAMD_SPF_FLAG_ANY) {
-				if (rec->failed) {
+				if (rec->temp_failed) {
 					msg_info_task ("do not apply SPF failed policy, as we have "
 							"some addresses unresolved");
 					spf_symbol = spf_module_ctx->symbol_dnsfail;
@@ -442,20 +466,32 @@ spf_plugin_callback (struct spf_resolved *record, struct rspamd_task *task,
 	struct spf_resolved *l;
 	struct rspamd_async_watcher *w = ud;
 
-	if (record && record->elts->len == 0 && !record->failed) {
+	if (record && record->na) {
 		rspamd_task_insert_result (task,
 				spf_module_ctx->symbol_na,
 				1,
 				NULL);
 	}
-	else if (record && record->elts->len > 0 && record->domain) {
+	else if (record && record->elts->len == 0 && record->temp_failed) {
+		rspamd_task_insert_result (task,
+				spf_module_ctx->symbol_dnsfail,
+				1,
+				NULL);
+	}
+	else if (record && record->elts->len == 0) {
+		rspamd_task_insert_result (task,
+				spf_module_ctx->symbol_permfail,
+				1,
+				NULL);
+	}
+	else if (record && record->domain) {
 
 		if ((l = rspamd_lru_hash_lookup (spf_module_ctx->spf_hash,
 					record->domain, task->tv.tv_sec)) == NULL) {
 
 			l = spf_record_ref (record);
 
-			if (!record->failed) {
+			if (!record->temp_failed) {
 				rspamd_lru_hash_insert (spf_module_ctx->spf_hash,
 						record->domain, l,
 						task->tv.tv_sec, record->ttl);
diff --git a/test/functional/cases/115_dmarc.robot b/test/functional/cases/115_dmarc.robot
index 2f2c51aa5..82616c8f9 100644
--- a/test/functional/cases/115_dmarc.robot
+++ b/test/functional/cases/115_dmarc.robot
@@ -92,11 +92,26 @@ SPF NA NA
   ...  -i  8.8.8.8  -F  x@za
   Check Rspamc  ${result}  R_SPF_NA
 
-SPF NA NXDOMAIN
+SPF NA NOREC
   ${result} =  Scan Message With Rspamc  ${TESTDIR}/messages/dmarc/bad_dkim1.eml
   ...  -i  8.8.8.8  -F  x@co.za
   Check Rspamc  ${result}  R_SPF_NA
 
+SPF NA NXDOMAIN
+  ${result} =  Scan Message With Rspamc  ${TESTDIR}/messages/dmarc/bad_dkim1.eml
+  ...  -i  8.8.8.8  -F  x@zzzzaaaa
+  Check Rspamc  ${result}  R_SPF_NA
+
+SPF DNSFAIL UNRESOLVEABLE REDIRECT
+  ${result} =  Scan Message With Rspamc  ${TESTDIR}/messages/dmarc/bad_dkim1.eml
+  ...  -i  8.8.8.8  -F  x@cacophony.za.org
+  Check Rspamc  ${result}  R_SPF_DNSFAIL
+
+SPF PERMFAIL
+  ${result} =  Scan Message With Rspamc  ${TESTDIR}/messages/dmarc/bad_dkim1.eml
+  ...  -i  8.8.8.8  -F  x@xzghgh.za.org
+  Check Rspamc  ${result}  R_SPF_PERMFAIL
+
 SPF FAIL
   ${result} =  Scan Message With Rspamc  ${TESTDIR}/messages/dmarc/bad_dkim1.eml
   ...  -i  8.8.8.8  -F  x@example.net