From 858b3f763b93d77a9a9567152d3ebdc79eb23ac9 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 24 Oct 2024 16:55:31 -0700 Subject: [PATCH] refactor, add missing EDNS options, and fix option names some EDNS option names, including DAU, DHU, N3U, and CHAIN, were not printed in dns_message_pseudosectiontotext() or _psuedosectiontoyaml(); they were displayed as unknown options. this has been corrected. that code was also refactored to use switch instead of if/else, and to look up the option code names in a table to prevent inconsistencies between the two formats. one such inconsistency was corrected: the "TCP-KEEPALIVE" option is now always printed with a hyphen, instead of being "TCP KEEPALIVE" when not using YAML. the keepalive system test has been updated to expect this. EDNS options that print DNS names (i.e., CHAIN and Report-Channel) now enclose them in quotation marks to ensure YAML correctness. the auth system test has been updated to expect this when grepping for Report-Channel options. (cherry picked from commit e2393ba27bd334fb2146e1540c0dafcc75e9d688) --- bin/tests/system/keepalive/tests_keepalive.py | 12 +- lib/dns/include/dns/message.h | 31 +- lib/dns/message.c | 273 ++++++++++++------ 3 files changed, 208 insertions(+), 108 deletions(-) diff --git a/bin/tests/system/keepalive/tests_keepalive.py b/bin/tests/system/keepalive/tests_keepalive.py index 5fc8aaaf5b..c815c3e84b 100644 --- a/bin/tests/system/keepalive/tests_keepalive.py +++ b/bin/tests/system/keepalive/tests_keepalive.py @@ -31,24 +31,24 @@ def test_dig_tcp_keepalive_handling(named_port, servers): dig = isctest.run.Dig(f"-p {str(named_port)}") isctest.log.info("check that dig handles TCP keepalive in query") - assert "; TCP KEEPALIVE" in dig("+qr +keepalive foo.example. @10.53.0.2") + assert "; TCP-KEEPALIVE" in dig("+qr +keepalive foo.example. @10.53.0.2") isctest.log.info("check that dig added TCP keepalive was received") assert get_keepalive_options_received() == 1 isctest.log.info("check that TCP keepalive is added for TCP responses") - assert "; TCP KEEPALIVE" in dig("+tcp +keepalive foo.example. @10.53.0.2") + assert "; TCP-KEEPALIVE" in dig("+tcp +keepalive foo.example. @10.53.0.2") isctest.log.info("check that TCP keepalive requires TCP") - assert "; TCP KEEPALIVE" not in dig("+keepalive foo.example. @10.53.0.2") + assert "; TCP-KEEPALIVE" not in dig("+keepalive foo.example. @10.53.0.2") isctest.log.info("check the default keepalive value") - assert "; TCP KEEPALIVE: 30.0 secs" in dig( + assert "; TCP-KEEPALIVE: 30.0 secs" in dig( "+tcp +keepalive foo.example. @10.53.0.3" ) isctest.log.info("check a keepalive configured value") - assert "; TCP KEEPALIVE: 15.0 secs" in dig( + assert "; TCP-KEEPALIVE: 15.0 secs" in dig( "+tcp +keepalive foo.example. @10.53.0.2" ) @@ -58,7 +58,7 @@ def test_dig_tcp_keepalive_handling(named_port, servers): assert "tcp-idle-timeout=300" in response assert "tcp-keepalive-timeout=300" in response assert "tcp-advertised-timeout=200" in response - assert "; TCP KEEPALIVE: 20.0 secs" in dig( + assert "; TCP-KEEPALIVE: 20.0 secs" in dig( "+tcp +keepalive foo.example. @10.53.0.2" ) diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h index f9c82f023d..89e20b8306 100644 --- a/lib/dns/include/dns/message.h +++ b/lib/dns/include/dns/message.h @@ -103,18 +103,25 @@ #define DNS_MESSAGEEXTFLAG_DO 0x8000U /*%< EDNS0 extended OPT codes */ -#define DNS_OPT_LLQ 1 /*%< LLQ opt code */ -#define DNS_OPT_UL 2 /*%< UL opt code */ -#define DNS_OPT_NSID 3 /*%< NSID opt code */ -#define DNS_OPT_CLIENT_SUBNET 8 /*%< client subnet opt code */ -#define DNS_OPT_EXPIRE 9 /*%< EXPIRE opt code */ -#define DNS_OPT_COOKIE 10 /*%< COOKIE opt code */ -#define DNS_OPT_TCP_KEEPALIVE 11 /*%< TCP keepalive opt code */ -#define DNS_OPT_PAD 12 /*%< PAD opt code */ -#define DNS_OPT_KEY_TAG 14 /*%< Key tag opt code */ -#define DNS_OPT_EDE 15 /*%< Extended DNS Error opt code */ -#define DNS_OPT_CLIENT_TAG 16 /*%< Client tag opt code */ -#define DNS_OPT_SERVER_TAG 17 /*%< Server tag opt code */ + +#define DNS_OPT_LLQ 1 /*%< LLQ opt code */ +#define DNS_OPT_UL 2 /*%< UL opt code */ +#define DNS_OPT_NSID 3 /*%< NSID opt code */ +#define DNS_OPT_DAU 5 /*%< DNSSEC algorithm understood */ +#define DNS_OPT_DHU 6 /*%< DNSSEC hash understood */ +#define DNS_OPT_N3U 7 /*%< NSEC3 hash understood */ +#define DNS_OPT_CLIENT_SUBNET 8 /*%< client subnet opt code */ +#define DNS_OPT_EXPIRE 9 /*%< EXPIRE opt code */ +#define DNS_OPT_COOKIE 10 /*%< COOKIE opt code */ +#define DNS_OPT_TCP_KEEPALIVE 11 /*%< TCP keepalive opt code */ +#define DNS_OPT_PAD 12 /*%< PAD opt code */ +#define DNS_OPT_CHAIN 13 /*%< CHAIN opt code */ +#define DNS_OPT_KEY_TAG 14 /*%< Key tag opt code */ +#define DNS_OPT_EDE 15 /*%< Extended DNS Error opt code */ +#define DNS_OPT_CLIENT_TAG 16 /*%< Client tag opt code */ +#define DNS_OPT_SERVER_TAG 17 /*%< Server tag opt code */ +#define DNS_OPT_REPORT_CHANNEL 18 /*%< DNS Reporting Channel */ +#define DNS_OPT_ZONEVERSION 19 /*%< Zoneversion opt code */ /*%< Experimental options [65001...65534] as per RFC6891 */ diff --git a/lib/dns/message.c b/lib/dns/message.c index 04699cfad5..f5aa7bb28f 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -3564,6 +3564,48 @@ cleanup: return result; } +static isc_result_t +render_nameopt(isc_buffer_t *optbuf, isc_buffer_t *target) { + dns_decompress_t dctx = DNS_DECOMPRESS_NEVER; + dns_fixedname_t fixed; + dns_name_t *name = dns_fixedname_initname(&fixed); + char namebuf[DNS_NAME_FORMATSIZE]; + isc_result_t result; + + result = dns_name_fromwire(name, optbuf, dctx, NULL); + if (result == ISC_R_SUCCESS && isc_buffer_activelength(optbuf) == 0) { + dns_name_format(name, namebuf, sizeof(namebuf)); + ADD_STRING(target, " \""); + ADD_STRING(target, namebuf); + ADD_STRING(target, "\""); + return result; + } + result = ISC_R_FAILURE; +cleanup: + return result; +} + +static const char *option_names[] = { + [DNS_OPT_LLQ] = "LLQ", + [DNS_OPT_UL] = "UL", + [DNS_OPT_NSID] = "NSID", + [DNS_OPT_DAU] = "DAU", + [DNS_OPT_DHU] = "DHU", + [DNS_OPT_N3U] = "N3U", + [DNS_OPT_CLIENT_SUBNET] = "CLIENT-SUBNET", + [DNS_OPT_EXPIRE] = "EXPIRE", + [DNS_OPT_COOKIE] = "COOKIE", + [DNS_OPT_TCP_KEEPALIVE] = "TCP-KEEPALIVE", + [DNS_OPT_PAD] = "PADDING", + [DNS_OPT_CHAIN] = "CHAIN", + [DNS_OPT_KEY_TAG] = "KEY-TAG", + [DNS_OPT_EDE] = "EDE", + [DNS_OPT_CLIENT_TAG] = "CLIENT-TAG", + [DNS_OPT_SERVER_TAG] = "SERVER-TAG", + [DNS_OPT_REPORT_CHANNEL] = "Report-Channel", + [DNS_OPT_ZONEVERSION] = "ZONEVERSION", +}; + static isc_result_t dns_message_pseudosectiontoyaml(dns_message_t *msg, dns_pseudosection_t section, const dns_master_style_t *style, @@ -3578,8 +3620,9 @@ dns_message_pseudosectiontoyaml(dns_message_t *msg, dns_pseudosection_t section, isc_buffer_t optbuf; uint16_t optcode, optlen; size_t saved_count; - unsigned char *optdata; + unsigned char *optdata = NULL; unsigned int indent; + isc_buffer_t ecsbuf; REQUIRE(DNS_MESSAGE_VALID(msg)); REQUIRE(target != NULL); @@ -3647,15 +3690,28 @@ dns_message_pseudosectiontoyaml(dns_message_t *msg, dns_pseudosection_t section, isc_buffer_add(&optbuf, rdata.length); while (isc_buffer_remaininglength(&optbuf) != 0) { bool extra_text = false; + const char *option_name = NULL; + msg->indent.count = indent; INSIST(isc_buffer_remaininglength(&optbuf) >= 4U); optcode = isc_buffer_getuint16(&optbuf); optlen = isc_buffer_getuint16(&optbuf); INSIST(isc_buffer_remaininglength(&optbuf) >= optlen); - if (optcode == DNS_OPT_LLQ) { - INDENT(style); - ADD_STRING(target, "LLQ:"); + INDENT(style); + if (optcode < ARRAY_SIZE(option_names)) { + option_name = option_names[optcode]; + } + if (option_name != NULL) { + ADD_STRING(target, option_names[optcode]) + } else { + snprintf(buf, sizeof(buf), "OPT=%u", optcode); + ADD_STRING(target, buf); + } + ADD_STRING(target, ":"); + + switch (optcode) { + case DNS_OPT_LLQ: if (optlen == 18U) { result = render_llq(&optbuf, target); if (result != ISC_R_SUCCESS) { @@ -3664,9 +3720,8 @@ dns_message_pseudosectiontoyaml(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, "\n"); continue; } - } else if (optcode == DNS_OPT_UL) { - INDENT(style); - ADD_STRING(target, "UL:"); + break; + case DNS_OPT_UL: if (optlen == 4U || optlen == 8U) { uint32_t secs, key = 0; secs = isc_buffer_getuint32(&optbuf); @@ -3697,16 +3752,8 @@ dns_message_pseudosectiontoyaml(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, ")\n"); continue; } - } else if (optcode == DNS_OPT_NSID) { - INDENT(style); - ADD_STRING(target, "NSID:"); - } else if (optcode == DNS_OPT_COOKIE) { - INDENT(style); - ADD_STRING(target, "COOKIE:"); - } else if (optcode == DNS_OPT_CLIENT_SUBNET) { - isc_buffer_t ecsbuf; - INDENT(style); - ADD_STRING(target, "CLIENT-SUBNET:"); + break; + case DNS_OPT_CLIENT_SUBNET: isc_buffer_init(&ecsbuf, isc_buffer_current(&optbuf), optlen); @@ -3721,9 +3768,8 @@ dns_message_pseudosectiontoyaml(dns_message_t *msg, dns_pseudosection_t section, continue; } ADD_STRING(target, "\n"); - } else if (optcode == DNS_OPT_EXPIRE) { - INDENT(style); - ADD_STRING(target, "EXPIRE:"); + break; + case DNS_OPT_EXPIRE: if (optlen == 4) { uint32_t secs; secs = isc_buffer_getuint32(&optbuf); @@ -3738,32 +3784,38 @@ dns_message_pseudosectiontoyaml(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, ")\n"); continue; } - } else if (optcode == DNS_OPT_TCP_KEEPALIVE) { + break; + case DNS_OPT_TCP_KEEPALIVE: if (optlen == 2) { unsigned int dsecs; dsecs = isc_buffer_getuint16(&optbuf); - INDENT(style); - ADD_STRING(target, "TCP-KEEPALIVE: "); snprintf(buf, sizeof(buf), "%u.%u", dsecs / 10U, dsecs % 10U); ADD_STRING(target, buf); ADD_STRING(target, " secs\n"); continue; } - INDENT(style); - ADD_STRING(target, "TCP-KEEPALIVE:"); - } else if (optcode == DNS_OPT_PAD) { - INDENT(style); - ADD_STRING(target, "PAD:"); - } else if (optcode == DNS_OPT_KEY_TAG) { - INDENT(style); - ADD_STRING(target, "KEY-TAG:"); + break; + case DNS_OPT_CHAIN: + if (optlen > 0U) { + isc_buffer_t sb = optbuf; + isc_buffer_setactive(&optbuf, optlen); + result = render_nameopt(&optbuf, + target); + if (result == ISC_R_SUCCESS) { + ADD_STRING(target, "\n"); + continue; + } + optbuf = sb; + } + break; + case DNS_OPT_KEY_TAG: if (optlen > 0U && (optlen % 2U) == 0U) { const char *sep = ""; - uint16_t id; while (optlen > 0U) { - id = isc_buffer_getuint16( - &optbuf); + uint16_t id = + isc_buffer_getuint16( + &optbuf); snprintf(buf, sizeof(buf), "%s %u", sep, id); ADD_STRING(target, buf); @@ -3773,9 +3825,8 @@ dns_message_pseudosectiontoyaml(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, "\n"); continue; } - } else if (optcode == DNS_OPT_EDE) { - INDENT(style); - ADD_STRING(target, "EDE:"); + break; + case DNS_OPT_EDE: if (optlen >= 2U) { uint16_t ede; ADD_STRING(target, "\n"); @@ -3800,31 +3851,40 @@ dns_message_pseudosectiontoyaml(dns_message_t *msg, dns_pseudosection_t section, extra_text = true; } } - } else if (optcode == DNS_OPT_CLIENT_TAG) { - uint16_t id; - INDENT(style); - ADD_STRING(target, "CLIENT-TAG:"); + break; + case DNS_OPT_CLIENT_TAG: if (optlen == 2U) { - id = isc_buffer_getuint16(&optbuf); + uint16_t id = + isc_buffer_getuint16(&optbuf); snprintf(buf, sizeof(buf), " %u\n", id); ADD_STRING(target, buf); continue; } - } else if (optcode == DNS_OPT_SERVER_TAG) { - uint16_t id; - INDENT(style); - ADD_STRING(target, "SERVER-TAG:"); + break; + case DNS_OPT_SERVER_TAG: if (optlen == 2U) { - id = isc_buffer_getuint16(&optbuf); + uint16_t id = + isc_buffer_getuint16(&optbuf); snprintf(buf, sizeof(buf), " %u\n", id); ADD_STRING(target, buf); continue; } - } else { - INDENT(style); - ADD_STRING(target, "OPT="); - snprintf(buf, sizeof(buf), "%u:", optcode); - ADD_STRING(target, buf); + break; + case DNS_OPT_REPORT_CHANNEL: + if (optlen > 0U) { + isc_buffer_t sb = optbuf; + isc_buffer_setactive(&optbuf, optlen); + result = render_nameopt(&optbuf, + target); + if (result == ISC_R_SUCCESS) { + ADD_STRING(target, "\n"); + continue; + } + optbuf = sb; + } + break; + default: + break; } if (optlen != 0) { @@ -3973,7 +4033,8 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, dns_rdata_t rdata; isc_buffer_t optbuf; uint16_t optcode, optlen; - unsigned char *optdata; + unsigned char *optdata = NULL; + isc_buffer_t ecsbuf; REQUIRE(DNS_MESSAGE_VALID(msg)); REQUIRE(target != NULL); @@ -4035,6 +4096,8 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, isc_buffer_init(&optbuf, rdata.data, rdata.length); isc_buffer_add(&optbuf, rdata.length); while (isc_buffer_remaininglength(&optbuf) != 0) { + const char *option_name = NULL; + INSIST(isc_buffer_remaininglength(&optbuf) >= 4U); optcode = isc_buffer_getuint16(&optbuf); optlen = isc_buffer_getuint16(&optbuf); @@ -4042,9 +4105,20 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, INSIST(isc_buffer_remaininglength(&optbuf) >= optlen); INDENT(style); + ADD_STRING(target, "; "); + if (optcode < ARRAY_SIZE(option_names)) { + option_name = option_names[optcode]; + } + if (option_name != NULL) { + ADD_STRING(target, option_names[optcode]) + } else { + snprintf(buf, sizeof(buf), "OPT=%u", optcode); + ADD_STRING(target, buf); + } + ADD_STRING(target, ":"); - if (optcode == DNS_OPT_LLQ) { - ADD_STRING(target, "; LLQ:"); + switch (optcode) { + case DNS_OPT_LLQ: if (optlen == 18U) { result = render_llq(&optbuf, target); if (result != ISC_R_SUCCESS) { @@ -4053,8 +4127,8 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, "\n"); continue; } - } else if (optcode == DNS_OPT_UL) { - ADD_STRING(target, "; UL:"); + break; + case DNS_OPT_UL: if (optlen == 4U || optlen == 8U) { uint32_t secs, key = 0; secs = isc_buffer_getuint32(&optbuf); @@ -4085,14 +4159,8 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, ")\n"); continue; } - } else if (optcode == DNS_OPT_NSID) { - ADD_STRING(target, "; NSID:"); - } else if (optcode == DNS_OPT_COOKIE) { - ADD_STRING(target, "; COOKIE:"); - } else if (optcode == DNS_OPT_CLIENT_SUBNET) { - isc_buffer_t ecsbuf; - - ADD_STRING(target, "; CLIENT-SUBNET:"); + break; + case DNS_OPT_CLIENT_SUBNET: isc_buffer_init(&ecsbuf, isc_buffer_current(&optbuf), optlen); @@ -4106,8 +4174,8 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, "\n"); continue; } - } else if (optcode == DNS_OPT_EXPIRE) { - ADD_STRING(target, "; EXPIRE:"); + break; + case DNS_OPT_EXPIRE: if (optlen == 4) { uint32_t secs; secs = isc_buffer_getuint32(&optbuf); @@ -4122,8 +4190,8 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, ")\n"); continue; } - } else if (optcode == DNS_OPT_TCP_KEEPALIVE) { - ADD_STRING(target, "; TCP KEEPALIVE:"); + break; + case DNS_OPT_TCP_KEEPALIVE: if (optlen == 2) { unsigned int dsecs; dsecs = isc_buffer_getuint16(&optbuf); @@ -4133,8 +4201,8 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, " secs\n"); continue; } - } else if (optcode == DNS_OPT_PAD) { - ADD_STRING(target, "; PAD:"); + break; + case DNS_OPT_PAD: if (optlen > 0U) { snprintf(buf, sizeof(buf), " (%u bytes)", optlen); @@ -4143,14 +4211,27 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, } ADD_STRING(target, "\n"); continue; - } else if (optcode == DNS_OPT_KEY_TAG) { - ADD_STRING(target, "; KEY-TAG:"); + case DNS_OPT_CHAIN: + if (optlen > 0U) { + isc_buffer_t sb = optbuf; + isc_buffer_setactive(&optbuf, optlen); + result = render_nameopt(&optbuf, + target); + if (result == ISC_R_SUCCESS) { + ADD_STRING(target, "\n"); + continue; + } + optbuf = sb; + } + ADD_STRING(target, "\n"); + break; + case DNS_OPT_KEY_TAG: if (optlen > 0U && (optlen % 2U) == 0U) { const char *sep = ""; - uint16_t id; while (optlen > 0U) { - id = isc_buffer_getuint16( - &optbuf); + uint16_t id = + isc_buffer_getuint16( + &optbuf); snprintf(buf, sizeof(buf), "%s %u", sep, id); ADD_STRING(target, buf); @@ -4160,8 +4241,8 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, "\n"); continue; } - } else if (optcode == DNS_OPT_EDE) { - ADD_STRING(target, "; EDE:"); + break; + case DNS_OPT_EDE: if (optlen >= 2U) { uint16_t ede; ede = isc_buffer_getuint16(&optbuf); @@ -4189,28 +4270,40 @@ dns_message_pseudosectiontotext(dns_message_t *msg, dns_pseudosection_t section, ADD_STRING(target, buf); continue; } - } else if (optcode == DNS_OPT_CLIENT_TAG) { - uint16_t id; - ADD_STRING(target, "; CLIENT-TAG:"); + break; + case DNS_OPT_CLIENT_TAG: if (optlen == 2U) { - id = isc_buffer_getuint16(&optbuf); + uint16_t id = + isc_buffer_getuint16(&optbuf); snprintf(buf, sizeof(buf), " %u\n", id); ADD_STRING(target, buf); continue; } - } else if (optcode == DNS_OPT_SERVER_TAG) { - uint16_t id; - ADD_STRING(target, "; SERVER-TAG:"); + break; + case DNS_OPT_SERVER_TAG: if (optlen == 2U) { - id = isc_buffer_getuint16(&optbuf); + uint16_t id = + isc_buffer_getuint16(&optbuf); snprintf(buf, sizeof(buf), " %u\n", id); ADD_STRING(target, buf); continue; } - } else { - ADD_STRING(target, "; OPT="); - snprintf(buf, sizeof(buf), "%u:", optcode); - ADD_STRING(target, buf); + break; + case DNS_OPT_REPORT_CHANNEL: + if (optlen > 0U) { + isc_buffer_t sb = optbuf; + isc_buffer_setactive(&optbuf, optlen); + result = render_nameopt(&optbuf, + target); + if (result == ISC_R_SUCCESS) { + ADD_STRING(target, "\n"); + continue; + } + optbuf = sb; + } + break; + default: + break; } if (optlen != 0) {