Michael Deutschmann
2013-09-07 00:30:09 UTC
Some serious bugs in libspf2's support for "ip6" mechanisms have come to
my attention. It was a little tricky to diagnose because the three bugs
interact in ways that make the problems hard to see.
It's relevant to discuss this here rather than some libspf2-specific
forum, because it means that SPF publishers may want to avoid using the
ip6 mechanism to avoid spurious fails.
The bugs are in the following function from "src/libspf2/spf_compile.c",
below:
static SPF_errcode_t
SPF_c_parse_ip6([...])
{
[...]
char buf[ INET_ADDRSTRLEN ];
[...
at this point start and end are char pointers
bracketing the IPv6 address, exclusive of any CIDR size
]
len = end - start;
if ( len > sizeof( buf ) - 1 )
return SPF_E_INVALID_IP6;
memcpy( buf, start, len );
buf[ len ] = '\0';
addr = SPF_mech_ip6_data(mech);
err = inet_pton( AF_INET6, buf, addr );
if ( err <= 0 )
return SPF_response_add_error_ptr(spf_response,
SPF_E_INVALID_IP6, NULL, buf, NULL);
return SPF_E_SUCCESS;
}
The errors are as follows:
1. "buf" is declared as an array of size INET_ADDRSTRLEN, which is
enough space to write any IPv4 address in ASCII (16 bytes including the
null terminator). That is way too small for an IPv6 address.
INET6_ADDRSTRLEN holds the correct value. Alone, this bug would cause
frequent permerrors whenever ip6 mechanisms are used.
2. In the case that the candidate address is too long (which will happen
often for valid addresses due to bug #1), the code does a "return
SPF_E_INVALID_IP6". But this doesn't actually cause an error to be
returned to the client -- it's the SPF_response_add_error_ptr() that
really does it. Instead of causing a permerror, these ip6 records are
silently ignored. Since bug #1 causes this to happen to valid ip6
mechanisms, this will cause spurious fails for IPv6 traffic.
3. Finally, the code assumes that inet_pton always actually supports
AF_INET6. I use uClibc, and if IPv6 is not explictly compiled into it,
it will provide an inet_pton() function that only works for
inet_pton(AF_INET,x,x), otherwise returning EAFNOSUPPORT. Alone this
bug would cause permerrors whenever a putative sender uses ip6 records,
but the other two bugs work together to make it invisible most of the
time.
I noticed the problem when a spam came in, putatively from gmail, and
had a permerror status. Using spfquery I determined that libspf2 was
barfing on the mechanism "ip6:2800:3f0:4000::/36". Some experimentation
determined that "ip6:2800:3f00:4000::/36" would have been accepted.
That falsely led me to think of a problem with inet_pton() being too
strict, so you can imagine my confusion when I found uClibc's inet_pton
never works and there was no sign of replacement code in libspf2.
What was really happening was that "2800:03f0:4000::" is 17 bytes and
thus seemed to pass due to the combined action of bugs #1 and #2.
"2800:3f0:4000::" is 16 bytes, so it avoided bug #1 and hit bug #3.
---- Michael Deutschmann <***@talamasca.ocis.net>
my attention. It was a little tricky to diagnose because the three bugs
interact in ways that make the problems hard to see.
It's relevant to discuss this here rather than some libspf2-specific
forum, because it means that SPF publishers may want to avoid using the
ip6 mechanism to avoid spurious fails.
The bugs are in the following function from "src/libspf2/spf_compile.c",
below:
static SPF_errcode_t
SPF_c_parse_ip6([...])
{
[...]
char buf[ INET_ADDRSTRLEN ];
[...
at this point start and end are char pointers
bracketing the IPv6 address, exclusive of any CIDR size
]
len = end - start;
if ( len > sizeof( buf ) - 1 )
return SPF_E_INVALID_IP6;
memcpy( buf, start, len );
buf[ len ] = '\0';
addr = SPF_mech_ip6_data(mech);
err = inet_pton( AF_INET6, buf, addr );
if ( err <= 0 )
return SPF_response_add_error_ptr(spf_response,
SPF_E_INVALID_IP6, NULL, buf, NULL);
return SPF_E_SUCCESS;
}
The errors are as follows:
1. "buf" is declared as an array of size INET_ADDRSTRLEN, which is
enough space to write any IPv4 address in ASCII (16 bytes including the
null terminator). That is way too small for an IPv6 address.
INET6_ADDRSTRLEN holds the correct value. Alone, this bug would cause
frequent permerrors whenever ip6 mechanisms are used.
2. In the case that the candidate address is too long (which will happen
often for valid addresses due to bug #1), the code does a "return
SPF_E_INVALID_IP6". But this doesn't actually cause an error to be
returned to the client -- it's the SPF_response_add_error_ptr() that
really does it. Instead of causing a permerror, these ip6 records are
silently ignored. Since bug #1 causes this to happen to valid ip6
mechanisms, this will cause spurious fails for IPv6 traffic.
3. Finally, the code assumes that inet_pton always actually supports
AF_INET6. I use uClibc, and if IPv6 is not explictly compiled into it,
it will provide an inet_pton() function that only works for
inet_pton(AF_INET,x,x), otherwise returning EAFNOSUPPORT. Alone this
bug would cause permerrors whenever a putative sender uses ip6 records,
but the other two bugs work together to make it invisible most of the
time.
I noticed the problem when a spam came in, putatively from gmail, and
had a permerror status. Using spfquery I determined that libspf2 was
barfing on the mechanism "ip6:2800:3f0:4000::/36". Some experimentation
determined that "ip6:2800:3f00:4000::/36" would have been accepted.
That falsely led me to think of a problem with inet_pton() being too
strict, so you can imagine my confusion when I found uClibc's inet_pton
never works and there was no sign of replacement code in libspf2.
What was really happening was that "2800:03f0:4000::" is 17 bytes and
thus seemed to pass due to the combined action of bugs #1 and #2.
"2800:3f0:4000::" is 16 bytes, so it avoided bug #1 and hit bug #3.
---- Michael Deutschmann <***@talamasca.ocis.net>