From baf74ba8c1f6bff834d93aaee9e6afd6b5c97aef Mon Sep 17 00:00:00 2001 From: Andrew Lewis Date: Thu, 22 Sep 2016 11:55:07 +0200 Subject: [PATCH] [Feature] Add R_SPF_PERMFAIL symbol - Also yield R_SPF_DNSFAIL on bogus redirect - Also grow SPF tests --- src/libserver/spf.c | 68 +++++++++++++++++++-------- src/libserver/spf.h | 4 +- src/plugins/spf.c | 46 ++++++++++++++++-- test/functional/cases/115_dmarc.robot | 17 ++++++- 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; @@ -156,6 +159,15 @@ spf_module_init (struct rspamd_config *cfg, struct module_ctx **ctx) 0, NULL, 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", @@ -225,6 +237,13 @@ spf_module_config (struct rspamd_config *cfg) else { 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); @@ -251,6 +270,11 @@ spf_module_config (struct rspamd_config *cfg) NULL, NULL, 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, @@ -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 -- 2.39.5