From 0e00e28ec645e22e0b892d9518eb1832895beb9c Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 9 Sep 2024 15:59:30 +1000 Subject: [PATCH] Parse the URI template and check for a dns variable The 'dns' variable in dohpath can be in various forms ({?dns}, {dns}, {&dns} etc.). To check for a valid dohpath it ends up being simpler to just parse the URI template rather than looking for all the various forms if substring. (cherry picked from commit af54ef9f5d703ed61c140f3e32e6fab5d88923a1) --- lib/dns/rdata.c | 171 +++++++++++++++++++++++++++++++++++ lib/dns/rdata/in_1/svcb_64.c | 25 +---- tests/dns/rdata_test.c | 45 ++++++++- 3 files changed, 212 insertions(+), 29 deletions(-) diff --git a/lib/dns/rdata.c b/lib/dns/rdata.c index db7351a0cd..976b3c6d9c 100644 --- a/lib/dns/rdata.c +++ b/lib/dns/rdata.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -636,6 +637,176 @@ check_private(isc_buffer_t *source, dns_secalg_t alg) { return ISC_R_SUCCESS; } +/* + * A relative URI template that has a "dns" variable. + */ +static bool +validate_dohpath(isc_region_t *region) { + const unsigned char *p; + const unsigned char *v = NULL; + const unsigned char *n = NULL; + unsigned char c; + bool dns = false; + bool wasop = false; + enum { + path, + variable, + percent1, + percent2, + variable_percent1, + variable_percent2, + prefix, + explode + } state = path; + + if (region->length == 0 || *region->base != '/' || + !isc_utf8_valid(region->base, region->length)) + { + return false; + } + + /* + * RFC 6570 URI Template check + "dns" variable. + */ + p = region->base; + while (p < region->base + region->length) { + switch (state) { + case path: + switch (*p++) { + case '{': /*}*/ + state = variable; + wasop = false; + v = p; + break; + case '%': + state = percent1; + break; + default: + break; + } + break; + case variable: + c = *p++; + switch (c) { + case '+': + case '#': + case '.': + case '/': + case ';': + case '?': + case '&': + /* Operators. */ + if (p != v + 1 || wasop) { + return false; + } + wasop = true; + v = p; + break; + case '=': + case '!': + case '@': + case '|': + /* Reserved operators. */ + return false; + case '*': + case ':': + case '}': + case ',': + /* Found the end of the variable name. */ + if (p == (v + 1)) { + return false; + } + /* 'p' has been incremented so 4 not 3 */ + if ((p - v) == 4 && memcmp(v, "dns", 3) == 0) { + dns = true; + } + switch (c) { + case ':': + state = prefix; + n = p; + break; + case /*{*/ '}': + state = path; + break; + case '*': + state = explode; + break; + case ',': + wasop = false; + v = p; + break; + } + break; + case '%': + /* Percent encoded variable name. */ + state = variable_percent1; + break; + default: + /* Valid variable name character? */ + if (c != '_' && !isalnum(c)) { + return false; + } + break; + } + break; + case explode: + switch (*p++) { + case ',': + state = variable; + wasop = false; + v = p; + break; + case /*}*/ '}': + state = path; + break; + default: + return false; + } + break; + /* Check % encoding */ + case percent1: + case percent2: + case variable_percent1: + case variable_percent2: + /* bad percent encoding? */ + if (!isxdigit(*p++)) { + return false; + } + if (state == percent1) { + state = percent2; + } else if (state == percent2) { + state = path; + } else if (state == variable_percent1) { + state = variable_percent2; + } else { + state = variable; + } + break; + case prefix: + c = *p++; + if (!isdigit(c)) { + /* valid number range [1..9999] */ + if ((p == n + 1) || (p - n) > 5 || *n == '0') { + return false; + } + switch (c) { + case ',': + state = variable; + wasop = false; + break; + case /*{*/ '}': + state = path; + break; + default: + return false; + } + } + break; + } + } + return state == path && dns; +} + #include "code.h" #define META 0x0001 diff --git a/lib/dns/rdata/in_1/svcb_64.c b/lib/dns/rdata/in_1/svcb_64.c index 754e3ef71f..8764617688 100644 --- a/lib/dns/rdata/in_1/svcb_64.c +++ b/lib/dns/rdata/in_1/svcb_64.c @@ -155,30 +155,7 @@ svcb_validate(uint16_t key, isc_region_t *region) { case sbpr_base64: break; case sbpr_dohpath: - /* - * Minimum valid dohpath is "/{?dns}" as - * it MUST be relative (leading "/") and - * MUST contain "{?dns}" or "{&dns}". - */ - if (region->length < 7) { - return DNS_R_FORMERR; - } - /* MUST be relative */ - if (region->base[0] != '/') { - return DNS_R_FORMERR; - } - /* MUST be UTF8 */ - if (!isc_utf8_valid(region->base, - region->length)) - { - return DNS_R_FORMERR; - } - /* MUST contain "{?dns}" or "{&dns}" */ - if (strnstr((char *)region->base, "{?dns}", - region->length) == NULL && - strnstr((char *)region->base, "{&dns}", - region->length) == NULL) - { + if (!validate_dohpath(region)) { return DNS_R_FORMERR; } break; diff --git a/tests/dns/rdata_test.c b/tests/dns/rdata_test.c index c2762176a1..6ce3b10660 100644 --- a/tests/dns/rdata_test.c +++ b/tests/dns/rdata_test.c @@ -2655,15 +2655,50 @@ ISC_RUN_TEST_IMPL(https_svcb) { TEXT_INVALID("1 foo.example.com. ( mandatory=key123,key123 " "key123=abc)"), /* dohpath tests */ + TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=/{dns}", + "1 example.net. key7=\"/{dns}\""), + TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=/{+dns}", + "1 example.net. key7=\"/{+dns}\""), + TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=/{#dns}", + "1 example.net. key7=\"/{#dns}\""), + TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=/{.dns}", + "1 example.net. key7=\"/{.dns}\""), + TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=\"/{;dns}\"", + "1 example.net. key7=\"/{;dns}\""), TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=/{?dns}", "1 example.net. key7=\"/{?dns}\""), TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=/some/path{?dns}", "1 example.net. key7=\"/some/path{?dns}\""), - TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=/some/path?key=value{&dns}", - "1 example.net. key7=\"/some/path?key=value{&dns}\""), - TEXT_INVALID("1 example.com. dohpath=no-slash"), - TEXT_INVALID("1 example.com. dohpath=/{?notdns}"), - TEXT_INVALID("1 example.com. dohpath=/notvariable"), + TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=/{dns:9999}", + "1 example.net. key7=\"/{dns:9999}\""), + TEXT_VALID_LOOPCHG(1, "1 example.net. dohpath=/{dns*}", + "1 example.net. key7=\"/{dns*}\""), + TEXT_VALID_LOOPCHG( + 1, "1 example.net. dohpath=/some/path?key=value{&dns}", + "1 example.net. key7=\"/some/path?key=value{&dns}\""), + TEXT_VALID_LOOPCHG(1, + "1 example.net. " + "dohpath=/some/path?key=value{&dns,x*}", + "1 example.net. " + "key7=\"/some/path?key=value{&dns,x*}\""), + TEXT_INVALID("1 example.com. dohpath=not-relative"), + TEXT_INVALID("1 example.com. dohpath=/{?no_dns_variable}"), + TEXT_INVALID("1 example.com. dohpath=/novariable"), + TEXT_INVALID("1 example.com. dohpath=/{?dnsx}"), + /* index too big > 9999 */ + TEXT_INVALID("1 example.com. dohpath=/{?dns:10000}"), + /* index not postive */ + TEXT_INVALID("1 example.com. dohpath=/{?dns:0}"), + /* index leading zero */ + TEXT_INVALID("1 example.com. dohpath=/{?dns:01}"), + /* two operators */ + TEXT_INVALID("1 example.com. dohpath=/{??dns}"), + /* invalid % encoding */ + TEXT_INVALID("1 example.com. dohpath=/%a{?dns}"), + /* invalid % encoding */ + TEXT_INVALID("1 example.com. dohpath=/{?dns,%a}"), + /* incomplete macro */ + TEXT_INVALID("1 example.com. dohpath=/{?dns" /*}*/), TEXT_SENTINEL() };