[9.18] fix: usr: '{&dns}' is as valid as '{?dns}' in a SVCB's dohpath

`dig` fails to parse a valid (as far as I can tell, and accepted by `kdig` and `Wireshark`) `SVCB` record with a `dohpath` URI template containing a `{&dns}`, like `dohpath=/some/path?key=value{&dns}"`. If the URI template contains a `{?dns}` instead `dig` is happy, but my understanding of rfc9461 and section 1.2. "Levels and Expression Types" of rfc6570 is that `{&dns}` is valid.
See for example section 1.2. "Levels and Expression Types" of rfc6570.

Note that Peter van Dijk suggested that `{dns}` and `{dns,someothervar}` might be valid forms as well, so my patch might be too restrictive, although it's anyone's guess how DoH clients would handle complex templates.

Closes https://gitlab.isc.org/isc-projects/bind9/-/issues/4922

Backport of MR !9455

Merge branch 'backport-svcb-dohpath-uri-template-9.18' into 'bind-9.18'

See merge request isc-projects/bind9!9770
This commit is contained in:
Mark Andrews
2024-11-26 05:22:09 +00:00
3 changed files with 212 additions and 25 deletions

View File

@@ -25,6 +25,7 @@
#include <isc/print.h>
#include <isc/result.h>
#include <isc/string.h>
#include <isc/utf8.h>
#include <isc/util.h>
#include <dns/callbacks.h>
@@ -602,6 +603,176 @@ typemap_test(isc_region_t *sr, bool allow_empty) {
static const char hexdigits[] = "0123456789abcdef";
static const char decdigits[] = "0123456789";
/*
* 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

View File

@@ -154,28 +154,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}".
*/
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}" */
if (strnstr((char *)region->base, "{?dns}",
region->length) == NULL)
{
if (!validate_dohpath(region)) {
return DNS_R_FORMERR;
}
break;

View File

@@ -2609,13 +2609,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_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()
};