Adds SVCB parameter key 7 (dohpath) #547
Conversation
|
Thank you for the code! This looks like it implements the dohpath option for SVCB and HTTPS fine. The document is still a draft, and because we do not want to create a too-early deployment issue, we do not want to ship this at this early stage. It would be best if we waited for discussion at the IETF to complete. Meanwhile, for experimentation there can be code. The code from the diff of this pullrequest can perhaps help people that want it. Deploy by applying the patch at https://github.com/NLnetLabs/unbound/pull/547.diff and then dohpath should work. Another way, but perhaps later, is to add compile time switches that default to off, that turn on this new feature. |
There was a problem hiding this comment.
Hi @okanaganrusty,
Thanks again for the code. I found two small changes, but overall it looks very nice :)
| } | ||
|
|
||
| dst_len = sldns_str2wire_svcbparam_parse_copy_unescaped(dst, val, val_len); | ||
|
|
There was a problem hiding this comment.
This write could result in a buffer overflow, as the rd_len isn't checked before writing the data. It's missing:
| if (*rd_len < 4 + dst_len) | |
| return LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL; |
| @@ -8,3 +8,4 @@ s06.success-cases. 3600 IN HTTPS 0 . ech="aGVsbG93b3JsZCE=" | |||
| 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" | |||
| s10.success-cases. 3600 IN HTTPS 0 . alpn="h2" ipv4hint="127.0.0.1" ipv6hint="fe80::1" dohpath="/dns-query?{dns}" | |||
There was a problem hiding this comment.
This test seems to fail currently. Removing the quotes around the ipv*hints fixes this.
| s10.success-cases. 3600 IN HTTPS 0 . alpn="h2" ipv4hint="127.0.0.1" ipv6hint="fe80::1" dohpath="/dns-query?{dns}" | |
| s10.success-cases. 3600 IN HTTPS 0 . alpn="h2" ipv4hint=127.0.0.1 ipv6hint=fe80::1 dohpath="/dns-query?{dns}" |
|
Adressed by #739 |
|
This seems to be addressed by #739 which is now merged; closing this. |
PR to add support for
dohpathin the SVCB request.Supporting issue #501
https://datatracker.ietf.org/doc/html/draft-schwartz-svcb-dns-00#section-5.1