From 5ed1d6e53759514fa5af277655725fa650f6fcf3 Mon Sep 17 00:00:00 2001 From: "Murray S. Kucherawy" Date: Wed, 14 Apr 2021 18:55:44 -0700 Subject: [PATCH] Several fixes related to the internal SPF implementation: * Build the SPF test suite if either the internal implementation or libspf2 is selected. Previously the tests only ran if libspf2 was selected. In fact the only way to test the internal implementation is to request libspf2 but give it a bogus location; the package falls back to the internal SPF implementation, but still schedules the tests. * In several places, IPv4 addresses were being passed around as u_long instead of uint32_t. On at least some environments, u_long is eight bytes long, which leads to false comparisons. * When doing relative comparisons on IPv4 addresses, be sure everything is in host byte order. * Re-fix the type check in the A/AAAA retrieval code. * Don't throw away replies to the A query when T_AAAA is defined. * Parse T_AAAA replies properly. * Document what the CIDR function expects in terms of byte order. --- RELEASE_NOTES | 2 ++ configure.ac | 3 ++- libopendmarc/opendmarc_internal.h | 2 +- libopendmarc/opendmarc_spf.c | 27 ++++++++++++++++------ libopendmarc/opendmarc_spf_dns.c | 38 +++++++++++++++++++++++-------- libopendmarc/tests/Makefile.am | 2 +- libopendmarc/tests/test_spf.c | 1 - 7 files changed, 55 insertions(+), 20 deletions(-) diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 65861680..9bdd4a51 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -50,6 +50,8 @@ release, and a summary of the changes in that release. htonl() since it's already in network byte order. This was causing SPF errors when the internal SPF implementation was in use. + LIBOPENDMARC: Fix numerous problems with the internal SPF + implementation. 1.4.0 2021/01/28 Add ARC support. Extensive work contributed by ValiMail, with patches diff --git a/configure.ac b/configure.ac index c13c7d4e..5c7b629e 100644 --- a/configure.ac +++ b/configure.ac @@ -325,6 +325,7 @@ AC_ARG_WITH([spf2-lib], AS_HELP_STRING([--with-spf2-lib], [path to libspf2 libraries]), SPF2_LIB="$withval", []) + use_spf="no" if test "x$SPF2_INCLUDE" != "x" -a "x$SPF2_LIB" != "x" then @@ -343,7 +344,7 @@ then AC_SEARCH_LIBS(SPF_server_new, spf2) fi -AM_CONDITIONAL([USE_SPF], [test x"$use_spf" = x"yes"]) +AM_CONDITIONAL([TEST_SPF], [test x"$with_spf" = x"yes"]) # # opendmarc diff --git a/libopendmarc/opendmarc_internal.h b/libopendmarc/opendmarc_internal.h index c1bced79..c48d6d3a 100644 --- a/libopendmarc/opendmarc_internal.h +++ b/libopendmarc/opendmarc_internal.h @@ -291,7 +291,7 @@ char * opendmarc_spf_dns_get_record(char *domain, int *reply, char *txt, size_ int opendmarc_spf_dns_does_domain_exist(char *domain, int *reply); char * opendmarc_spf_dns_get_record(char *domain, int *reply, char *txt, size_t txtlen, char *cname, size_t cnamelen, int spfcheck); int opendmarc_spf_ipv6_cidr_check(char *ipv6_str, char *cidr_string); -int opendmarc_spf_cidr_address(u_long ip, char *cidr_addr); +int opendmarc_spf_cidr_address(uint32_t ip, char *cidr_addr); SPF_CTX_T * opendmarc_spf_alloc_ctx(); SPF_CTX_T * opendmarc_spf_free_ctx(SPF_CTX_T *spfctx); int opendmarc_spf_status_to_pass(int status, int none_pass); diff --git a/libopendmarc/opendmarc_spf.c b/libopendmarc/opendmarc_spf.c index 88f5bc0c..41fdce50 100644 --- a/libopendmarc/opendmarc_spf.c +++ b/libopendmarc/opendmarc_spf.c @@ -430,16 +430,28 @@ opendmarc_spf_status_to_pass(int status, int none_pass) return r; } +/* +** OPENDMARC_SPF_CIDR_ADDRESS -- see if an IP address is covered by a CIDR +** expression +** +** Parameters: +** ip -- IP address to test, in network byte order +** cidr_addr -- CIDR expression to which to compare it +** +** Return value: +** TRUE iff "ip" is inside (or equal to) "cidr_addr". +*/ + int -opendmarc_spf_cidr_address(u_long ip, char *cidr_addr) +opendmarc_spf_cidr_address(uint32_t ip, char *cidr_addr) { char *cidr; char *cp, *ep; char buf[BUFSIZ]; - u_long i; - u_long bits; - u_long mask; - u_long high, low; + uint32_t i; + uint32_t bits; + uint32_t mask; + uint32_t high, low; struct sockaddr_in sin; if (cidr_addr == NULL) @@ -453,8 +465,8 @@ opendmarc_spf_cidr_address(u_long ip, char *cidr_addr) { if (inet_aton(cidr_addr, &sin.sin_addr) != 0) { - (void)memcpy(&low, &sin.sin_addr, sizeof(sin.sin_addr)); - (void)memcpy(&high, &sin.sin_addr, sizeof(sin.sin_addr)); + (void)memcpy(&low, &sin.sin_addr.s_addr, sizeof(sin.sin_addr.s_addr)); + (void)memcpy(&high, &sin.sin_addr.s_addr, sizeof(sin.sin_addr.s_addr)); if (ip >= low && ip <= high) return TRUE; } @@ -492,6 +504,7 @@ opendmarc_spf_cidr_address(u_long ip, char *cidr_addr) low = i & mask; high = i | (~mask & 0xFFFFFFFF); + ip = ntohl(ip); if (ip >= low && ip <= high) return TRUE; return FALSE; diff --git a/libopendmarc/opendmarc_spf_dns.c b/libopendmarc/opendmarc_spf_dns.c index 805f1681..f5a80a0e 100644 --- a/libopendmarc/opendmarc_spf_dns.c +++ b/libopendmarc/opendmarc_spf_dns.c @@ -173,19 +173,39 @@ opendmarc_spf_dns_lookup_a_actual(char *domain, int sought, char **ary, int *cnt cp += k; continue; } - else if (type != T_TXT) + else if (type != sought) { - /* not a TXT; walk past the whole thing and continue */ + /* not a type we want; skip the rest and continue */ cp += l; continue; } + else if (type == T_A) + { + GETLONG(a, cp); + a = htonl(a); + (void) memcpy(&in.s_addr, &a, sizeof(uint32_t)); + (void) memset(hbuf, '\0', sizeof hbuf); + (void) strncpy(hbuf, inet_ntoa(in), sizeof hbuf); + ary = opendmarc_util_pushnargv(hbuf, ary, cnt); + } +#ifdef T_AAAA + else if (type == T_AAAA) + { + struct in6_addr s6; - GETLONG(a, cp); - (void) memcpy(&in.s_addr, &a, sizeof(uint32_t)); - in.s_addr = ntohl(in.s_addr); - (void) memset(hbuf, '\0', sizeof hbuf); - (void) strncpy(hbuf, inet_ntoa(in), sizeof hbuf); - ary = opendmarc_util_pushnargv(hbuf, ary, cnt); + /* just to be sure... */ + if (l != sizeof s6.s6_addr) + { + cp += l; + continue; + } + + (void) memcpy(&s6.s6_addr, cp, sizeof s6.s6_addr); + (void) memset(hbuf, '\0', sizeof hbuf); + inet_ntop(AF_INET6, &s6.s6_addr, hbuf, sizeof hbuf - 1); + ary = opendmarc_util_pushnargv(hbuf, ary, cnt); + } +#endif /* T_AAAA */ } return ary; } @@ -211,7 +231,7 @@ opendmarc_spf_dns_lookup_a(char *domain, char **ary, int *cnt) retp = opendmarc_spf_dns_lookup_a_actual(domain, T_A, ary, cnt); #ifdef T_AAAA - retp = opendmarc_spf_dns_lookup_a_actual(domain, T_AAAA, ary, cnt); + retp = opendmarc_spf_dns_lookup_a_actual(domain, T_AAAA, retp, cnt); #endif /* T_AAAA */ return retp; } diff --git a/libopendmarc/tests/Makefile.am b/libopendmarc/tests/Makefile.am index d8f63e0c..b8b01bc0 100644 --- a/libopendmarc/tests/Makefile.am +++ b/libopendmarc/tests/Makefile.am @@ -12,7 +12,7 @@ check_PROGRAMS = test_tld \ if LIVE_TESTS #check_PROGRAMS += test_dns_lookup #test_dns_lookup_SOURCES = test_dns_lookup.c -if USE_SPF +if TEST_SPF check_PROGRAMS += test_spf test_spf_SOURCES = test_spf.c endif diff --git a/libopendmarc/tests/test_spf.c b/libopendmarc/tests/test_spf.c index 70be9929..a02f9af7 100644 --- a/libopendmarc/tests/test_spf.c +++ b/libopendmarc/tests/test_spf.c @@ -206,7 +206,6 @@ opendmarc_spf_ip4_tests(void) for (ipp = ip4list; ipp->connect_ip != NULL; ++ipp) { ip = inet_addr(ipp->connect_ip); - ip = htonl(ip); if (opendmarc_spf_cidr_address(ip, ipp->record_ip) != ipp->outcome) { printf("Error: %s compared to %s failed\n", ipp->connect_ip, ipp->record_ip);