From: TCY16 <tom@nlnetlabs.nl>
Date: Wed, 24 Aug 2022 10:38:08 +0000 (+0200)
Subject: implement @gthess' review comments; fix check on compulsory text and add tests
X-Git-Tag: release-1.18.0rc1~24^2~18^2~6
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f3fa3634431806759d61470458fef123dbf1f1f8;p=thirdparty%2Funbound.git

implement @gthess' review comments; fix check on compulsory text and add tests
---

diff --git a/sldns/str2wire.c b/sldns/str2wire.c
index 9f1fb0fca..90648245b 100644
--- a/sldns/str2wire.c
+++ b/sldns/str2wire.c
@@ -1525,6 +1525,7 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 	uint8_t* rd, size_t* rd_len)
 {
 	size_t val_len;
+	char* open_bracket, * close_bracket, * expr_ptr;
 
 	/* RFC6570#section-2.1
 	 * "The characters outside of expressions in a URI Template string are
@@ -1542,8 +1543,17 @@ sldns_str2wire_svcbparam_dohpath_value(const char* val,
 	/* draft-ietf-add-svcb-dns-06#section-5.1
 	 * The URI Template MUST contain a "dns" variable
 	 */
-	if (!(strstr(val, "?dns"))) {
-		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
+	open_bracket = strchr(val, '{');
+	close_bracket = strchr(val, '}');
+
+	if (!open_bracket && !close_bracket) {
+		return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;	
+	} else {
+		expr_ptr = strstr(open_bracket+1, "?dns");
+
+		if (!expr_ptr || !((close_bracket - expr_ptr) >= 4 ) ) {
+			return LDNS_WIREPARSE_ERR_SVCB_NO_DNS_VAR_IN_DOHPATH;
+		}
 	}
 
 	sldns_write_uint16(rd, SVCB_KEY_DOHPATH);
diff --git a/testdata/svcb.tdir/svcb.failure-cases-05 b/testdata/svcb.tdir/svcb.failure-cases-05
new file mode 100644
index 000000000..67246c954
--- /dev/null
+++ b/testdata/svcb.tdir/svcb.failure-cases-05
@@ -0,0 +1,8 @@
+$ORIGIN failure-cases.
+$TTL 3600
+
+@       SOA     primary admin 0 0 0 0 0
+
+; Dohpath must have '?dns' in the URI template variable
+
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?d}
\ No newline at end of file
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone b/testdata/svcb.tdir/svcb.success-cases.zone
index d63456369..f625c6c5e 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone
+++ b/testdata/svcb.tdir/svcb.success-cases.zone
@@ -50,3 +50,4 @@ s09     HTTPS   0 . ( alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath="/dns-query{?dns}"
 _dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?dns}
+_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-queryéè{?dns}
diff --git a/testdata/svcb.tdir/svcb.success-cases.zone.cmp b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
index 545333ba8..9075cc6b5 100644
--- a/testdata/svcb.tdir/svcb.success-cases.zone.cmp
+++ b/testdata/svcb.tdir/svcb.success-cases.zone.cmp
@@ -9,4 +9,5 @@ s07.success-cases.	3600	IN	HTTPS	0 . ech="aGVsbG93b3JsZCE="
 s08.success-cases.	3600	IN	HTTPS	0 . key11="a" key12="a" key13="a" key14="a" key15="a" key16="a" key17="a" key18="a" key19="a" key110="a" key111="a" key112="a" key113="a" key114="a" key115="a" key116="a" key117="a" key118="a" key119="a" key120="a" key121="a" key122="a" key123="a" key124="a" key125="a" key126="a" key127="a" key128="a" key129="a" key130="a" key131="a" key132="a" key133="a" key134="a" key135="a" key136="a" key137="a" key138="a" key139="a" key140="a" key141="a" key142="a" key143="a" key144="a" key145="a" key146="a" key147="a" key148="a" key149="a" key150="a" key151="a" key152="a" key153="a" key154="a" key155="a" key156="a" key157="a" key158="a" key159="a" key160="a" key161="a" key162="a" key163="a"
 s09.success-cases.	3600	IN	HTTPS	0 . alpn="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
 _dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
-_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
\ No newline at end of file
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"
+_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query\195\169\195\168{?dns}"
diff --git a/testdata/svcb.tdir/svcb.test b/testdata/svcb.tdir/svcb.test
index 17330e08f..88a9e95ff 100644
--- a/testdata/svcb.tdir/svcb.test
+++ b/testdata/svcb.tdir/svcb.test
@@ -75,6 +75,12 @@ then
 	echo "Failure case 04: 256 is too many characters for an alpn; maximum is 255"
 	echo "Incorrectly succeeded"
 	exit 1
+
+elif $PRE/readzone svcb.failure-cases-05
+then
+	echo "Dohpath must have '?dns' in the URI template variable"
+	echo "Incorrectly succeeded"
+	exit 1
 else
 	echo "All failure cases test successfully"
 fi