From a95ec4fb1144c5602a5e4a7ccb14bbc0c8542cc5 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 23 Mar 2016 19:04:04 -0700 Subject: [PATCH] [v9_10] fix ECS family 0 handling 4341. [bug] Correct the handling of ECS options with address family 0. [RT #41377] --- CHANGES | 3 ++ bin/dig/dig.1 | 6 ++-- bin/dig/dig.docbook | 15 +++++---- bin/dig/dig.html | 15 +++++---- bin/dig/dighost.c | 58 ++++++++++++++++++++++------------ bin/named/client.c | 2 +- lib/dns/message.c | 17 ++++++++-- lib/dns/rdata/generic/opt_41.c | 28 ++++++++++++++-- 8 files changed, 101 insertions(+), 43 deletions(-) diff --git a/CHANGES b/CHANGES index 2e0e7c09e2..e349d1c24b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,9 @@ 4342. [bug] 'rndc flushtree' could fail to clean the tree if there wasn't a node at the specified name. [RT #41846] +4341. [bug] Correct the handling of ECS options with + address family 0. [RT #41377] + 4338. [bug] Reimplement change 4324 as it wasn't properly doing all the required book keeping. [RT #41941] diff --git a/bin/dig/dig.1 b/bin/dig/dig.1 index a61350b4d0..f6d4131f87 100644 --- a/bin/dig/dig.1 +++ b/bin/dig/dig.1 @@ -587,13 +587,15 @@ causes fields not to be split at all\&. The default is 56 characters, or 44 char This query option toggles the printing of statistics: when the query was made, the size of the reply and so on\&. The default behavior is to print the query statistics\&. .RE .PP -\fB+[no]subnet=addr[/netmask]\fR +\fB+[no]subnet=addr[/prefix\-length]\fR .RS 4 Send (don\*(Aqt send) an EDNS Client Subnet option with the specified IP address or network prefix\&. .sp \fBdig +subnet=0\&.0\&.0\&.0/0\fR, or simply \fBdig +subnet=0\fR -for short, sends a Client Subnet option with an empty address and a source prefix length of zero, which signals a resolver that the EDNS Client Subnet option should not be used when resolving this query\&. +for short, sends an EDNS client\-subnet option with an empty address and a source prefix\-length of zero, which signals a resolver that the client\*(Aqs address information must +\fInot\fR +be used when resolving this query\&. .RE .PP \fB+[no]tcp\fR diff --git a/bin/dig/dig.docbook b/bin/dig/dig.docbook index edfd894028..c54d677793 100644 --- a/bin/dig/dig.docbook +++ b/bin/dig/dig.docbook @@ -977,19 +977,20 @@ - + Send (don't send) an EDNS Client Subnet option with the - specified IP address or network prefix. + specified IP address or network prefix. dig +subnet=0.0.0.0/0, or simply - dig +subnet=0 for short, sends a - Client Subnet option with an empty address and a - source prefix length of zero, which signals a resolver - that the EDNS Client Subnet option should not be used - when resolving this query. + dig +subnet=0 for short, sends an EDNS + 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 + not be used when resolving + this query. diff --git a/bin/dig/dig.html b/bin/dig/dig.html index c66e385f65..8a91671518 100644 --- a/bin/dig/dig.html +++ b/bin/dig/dig.html @@ -590,19 +590,20 @@ so on. The default behavior is to print the query statistics.

-
+[no]subnet=addr[/netmask]
+
+[no]subnet=addr[/prefix-length]

Send (don't send) an EDNS Client Subnet option with the - specified IP address or network prefix. + specified IP address or network prefix.

dig +subnet=0.0.0.0/0, or simply - dig +subnet=0 for short, sends a - Client Subnet option with an empty address and a - source prefix length of zero, which signals a resolver - that the EDNS Client Subnet option should not be used - when resolving this query. + dig +subnet=0 for short, sends an EDNS + 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 + not be used when resolving + this query.

+[no]tcp
diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index c3f92c87fd..3ca7cb9448 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -1065,7 +1065,7 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) { isc_sockaddr_t *sa = NULL; struct in_addr in4; struct in6_addr in6; - isc_uint32_t netmask = 0xffffffff; + isc_uint32_t prefix_length = 0xffffffff; char *slash = NULL; isc_boolean_t parsed = ISC_FALSE; char buf[sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:XXX.XXX.XXX.XXX/128")]; @@ -1076,13 +1076,16 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) { slash = strchr(buf, '/'); if (slash != NULL) { *slash = '\0'; - result = isc_parse_uint32(&netmask, slash + 1, 10); + result = isc_parse_uint32(&prefix_length, slash + 1, 10); if (result != ISC_R_SUCCESS) { fatal("invalid prefix length in '%s': %s\n", value, isc_result_totext(result)); } - } else if (strcmp(value, "0") == 0) { - netmask = 0; + } + + if (strcmp(buf, "0") == 0) { + parsed = ISC_TRUE; + prefix_length = 0; } sa = isc_mem_allocate(mctx, sizeof(*sa)); @@ -1091,14 +1094,14 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) { if (inet_pton(AF_INET6, buf, &in6) == 1) { parsed = ISC_TRUE; isc_sockaddr_fromin6(sa, &in6, 0); - if (netmask > 128) - netmask = 128; + if (prefix_length > 128) + prefix_length = 128; } else if (inet_pton(AF_INET, buf, &in4) == 1) { parsed = ISC_TRUE; isc_sockaddr_fromin(sa, &in4, 0); - if (netmask > 32) - netmask = 32; - } else if (netmask != 0xffffffff) { + if (prefix_length > 32) + prefix_length = 32; + } else if (prefix_length != 0xffffffff && prefix_length != 0) { int i; for (i = 0; i < 3 && strlen(buf) < sizeof(buf) - 2; i++) { @@ -1110,14 +1113,17 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) { } } - if (netmask > 32) - netmask = 32; + if (prefix_length > 32) + prefix_length = 32; } if (!parsed) fatal("invalid address '%s'", value); - sa->length = netmask; + sa->length = prefix_length; + if (prefix_length == 0) + sa->type.sa.sa_family = AF_UNSPEC; + *sap = sa; return (ISC_R_SUCCESS); @@ -2467,7 +2473,7 @@ setup_lookup(dig_lookup_t *lookup) { } if (lookup->ecs_addr != NULL) { - isc_uint8_t addr[16], family; + isc_uint8_t addr[16], family, proto; isc_uint32_t plen; struct sockaddr *sa; struct sockaddr_in *sin; @@ -2485,17 +2491,29 @@ setup_lookup(dig_lookup_t *lookup) { opts[i].length = (isc_uint16_t) addrl + 4; check_result(result, "isc_buffer_allocate"); isc_buffer_init(&b, ecsbuf, sizeof(ecsbuf)); - if (sa->sa_family == AF_INET) { + + /* If prefix length is zero, don't set family */ + proto = sa->sa_family; + if (plen == 0) + proto = AF_UNSPEC; + + switch (proto) { + case AF_UNSPEC: + INSIST(plen == 0); + family = 0; + break; + case AF_INET: family = 1; sin = (struct sockaddr_in *) sa; - memcpy(addr, &sin->sin_addr, 4); - if ((plen % 8) != 0) - addr[addrl-1] &= - ~0 << (8 - (plen % 8)); - } else { + memmove(addr, &sin->sin_addr, 4); + break; + case AF_INET6: family = 2; sin6 = (struct sockaddr_in6 *) sa; - memcpy(addr, &sin6->sin6_addr, 16); + memmove(addr, &sin6->sin6_addr, 16); + break; + default: + INSIST(0); } /* Mask off last address byte */ diff --git a/bin/named/client.c b/bin/named/client.c index e0262b3aa8..a824eae8c9 100644 --- a/bin/named/client.c +++ b/bin/named/client.c @@ -1678,7 +1678,7 @@ process_cookie(ns_client_t *client, isc_buffer_t *buf, size_t optlen) { isc_buffer_t db; /* - * If we have already seen a ECS option skip this ECS option. + * If we have already seen a SIT option skip this SIT option. */ if ((client->attributes & NS_CLIENTATTR_WANTSIT) != 0) { isc_buffer_forward(buf, (isc_uint32_t)optlen); diff --git a/lib/dns/message.c b/lib/dns/message.c index d591984a39..c49bfa2878 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -3234,6 +3234,9 @@ 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); @@ -3246,15 +3249,23 @@ render_ecs(isc_buffer_t *ecsbuf, isc_buffer_t *target) { for (i = 0; i < addrbytes; i ++) addr[i] = isc_buffer_getuint8(ecsbuf); - if (family == 1) { + switch (family) { + case 0: + if (addrlen != 0U || scopelen != 0U) + return (DNS_R_OPTERR); + strlcpy(addr_text, "0", sizeof(addr_text)); + break; + case 1: if (addrlen > 32 || scopelen > 32) return (DNS_R_OPTERR); inet_ntop(AF_INET, addr, addr_text, sizeof(addr_text)); - } else if (family == 2) { + break; + case 2: if (addrlen > 128 || scopelen > 128) return (DNS_R_OPTERR); inet_ntop(AF_INET6, addr, addr_text, sizeof(addr_text)); - } else { + break; + default: snprintf(addr_text, sizeof(addr_text), "Unsupported family %u", family); ADD_STRING(target, addr_text); diff --git a/lib/dns/rdata/generic/opt_41.c b/lib/dns/rdata/generic/opt_41.c index 8a00fdb658..ea0e8cd44c 100644 --- a/lib/dns/rdata/generic/opt_41.c +++ b/lib/dns/rdata/generic/opt_41.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2004, 2005, 2007, 2009, 2011-2015 Internet Systems Consortium, Inc. ("ISC") + * Copyright (C) 2004, 2005, 2007, 2009, 2011-2016 Internet Systems Consortium, Inc. ("ISC") * Copyright (C) 1998-2002 Internet Software Consortium. * * Permission to use, copy, modify, and/or distribute this software for any @@ -15,8 +15,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id$ */ - /* Reviewed: Thu Mar 16 14:06:44 PST 2000 by gson */ /* RFC2671 */ @@ -135,7 +133,24 @@ fromwire_opt(ARGS_FROMWIRE) { isc_region_consume(&sregion, 1); scope = uint8_fromregion(&sregion); isc_region_consume(&sregion, 1); + + if (addrlen == 0U && family != 0U) + return (DNS_R_OPTERR); + switch (family) { + case 0: + /* + * XXXMUKS: In queries and replies, if + * FAMILY is set to 0, SOURCE + * PREFIX-LENGTH and SCOPE PREFIX-LENGTH + * must be 0 and ADDRESS should not be + * present as the address and prefix + * lengths don't make sense because the + * family is unknown. + */ + if (addrlen != 0U || scope != 0U) + return (DNS_R_OPTERR); + break; case 1: if (addrlen > 32U || scope > 32U) return (DNS_R_OPTERR); @@ -144,6 +159,8 @@ fromwire_opt(ARGS_FROMWIRE) { if (addrlen > 128U || scope > 128U) return (DNS_R_OPTERR); break; + default: + return (DNS_R_OPTERR); } addrbytes = (addrlen + 7) / 8; if (addrbytes + 4 != length) @@ -166,6 +183,11 @@ fromwire_opt(ARGS_FROMWIRE) { return (DNS_R_OPTERR); isc_region_consume(&sregion, length); break; + case DNS_OPT_COOKIE: + if (length != 8 && (length < 16 || length > 40)) + return (DNS_R_OPTERR); + isc_region_consume(&sregion, length); + break; default: isc_region_consume(&sregion, length); break;