Skip to content

Commit

Permalink
Several fixes related to the internal SPF implementation:
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
Murray S. Kucherawy committed Apr 15, 2021
1 parent 9da59ec commit 5ed1d6e
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 20 deletions.
2 changes: 2 additions & 0 deletions RELEASE_NOTES
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion libopendmarc/opendmarc_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 20 additions & 7 deletions libopendmarc/opendmarc_spf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
38 changes: 29 additions & 9 deletions libopendmarc/opendmarc_spf_dns.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion libopendmarc/tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion libopendmarc/tests/test_spf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 5ed1d6e

Please sign in to comment.