diff --git a/CHANGES b/CHANGES index d449d1d1db..5c015501b2 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,8 @@ --- 9.10.0a1 released -- +3644. [protocol] Check that EDNS subnet client options are well formed. + [RT #34718] + 3643. [doc] Clarify RRL "slip" documentation. 3642. [func] Allow externally generated DNSKEY to be imported diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h index 089b8ff294..b5ac3a835b 100644 --- a/lib/dns/include/dns/message.h +++ b/lib/dns/include/dns/message.h @@ -105,6 +105,7 @@ /*%< EDNS0 extended OPT codes */ #define DNS_OPT_NSID 0x0003 /*%< NSID opt code */ +#define DNS_OPT_CLIENT_SUBNET 0x0008 /*%< client subnet opt code */ #define DNS_MESSAGE_REPLYPRESERVE (DNS_MESSAGEFLAG_RD|DNS_MESSAGEFLAG_CD) #define DNS_MESSAGEEXTFLAG_REPLYPRESERVE (DNS_MESSAGEEXTFLAG_DO) diff --git a/lib/dns/rdata/generic/opt_41.c b/lib/dns/rdata/generic/opt_41.c index 4b51804317..ee233663e3 100644 --- a/lib/dns/rdata/generic/opt_41.c +++ b/lib/dns/rdata/generic/opt_41.c @@ -97,6 +97,7 @@ static inline isc_result_t fromwire_opt(ARGS_FROMWIRE) { isc_region_t sregion; isc_region_t tregion; + isc_uint16_t opt; isc_uint16_t length; unsigned int total; @@ -112,17 +113,48 @@ fromwire_opt(ARGS_FROMWIRE) { while (sregion.length != 0) { if (sregion.length < 4) return (ISC_R_UNEXPECTEDEND); - /* - * Eat the 16bit option code. There is nothing to - * be done with it currently. - */ + opt = uint16_fromregion(&sregion); isc_region_consume(&sregion, 2); length = uint16_fromregion(&sregion); isc_region_consume(&sregion, 2); total += 4; if (sregion.length < length) return (ISC_R_UNEXPECTEDEND); - isc_region_consume(&sregion, length); + switch (opt) { + case DNS_OPT_CLIENT_SUBNET: { + isc_uint16_t family; + isc_uint8_t addrlen; + isc_uint8_t scope; + isc_uint8_t addrbytes; + + if (length < 4) + return (DNS_R_FORMERR); + family = uint16_fromregion(&sregion); + isc_region_consume(&sregion, 2); + addrlen = uint8_fromregion(&sregion); + isc_region_consume(&sregion, 1); + scope = uint8_fromregion(&sregion); + isc_region_consume(&sregion, 1); + switch (family) { + case 1: + if (addrlen > 32U || scope > 32U) + return (DNS_R_FORMERR); + break; + case 2: + if (addrlen > 128U || scope > 128U) + return (DNS_R_FORMERR); + break; + } + addrbytes = (addrlen + 7) / 8; + if (addrbytes + 4 != length) + return (DNS_R_FORMERR); + isc_region_consume(&sregion, addrbytes); + break; + } + default: + isc_region_consume(&sregion, length); + break; + } total += length; } diff --git a/lib/dns/tests/rdata_test.c b/lib/dns/tests/rdata_test.c index 5312267729..d31fc09b2e 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -75,11 +75,164 @@ ATF_TC_BODY(hip, tc) { ATF_REQUIRE_EQ(result, DNS_R_FORMERR); } +ATF_TC(edns_client_subnet); +ATF_TC_HEAD(edns_client_subnet, tc) { + atf_tc_set_md_var(tc, "descr", + "check EDNS client subnet option parsing"); +} +ATF_TC_BODY(edns_client_subnet, tc) { + struct { + unsigned char data[64]; + size_t len; + isc_boolean_t ok; + } test_data[] = { + { + /* option code with no content */ + { 0x00, 0x08, 0x0, 0x00 }, 4, ISC_FALSE + }, + { + /* Option code family 0, source 0, scope 0 */ + { + 0x00, 0x08, 0x00, 0x04, + 0x00, 0x00, 0x00, 0x00 + }, + 8, ISC_TRUE + }, + { + /* Option code family 1 (ipv4), source 0, scope 0 */ + { + 0x00, 0x08, 0x00, 0x04, + 0x00, 0x01, 0x00, 0x00 + }, + 8, ISC_TRUE + }, + { + /* Option code family 2 (ipv6) , source 0, scope 0 */ + { + 0x00, 0x08, 0x00, 0x04, + 0x00, 0x02, 0x00, 0x00 + }, + 8, ISC_TRUE + }, + { + /* extra octet */ + { + 0x00, 0x08, 0x00, 0x05, + 0x00, 0x00, 0x00, 0x00, + 0x00 + }, + 9, ISC_FALSE + }, + { + /* source too long for IPv4 */ + { + 0x00, 0x08, 0x00, 8, + 0x00, 0x01, 33, 0x00, + 0x00, 0x00, 0x00, 0x00 + }, + 12, ISC_FALSE + }, + { + /* source too long for IPv6 */ + { + 0x00, 0x08, 0x00, 20, + 0x00, 0x02, 129, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + }, + 24, ISC_FALSE + }, + { + /* scope too long for IPv4 */ + { + 0x00, 0x08, 0x00, 8, + 0x00, 0x01, 0x00, 33, + 0x00, 0x00, 0x00, 0x00 + }, + 12, ISC_FALSE + }, + { + /* scope too long for IPv6 */ + { + 0x00, 0x08, 0x00, 20, + 0x00, 0x02, 0x00, 129, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + }, + 24, ISC_FALSE + }, + { + /* length too short for source generic */ + { + 0x00, 0x08, 0x00, 5, + 0x00, 0x00, 17, 0x00, + 0x00, 0x00, + }, + 19, ISC_FALSE + }, + { + /* length too short for source ipv4 */ + { + 0x00, 0x08, 0x00, 7, + 0x00, 0x01, 32, 0x00, + 0x00, 0x00, 0x00, 0x00 + }, + 11, ISC_FALSE + }, + { + /* length too short for source ipv6 */ + { + 0x00, 0x08, 0x00, 19, + 0x00, 0x02, 128, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + }, + 23, ISC_FALSE + }, + { + /* sentinal */ + { 0x00 }, 0, ISC_FALSE + } + }; + unsigned char buf[1024*1024]; + isc_buffer_t source, target; + dns_rdata_t rdata; + dns_decompress_t dctx; + isc_result_t result; + size_t i; + + UNUSED(tc); + + for (i = 0; test_data[i].len != 0; i++) { + isc_buffer_init(&source, test_data[i].data, test_data[i].len); + isc_buffer_add(&source, test_data[i].len); + isc_buffer_setactive(&source, test_data[i].len); + isc_buffer_init(&target, buf, sizeof(buf)); + dns_rdata_init(&rdata); + dns_decompress_init(&dctx, -1, DNS_DECOMPRESS_ANY); + result = dns_rdata_fromwire(&rdata, dns_rdataclass_in, + dns_rdatatype_opt, &source, + &dctx, 0, &target); + dns_decompress_invalidate(&dctx); + if (test_data[i].ok) + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + else + ATF_REQUIRE(result != ISC_R_SUCCESS); + } +} + /* * Main */ ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, hip); + ATF_TP_ADD_TC(tp, edns_client_subnet); return (atf_no_error()); }