Commit df172901 authored by Mark Andrews's avatar Mark Andrews
Browse files

4468. [bug] Address ECS option handling issues. [RT #43191]

parent b6f65b09
4468. [bug] Address ECS option handling issues. [RT #43191]
4467. [security] It was possible to trigger a assertion when rendering
a message. (CVE-2016-2776) [RT #43139]
......
......@@ -1020,7 +1020,7 @@
<para>
<command>dig +subnet=0.0.0.0/0</command>, or simply
<command>dig +subnet=0</command> for short, sends an EDNS
client-subnet option with an empty address and a source
CLIENT-SUBNET option with an empty address and a source
prefix-length of zero, which signals a resolver that
the client's address information must
<emphasis>not</emphasis> be used when resolving
......
......@@ -1066,11 +1066,24 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) {
isc_uint32_t prefix_length = 0xffffffff;
char *slash = NULL;
isc_boolean_t parsed = ISC_FALSE;
isc_boolean_t prefix_parsed = ISC_FALSE;
char buf[sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:XXX.XXX.XXX.XXX/128")];
if (strlcpy(buf, value, sizeof(buf)) >= sizeof(buf))
fatal("invalid prefix '%s'\n", value);
sa = isc_mem_allocate(mctx, sizeof(*sa));
if (sa == NULL)
fatal("out of memory");
memset(sa, 0, sizeof(*sa));
if (strcmp(buf, "0") == 0) {
sa->type.sa.sa_family = AF_UNSPEC;
parsed = ISC_TRUE;
prefix_length = 0;
goto done;
}
slash = strchr(buf, '/');
if (slash != NULL) {
*slash = '\0';
......@@ -1079,16 +1092,9 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) {
fatal("invalid prefix length in '%s': %s\n",
value, isc_result_totext(result));
}
prefix_parsed = ISC_TRUE;
}
if (strcmp(buf, "0") == 0) {
parsed = ISC_TRUE;
prefix_length = 0;
}
sa = isc_mem_allocate(mctx, sizeof(*sa));
if (sa == NULL)
fatal("out of memory");
if (inet_pton(AF_INET6, buf, &in6) == 1) {
parsed = ISC_TRUE;
isc_sockaddr_fromin6(sa, &in6, 0);
......@@ -1099,7 +1105,7 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) {
isc_sockaddr_fromin(sa, &in4, 0);
if (prefix_length > 32)
prefix_length = 32;
} else if (prefix_length != 0xffffffff && prefix_length != 0) {
} else if (prefix_parsed) {
int i;
for (i = 0; i < 3 && strlen(buf) < sizeof(buf) - 2; i++) {
......@@ -1118,10 +1124,8 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) {
if (!parsed)
fatal("invalid address '%s'", value);
done:
sa->length = prefix_length;
if (prefix_length == 0)
sa->type.sa.sa_family = AF_UNSPEC;
*sap = sa;
return (ISC_R_SUCCESS);
......@@ -2510,8 +2514,8 @@ setup_lookup(dig_lookup_t *lookup) {
}
if (lookup->ecs_addr != NULL) {
isc_uint8_t addr[16], family;
int proto;
isc_uint8_t addr[16];
isc_uint16_t family;
isc_uint32_t plen;
struct sockaddr *sa;
struct sockaddr_in *sin;
......@@ -2528,46 +2532,65 @@ setup_lookup(dig_lookup_t *lookup) {
opts[i].code = DNS_OPT_CLIENT_SUBNET;
opts[i].length = (isc_uint16_t) addrl + 4;
check_result(result, "isc_buffer_allocate");
isc_buffer_init(&b, ecsbuf, sizeof(ecsbuf));
/* If prefix length is zero, don't set family */
proto = sa->sa_family;
if (plen == 0)
proto = AF_UNSPEC;
switch (proto) {
/*
* XXXMUKS: According to RFC7871, "If there is
* no ADDRESS set, i.e., SOURCE PREFIX-LENGTH is
* set to 0, then FAMILY SHOULD be set to the
* transport over which the query is sent."
*
* However, at this point we don't know what
* transport(s) we'll be using, so we can't
* set the value now. For now, we're using
* IPv4 as the default the +subnet option
* used an IPv4 prefix, or for +subnet=0,
* and IPv6 if the +subnet option used an
* IPv6 prefix.
*
* (For future work: preserve the offset into
* the buffer where the family field is;
* that way we can update it in send_udp()
* or send_tcp_connect() once we know
* what it outght to be.)
*/
switch (sa->sa_family) {
case AF_UNSPEC:
INSIST(plen == 0);
family = 0;
family = 1;
break;
case AF_INET:
INSIST(plen <= 32);
family = 1;
sin = (struct sockaddr_in *) sa;
memmove(addr, &sin->sin_addr, 4);
memmove(addr, &sin->sin_addr, addrl);
break;
case AF_INET6:
INSIST(plen <= 128);
family = 2;
sin6 = (struct sockaddr_in6 *) sa;
memmove(addr, &sin6->sin6_addr, 16);
memmove(addr, &sin6->sin6_addr, addrl);
break;
default:
INSIST(0);
}
/* Mask off last address byte */
if (addrl > 0 && (plen % 8) != 0)
addr[addrl - 1] &= ~0U << (8 - (plen % 8));
isc_buffer_init(&b, ecsbuf, sizeof(ecsbuf));
/* family */
isc_buffer_putuint16(&b, family);
/* source prefix-length */
isc_buffer_putuint8(&b, plen);
/* scope prefix-length */
isc_buffer_putuint8(&b, 0);
/* address */
if (addrl > 0)
if (addrl > 0) {
/* Mask off last address byte */
if ((plen % 8) != 0)
addr[addrl - 1] &=
~0U << (8 - (plen % 8));
isc_buffer_putmem(&b, addr,
(unsigned)addrl);
}
opts[i].value = (isc_uint8_t *) ecsbuf;
i++;
......
......@@ -1614,35 +1614,57 @@ ns_client_addopt(ns_client_t *client, dns_message_t *message,
client->ecs_addr.family == AF_INET6 ||
client->ecs_addr.family == AF_UNSPEC))
{
int i, addrbytes = (client->ecs_addrlen + 7) / 8;
isc_uint8_t *paddr;
isc_buffer_t buf;
isc_uint8_t addr[16];
isc_uint32_t plen, addrl;
isc_uint16_t family;
/* Add CLIENT-SUBNET option. */
plen = client->ecs_addrlen;
/* Round up prefix len to a multiple of 8 */
addrl = (plen + 7) / 8;
switch (client->ecs_addr.family) {
case AF_UNSPEC:
INSIST(plen == 0);
family = 0;
break;
case AF_INET:
INSIST(plen <= 32);
family = 1;
memmove(addr, &client->ecs_addr.type, addrl);
break;
case AF_INET6:
INSIST(plen <= 128);
family = 2;
memmove(addr, &client->ecs_addr.type, addrl);
break;
default:
INSIST(0);
}
/* Add client subnet option. */
isc_buffer_init(&buf, ecs, sizeof(ecs));
if (client->ecs_addr.family == AF_UNSPEC ||
client->ecs_addrlen == 0)
isc_buffer_putuint16(&buf, 0);
else if (client->ecs_addr.family == AF_INET)
isc_buffer_putuint16(&buf, 1);
else
isc_buffer_putuint16(&buf, 2);
/* family */
isc_buffer_putuint16(&buf, family);
/* source prefix-length */
isc_buffer_putuint8(&buf, client->ecs_addrlen);
/* scope prefix-length */
isc_buffer_putuint8(&buf, client->ecs_scope);
paddr = (isc_uint8_t *) &client->ecs_addr.type;
for (i = 0; i < addrbytes; i++) {
unsigned char uc;
uc = paddr[i];
if (i == addrbytes - 1 &&
((client->ecs_addrlen % 8) != 0))
uc &= (0xffU << (8 -
(client->ecs_addrlen % 8)));
isc_buffer_putuint8(&buf, uc);
/* address */
if (addrl > 0) {
/* Mask off last address byte */
if ((plen % 8) != 0)
addr[addrl - 1] &=
~0U << (8 - (plen % 8));
isc_buffer_putmem(&buf, addr,
(unsigned) addrl);
}
ednsopts[count].code = DNS_OPT_CLIENT_SUBNET;
ednsopts[count].length = addrbytes + 4;
ednsopts[count].length = addrl + 4;
ednsopts[count].value = ecs;
count++;
}
......@@ -1975,14 +1997,6 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) {
return (DNS_R_OPTERR);
}
if (addrlen == 0U && family != 0U) {
ns_client_log(client, NS_LOGCATEGORY_CLIENT,
NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
"EDNS client-subnet option: "
"source == 0 but family != 0");
return (DNS_R_OPTERR);
}
memset(&caddr, 0, sizeof(caddr));
switch (family) {
case 0:
......
......@@ -281,7 +281,9 @@ if [ -x ${DIG} ] ; then
echo "I:checking dig +subnet=0/0 ($n)"
ret=0
$DIG $DIGOPTS +tcp @10.53.0.2 +subnet=0/0 A a.example > dig.out.test$n 2>&1 || ret=1
grep "CLIENT-SUBNET: 0/0/0" < dig.out.test$n > /dev/null || ret=1
grep "status: NOERROR" < dig.out.test$n > /dev/null || ret=1
grep "CLIENT-SUBNET: 0.0.0.0/0/0" < dig.out.test$n > /dev/null || ret=1
grep "10.0.0.1" < dig.out.test$n > /dev/null || ret=1
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
......@@ -289,7 +291,9 @@ if [ -x ${DIG} ] ; then
echo "I:checking dig +subnet=0 ($n)"
ret=0
$DIG $DIGOPTS +tcp @10.53.0.2 +subnet=0 A a.example > dig.out.test$n 2>&1 || ret=1
grep "CLIENT-SUBNET: 0/0/0" < dig.out.test$n > /dev/null || ret=1
grep "status: NOERROR" < dig.out.test$n > /dev/null || ret=1
grep "CLIENT-SUBNET: 0.0.0.0/0/0" < dig.out.test$n > /dev/null || ret=1
grep "10.0.0.1" < dig.out.test$n > /dev/null || ret=1
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
......@@ -297,7 +301,30 @@ if [ -x ${DIG} ] ; then
echo "I:checking dig +subnet=::/0 ($n)"
ret=0
$DIG $DIGOPTS +tcp @10.53.0.2 +subnet=::/0 A a.example > dig.out.test$n 2>&1 || ret=1
grep "status: NOERROR" < dig.out.test$n > /dev/null || ret=1
grep "CLIENT-SUBNET: ::/0/0" < dig.out.test$n > /dev/null || ret=1
grep "10.0.0.1" < dig.out.test$n > /dev/null || ret=1
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
n=`expr $n + 1`
echo "I:checking dig +ednsopt=8:00000000 (family=0, source=0, scope=0) ($n)"
ret=0
$DIG $DIGOPTS +tcp @10.53.0.2 +ednsopt=8:00000000 A a.example > dig.out.test$n 2>&1 || ret=1
grep "status: NOERROR" < dig.out.test$n > /dev/null || ret=1
grep "CLIENT-SUBNET: 0/0/0" < dig.out.test$n > /dev/null || ret=1
grep "10.0.0.1" < dig.out.test$n > /dev/null || ret=1
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
n=`expr $n + 1`
echo "I:checking dig +ednsopt=8:00030000 (family=3, source=0, scope=0) ($n)"
ret=0
$DIG $DIGOPTS +qr +tcp @10.53.0.2 +ednsopt=8:00030000 A a.example > dig.out.test$n 2>&1 || ret=1
grep "status: FORMERR" < dig.out.test$n > /dev/null || ret=1
grep "CLIENT-SUBNET: 00 03 00 00" < dig.out.test$n > /dev/null || ret=1
lines=`grep "CLIENT-SUBNET: 00 03 00 00" dig.out.test$n | wc -l`
[ ${lines:-0} -eq 1 ] || ret=1
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
......
......@@ -3255,9 +3255,6 @@ render_ecs(isc_buffer_t *ecsbuf, isc_buffer_t *target) {
addrlen = isc_buffer_getuint8(ecsbuf);
scopelen = isc_buffer_getuint8(ecsbuf);
if (addrlen == 0 && family != 0)
return (DNS_R_OPTERR);
addrbytes = (addrlen + 7) / 8;
if (isc_buffer_remaininglength(ecsbuf) < addrbytes)
return (DNS_R_OPTERR);
......@@ -3265,7 +3262,6 @@ render_ecs(isc_buffer_t *ecsbuf, isc_buffer_t *target) {
if (addrbytes > sizeof(addr))
return (DNS_R_OPTERR);
ADD_STRING(target, ": ");
memset(addr, 0, sizeof(addr));
for (i = 0; i < addrbytes; i ++)
addr[i] = isc_buffer_getuint8(ecsbuf);
......@@ -3287,12 +3283,10 @@ render_ecs(isc_buffer_t *ecsbuf, isc_buffer_t *target) {
inet_ntop(AF_INET6, addr, addr_text, sizeof(addr_text));
break;
default:
snprintf(addr_text, sizeof(addr_text),
"Unsupported family %u", family);
ADD_STRING(target, addr_text);
return (ISC_R_SUCCESS);
return (DNS_R_OPTERR);
}
ADD_STRING(target, ": ");
ADD_STRING(target, addr_text);
snprintf(addr_text, sizeof(addr_text), "/%d/%d", addrlen, scopelen);
ADD_STRING(target, addr_text);
......
......@@ -125,9 +125,6 @@ fromwire_opt(ARGS_FROMWIRE) {
scope = uint8_fromregion(&sregion);
isc_region_consume(&sregion, 1);
if (addrlen == 0U && family != 0U)
return (DNS_R_OPTERR);
switch (family) {
case 0:
/*
......
......@@ -97,7 +97,7 @@ ATF_TC_BODY(edns_client_subnet, tc) {
0x00, 0x08, 0x00, 0x04,
0x00, 0x01, 0x00, 0x00
},
8, ISC_FALSE
8, ISC_TRUE
},
{
/* Option code family 2 (ipv6) , source 0, scope 0 */
......@@ -105,7 +105,7 @@ ATF_TC_BODY(edns_client_subnet, tc) {
0x00, 0x08, 0x00, 0x04,
0x00, 0x02, 0x00, 0x00
},
8, ISC_FALSE
8, ISC_TRUE
},
{
/* extra octet */
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment