Commit 3e33f419 authored by Mark Andrews's avatar Mark Andrews

4154. [bug] A OPT record should be included with the FORMERR

                        response when there is a malformed EDNS option.
                        [RT #39647]

4153.   [bug]           Dig should zero non significant +subnet bits.  Check
                        that non significant ECS bits are zero on receipt.
                        [RT #39647]
parent 1879ff49
4154. [bug] A OPT record should be included with the FORMERR
response when there is a malformed EDNS option.
[RT #39647]
4153. [bug] Dig should zero non significant +subnet bits. Check
that non significant ECS bits are zero on receipt.
[RT #39647]
4152. [func] Implement DNS COOKIE option. This replaces the
experimental SIT option of BIND 9.10. The following
named.conf directives are avaliable: send-cookie,
......
......@@ -2491,13 +2491,19 @@ setup_lookup(dig_lookup_t *lookup) {
struct sockaddr *sa;
struct sockaddr_in *sin;
struct sockaddr_in6 *sin6;
const isc_uint8_t *addr;
size_t addrl;
isc_uint8_t mask;
sa = &lookup->ecs_addr->type.sa;
prefixlen = lookup->ecs_addr->length;
/* Round up prefix len to a multiple of 8 */
addrl = (prefixlen + 7) / 8;
if (prefixlen % 8 == 0)
mask = 0xff;
else
mask = 0xffU << (8 - (prefixlen % 8));
INSIST(i < DNS_EDNSOPTIONS);
opts[i].code = DNS_OPT_CLIENT_SUBNET;
......@@ -2506,20 +2512,36 @@ setup_lookup(dig_lookup_t *lookup) {
isc_buffer_init(&b, ecsbuf, sizeof(ecsbuf));
if (sa->sa_family == AF_INET) {
sin = (struct sockaddr_in *) sa;
addr = (isc_uint8_t *) &sin->sin_addr;
/* family */
isc_buffer_putuint16(&b, 1);
/* source prefix-length */
isc_buffer_putuint8(&b, prefixlen);
/* scope prefix-length */
isc_buffer_putuint8(&b, 0);
isc_buffer_putmem(&b,
(isc_uint8_t *) &sin->sin_addr,
(unsigned int) addrl);
/* address */
if (addrl > 0) {
isc_buffer_putmem(&b, addr, addrl - 1);
isc_buffer_putuint8(&b,
(addr[addrl - 1] &
mask));
}
} else {
sin6 = (struct sockaddr_in6 *) sa;
addr = (isc_uint8_t *) &sin6->sin6_addr;
/* family */
isc_buffer_putuint16(&b, 2);
/* source prefix-length */
isc_buffer_putuint8(&b, prefixlen);
/* scope prefix-length */
isc_buffer_putuint8(&b, 0);
isc_buffer_putmem(&b,
(isc_uint8_t *) &sin6->sin6_addr,
(unsigned int) addrl);
/* address */
if (addrl > 0) {
isc_buffer_putmem(&b, addr, addrl - 1);
isc_buffer_putuint8(&b,
(addr[addrl - 1] &
mask));
}
}
opts[i].value = (isc_uint8_t *) ecsbuf;
......
......@@ -1514,7 +1514,7 @@ ns_client_addopt(ns_client_t *client, dns_message_t *message,
uc = paddr[i];
if (i == addrbytes - 1 &&
((client->ecs_addrlen % 8) != 0))
uc &= (1U << (8 - (client->ecs_addrlen % 8)));
uc &= (0xffU << (8 - (client->ecs_addrlen % 8)));
isc_buffer_putuint8(&buf, uc);
}
......@@ -1818,10 +1818,16 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) {
isc_netaddr_t caddr;
int i;
/*
* XXXMUKS: Is there any need to repeat these checks here
* (except query's scope length) when they are done in the OPT
* RDATA fromwire code?
*/
if (optlen < 4U) {
ns_client_log(client, NS_LOGCATEGORY_CLIENT,
NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
"EDNS client subnet option too short");
"EDNS client-subnet option too short");
return (DNS_R_FORMERR);
}
......@@ -1833,8 +1839,8 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) {
if (scope != 0U) {
ns_client_log(client, NS_LOGCATEGORY_CLIENT,
NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
"EDNS client subnet option: invalid scope");
return (DNS_R_FORMERR);
"EDNS client-subnet option: invalid scope");
return (DNS_R_OPTERR);
}
memset(&caddr, 0, sizeof(caddr));
......@@ -1849,26 +1855,26 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) {
invalid_length:
ns_client_log(client, NS_LOGCATEGORY_CLIENT,
NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
"EDNS client subnet option: invalid "
"EDNS client-subnet option: invalid "
"address length (%u) for %s",
addrlen, family == 1 ? "IPv4" : "IPv6");
return (DNS_R_FORMERR);
return (DNS_R_OPTERR);
}
caddr.family = AF_INET6;
break;
default:
ns_client_log(client, NS_LOGCATEGORY_CLIENT,
NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
"EDNS client subnet option: invalid family");
return (DNS_R_FORMERR);
"EDNS client-subnet option: invalid family");
return (DNS_R_OPTERR);
}
addrbytes = (addrlen + 7) / 8;
if (isc_buffer_remaininglength(buf) < addrbytes) {
ns_client_log(client, NS_LOGCATEGORY_CLIENT,
NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
"EDNS client subnet option: address too short");
return (DNS_R_FORMERR);
"EDNS client-subnet option: address too short");
return (DNS_R_OPTERR);
}
paddr = (isc_uint8_t *) &caddr.type;
......@@ -1877,6 +1883,13 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) {
optlen--;
}
if (addrbytes != 0U && (addrlen % 8) != 0) {
isc_uint8_t bits = ~0 << (8 - (addrlen % 8));
bits &= paddr[addrbytes - 1];
if (bits != paddr[addrbytes - 1])
return (DNS_R_OPTERR);
}
memmove(&client->ecs_addr, &caddr, sizeof(caddr));
client->ecs_addrlen = addrlen;
client->ecs_scope = 0;
......
......@@ -544,5 +544,24 @@ grep "status: NOTIMP" dig.out.ns5.test${n} > /dev/null || ret=1
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
n=`expr $n + 1`
echo "I:check that EDNS client subnet with non-zeroed bits is handled correctly (${n})"
ret=0
# 0001 (IPv4) 1f (31 significant bits) 00 (0) ffffffff (255.255.255.255)
$DIG soa . @10.53.0.5 -p 5300 +ednsopt=8:00011f00ffffffff > dig.out.ns5.test${n} || ret=1
grep "status: FORMERR" dig.out.ns5.test${n} > /dev/null || ret=1
grep "; EDNS: version:" dig.out.ns5.test${n} > /dev/null || ret=1
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
n=`expr $n + 1`
echo "I:check that dig +subnet zeros address bits correctly (${n})"
ret=0
$DIG soa . @10.53.0.5 -p 5300 +subnet=255.255.255.255/23 > dig.out.ns5.test${n} || ret=1
grep "status: NOERROR" dig.out.ns5.test${n} > /dev/null || ret=1
grep "CLIENT-SUBNET: 255.255.254.0/23/0" dig.out.ns5.test${n} > /dev/null || ret=1
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
echo "I:exit status: $status"
exit $status
......@@ -588,7 +588,9 @@ sendquery(struct query *query, isc_task_t *task)
struct sockaddr *sa;
struct sockaddr_in *sin;
struct sockaddr_in6 *sin6;
const isc_uint8_t *addr;
size_t addrl;
isc_uint8_t mask;
isc_buffer_t b;
sa = &query->ecs_addr->type.sa;
......@@ -596,6 +598,10 @@ sendquery(struct query *query, isc_task_t *task)
/* Round up prefix len to a multiple of 8 */
addrl = (prefixlen + 7) / 8;
if (prefixlen % 8 == 0)
mask = 0xff;
else
mask = 0xffU << (8 - (prefixlen % 8));
INSIST(i < DNS_EDNSOPTIONS);
opts[i].code = DNS_OPT_CLIENT_SUBNET;
......@@ -604,20 +610,36 @@ sendquery(struct query *query, isc_task_t *task)
isc_buffer_init(&b, ecsbuf, sizeof(ecsbuf));
if (sa->sa_family == AF_INET) {
sin = (struct sockaddr_in *) sa;
addr = (isc_uint8_t *) &sin->sin_addr;
/* family */
isc_buffer_putuint16(&b, 1);
/* source prefix-length */
isc_buffer_putuint8(&b, prefixlen);
/* scope prefix-length */
isc_buffer_putuint8(&b, 0);
isc_buffer_putmem(&b,
(isc_uint8_t *) &sin->sin_addr,
(unsigned int) addrl);
/* address */
if (addrl > 0) {
isc_buffer_putmem(&b, addr, addrl - 1);
isc_buffer_putuint8(&b,
(addr[addrl - 1] &
mask));
}
} else {
sin6 = (struct sockaddr_in6 *) sa;
addr = (isc_uint8_t *) &sin6->sin6_addr;
/* family */
isc_buffer_putuint16(&b, 2);
/* source prefix-length */
isc_buffer_putuint8(&b, prefixlen);
/* scope prefix-length */
isc_buffer_putuint8(&b, 0);
isc_buffer_putmem(&b,
(isc_uint8_t *) &sin6->sin6_addr,
(unsigned int) addrl);
/* address */
if (addrl > 0) {
isc_buffer_putmem(&b, addr, addrl - 1);
isc_buffer_putuint8(&b,
(addr[addrl - 1] &
mask));
}
}
opts[i].value = (isc_uint8_t *) ecsbuf;
......
......@@ -150,6 +150,13 @@ fromwire_opt(ARGS_FROMWIRE) {
addrbytes = (addrlen + 7) / 8;
if (addrbytes + 4 != length)
return (DNS_R_OPTERR);
if (addrbytes != 0U && (addrlen % 8) != 0) {
isc_uint8_t bits = ~0 << (8 - (addrlen % 8));
bits &= sregion.base[addrbytes - 1];
if (bits != sregion.base[addrbytes - 1])
return (DNS_R_OPTERR);
}
isc_region_consume(&sregion, addrbytes);
break;
}
......@@ -163,7 +170,7 @@ fromwire_opt(ARGS_FROMWIRE) {
break;
case DNS_OPT_COOKIE:
if (length != 8 && (length < 16 || length > 40))
return (DNS_R_FORMERR);
return (DNS_R_OPTERR);
isc_region_consume(&sregion, length);
break;
default:
......
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