From: Jeremy Harris Date: Sun, 29 Dec 2019 14:46:10 +0000 (+0000) Subject: SPF: fix handling mix of spf and other txt records. Bug 2499 X-Git-Tag: exim-4.93.0.4^0 X-Git-Url: https://vcs.fsf.org/?p=exim.git;a=commitdiff_plain;h=bf4d783783e07ca6c3af9546658dbe86769f177c;ds=sidebyside SPF: fix handling mix of spf and other txt records. Bug 2499 (cherry picked from commits 4533e306fc, b17ea87dd9, 44e90dfa83, 10897f5d74) --- diff --git a/src/src/spf.c b/src/src/spf.c index b7041d3e3..8ead817b9 100644 --- a/src/src/spf.c +++ b/src/src/spf.c @@ -37,90 +37,111 @@ SPF_dns_rr_t * spf_nxdomain = NULL; static SPF_dns_rr_t * SPF_dns_exim_lookup(SPF_dns_server_t *spf_dns_server, -const char *domain, ns_type rr_type, int should_cache) + const char *domain, ns_type rr_type, int should_cache) { dns_answer * dnsa = store_get_dns_answer(); dns_scan dnss; SPF_dns_rr_t * spfrr; +unsigned found = 0; + +SPF_dns_rr_t srr = { + .domain = CS domain, /* query information */ + .domain_buf_len = 0, + .rr_type = rr_type, + + .rr_buf_len = 0, /* answer information */ + .rr_buf_num = 0, /* no free of s */ + .utc_ttl = 0, + + .hook = NULL, /* misc information */ + .source = spf_dns_server +}; +int dns_rc; DEBUG(D_receive) debug_printf("SPF_dns_exim_lookup '%s'\n", domain); -if (dns_lookup(dnsa, US domain, rr_type, NULL) == DNS_SUCCEED) - for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS); rr; - rr = dns_next_rr(dnsa, &dnss, RESET_NEXT)) - if ( rr->type == rr_type - && (rr_type != T_TXT || Ustrncmp(rr->data+1, "v=spf1", 6) == 0)) +switch (dns_rc = dns_lookup(dnsa, US domain, rr_type, NULL)) + { + case DNS_SUCCEED: srr.herrno = NETDB_SUCCESS; break; + case DNS_AGAIN: srr.herrno = TRY_AGAIN; break; + case DNS_NOMATCH: srr.herrno = HOST_NOT_FOUND; break; + case DNS_NODATA: srr.herrno = NO_DATA; break; + case DNS_FAIL: + default: srr.herrno = NO_RECOVERY; break; + } + +for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS); rr; + rr = dns_next_rr(dnsa, &dnss, RESET_NEXT)) + if (rr->type == rr_type) found++; + +if (found == 0) + { + SPF_dns_rr_dup(&spfrr, &srr); + return spfrr; + } + +srr.rr = store_malloc(sizeof(SPF_dns_rr_data_t) * found); + +found = 0; +for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS); rr; + rr = dns_next_rr(dnsa, &dnss, RESET_NEXT)) + if (rr->type == rr_type) + { + const uschar * s = rr->data; + + srr.ttl = rr->ttl; + switch(rr_type) { - const uschar * s = rr->data; - SPF_dns_rr_t srr = { - .domain = CS rr->name, /* query information */ - .domain_buf_len = DNS_MAXNAME, - .rr_type = rr->type, - - .num_rr = 1, /* answer information */ - .rr = NULL, - .rr_buf_len = 0, - .rr_buf_num = 0, - .ttl = rr->ttl, - .utc_ttl = 0, - .herrno = NETDB_SUCCESS, - - .hook = NULL, /* misc information */ - .source = spf_dns_server - }; - - switch(rr_type) + case T_MX: + s += 2; /* skip the MX precedence field */ + case T_PTR: { - case T_MX: - s += 2; /* skip the MX precedence field */ - case T_PTR: - { - uschar * buf = store_malloc(256); - (void)dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen, s, - (DN_EXPAND_ARG4_TYPE)buf, 256); - s = buf; - break; - } + uschar * buf = store_malloc(256); + (void)dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen, s, + (DN_EXPAND_ARG4_TYPE)buf, 256); + s = buf; + break; + } - case T_TXT: + case T_TXT: + { + gstring * g = NULL; + uschar chunk_len; + + if (strncmpic(rr->data+1, US"v=spf1", 6) != 0) { - gstring * g = NULL; - uschar chunk_len; - for (int off = 0; off < rr->size; off += chunk_len) - { - if (!(chunk_len = s[off++])) break; - g = string_catn(g, s+off, chunk_len); - } - if (!g) - { - HDEBUG(D_host_lookup) debug_printf("IP address lookup yielded an " - "empty name: treated as non-existent host name\n"); - continue; - } - gstring_release_unused(g); - s = string_copy_malloc(string_from_gstring(g)); - break; + HDEBUG(D_host_lookup) debug_printf("not an spf record\n"); + continue; } - case T_A: - case T_AAAA: - default: + for (int off = 0; off < rr->size; off += chunk_len) { - uschar * buf = store_malloc(dnsa->answerlen + 1); - s = memcpy(buf, s, dnsa->answerlen + 1); - break; + if (!(chunk_len = s[off++])) break; + g = string_catn(g, s+off, chunk_len); } + if (!g) + continue; + gstring_release_unused(g); + s = string_copy_malloc(string_from_gstring(g)); + break; } - DEBUG(D_receive) debug_printf("SPF_dns_exim_lookup '%s'\n", s); - srr.rr = (void *) &s; - - /* spfrr->rr must have been malloc()d for this */ - SPF_dns_rr_dup(&spfrr, &srr); - return spfrr; + case T_A: + case T_AAAA: + default: + { + uschar * buf = store_malloc(dnsa->answerlen + 1); + s = memcpy(buf, s, dnsa->answerlen + 1); + break; + } } + DEBUG(D_receive) debug_printf("SPF_dns_exim_lookup '%s'\n", s); + srr.rr[found++] = (void *) s; + } -SPF_dns_rr_dup(&spfrr, spf_nxdomain); +srr.num_rr = found; +/* spfrr->rr must have been malloc()d for this */ +SPF_dns_rr_dup(&spfrr, &srr); return spfrr; } @@ -129,14 +150,11 @@ return spfrr; SPF_dns_server_t * SPF_dns_exim_new(int debug) { -SPF_dns_server_t *spf_dns_server; +SPF_dns_server_t * spf_dns_server = store_malloc(sizeof(SPF_dns_server_t)); DEBUG(D_receive) debug_printf("SPF_dns_exim_new\n"); -if (!(spf_dns_server = malloc(sizeof(SPF_dns_server_t)))) - return NULL; memset(spf_dns_server, 0, sizeof(SPF_dns_server_t)); - spf_dns_server->destroy = NULL; spf_dns_server->lookup = SPF_dns_exim_lookup; spf_dns_server->get_spf = NULL; diff --git a/test/dnszones-src/db.example.com b/test/dnszones-src/db.example.com index 683772f77..b12d9a6a6 100644 --- a/test/dnszones-src/db.example.com +++ b/test/dnszones-src/db.example.com @@ -25,6 +25,15 @@ example.com. NS exim.example.com. example.com. TXT v=spf1 -all +double TXT v=spf1 include:_spf.google.com ~all + TXT v=spf1 +a +mx -all + +doubleplus TXT v=spf1 include:_spf.google.com ~all + TXT google-site-verification=q-4MSVLjluQIsBztu5jzJBxAcJXzNcHAk0jHTZEamB8 + TXT v=spf1 +a +mx -all + +uppercase TXT v=sPf1 +all + ; Alias A record for the local host, under the name "server1" server1 A HOSTIPV4 diff --git a/test/scripts/4600-SPF/4601 b/test/scripts/4600-SPF/4601 index ab434611c..96f06a6d1 100644 --- a/test/scripts/4600-SPF/4601 +++ b/test/scripts/4600-SPF/4601 @@ -1,12 +1,5 @@ # lookup string-expansion # -# It is rather difficult to properly test spf. We use libspf2 to do the work, and it -# does the DNS lookups, so we cannot intercept them in the testsuite's usual fashion -# to provide values for testcases. -# -# For now just check that what should be working syntax does not cause us to fall over. -# Be careful with envelope-domains used for testcases, as real DNS lookups will be done. -# exim -bd -DSERVER=server -oX PORT_D:PORT_S **** client 127.0.0.1 PORT_D @@ -31,3 +24,15 @@ quit **** # killdaemon +# +# SERVFAIL -> temperror +# A multiple spf-RR return should get permerror +# - and not crash with non-spf txt records +# v=spf1 is casr-insensitive +exim -be +none ${lookup {fred@v6.test.ex} spf {HOSTIPV4}} +temperror ${lookup {fred@test.again.dns} spf {HOSTIPV4}} +permerror ${lookup {fred@double.example.com} spf {8.8.8.8}} +permerror ${lookup {fred@doubleplus.example.com} spf {8.8.8.8}} +pass ${lookup {fred@uppercase.example.com} spf {HOSTIPV4}} +**** diff --git a/test/stdout/4601 b/test/stdout/4601 index 66746a470..cbb4cf502 100644 --- a/test/stdout/4601 +++ b/test/stdout/4601 @@ -26,3 +26,9 @@ Connecting to 127.0.0.1 port 1224 ... connected <<< 250 Accepted >>> quit End of script +> none none +> temperror temperror +> permerror permerror +> permerror permerror +> pass pass +>