Ballot for draft-ietf-add-svcb-dns
Yes
No Objection
Note: This ballot was opened for revision 05 and is now closed.
# Internet AD comments for {draft-ietf-add-svcb-dns-06} CC @ekline ## Nits ### S3.1 * "in an additional a prefix" -> "in an additional prefix", perhaps, or maybe "in an additional attrleaf prefix" ### S4.2 * "that it trusts not attempt" -> "that it trusts not to attempt", perhaps
# SEC AD comments for draft-ietf-add-svcb-dns-06 CC @paulwouters ## Comments ### SVCB record names (i.e., QNAMEs) for DNS services This should probably say "for nameserver services", as it is not meant for any and all DNS services (eg a DNS service presenting a JSON API to submit DNS record updates, or the DNS visualization service on dnsviz.net) This applies to further use of "DNS services" throughout the document. ### Clients SHOULD NOT query for any "HTTPS" RRs when using "dohpath". Instead, the SvcParams and address records associated with this SVCB record SHOULD be used for the HTTPS connection Why are these SHOULD NOT and SHOULD not MUST NOTs and MUST ? ### In section 7 examples, why "fooexp" and not "fooexp.example." ?
Thank you for the work on this document. Many thanks to Martin Dürst for his ART ART review: https://mailarchive.ietf.org/arch/msg/art/2tLphDE7pymmSMBtcQ2VSBpONn8/. Authors: please consider Martin's comments (also reported below) which would improve readability - in particular, expanding on uses and context would help a non-expert reader. Thanks, Francesca -- Reviewer: Martin Dürst Review result: On the Right Track I'm the assigned reviewer for the App Area for the draft "Service Binding Mapping for DNS Servers" (draft-ietf-add-svcb-dns). I have mainly reviewed -05, but also checked the diff to -06. This is an app area review, and so my concerns are mostly from an application perspective. Summary: As far as I was able to tell (not an expert on DNS, and didn't check [SVCB], which is required reading for implementers), the document is technically okay. But I'm not sure readers will understand where to use this information. Major: The main issue with the document is that there is no explanation on who/where/when this information is to be queried and used. Is this the job of an application (e.g. a web browser or an email user agent)? Is this the job of a resolver in an OS? Is it the job of DNS libraries, e.g. in programming languages such as Ruby and Python? Who/what decides which of the alternatives is used if there are multiple? How should (or shouldn't) DNS queries for SVCB records be combined with queries for other records? While I understand that there may be many different contexts, a pointer to a document describing various potential scenarios or so could help a lot. The addition of a bullet list to section 6, Limitations, is an improvement, but it would be way better if there were more such information, and if that information were worded in a positive way, and if that information were given upfront, or if there were at least a pointer up front to such information. Another idea would be a separate document explaining the possibilities and giving recommendations or proposals. Also, while the above considerations are mainly on the using side, some additional considerations for the publishing side are also desirable. Minor: Section 4.2: "This key is automatically mandatory for this binding. This means that a client that does not respect the "port" key MUST ignore any SVCB record that contains this key. (See Section 7 of [SVCB] for the definition of "automatically mandatory".)" Summary: "Mandatory means MUST ignore" -> This just doesn't make sense. Please improve the wording so that it becomes clear what you want to say on first reading. Section 4.3: "Future SvcParamKeys might also be applicable.": It's totally unclear HOW they might (or might not) be applicable. Section 5.1 ""dohpath" is a single-valued SvcParamKey whose value (both in presentation and wire format) MUST be a URI Template in relative form ([RFC6570], Section 1.1) encoded in UTF-8 [RFC3629].": It would be good to add that this essentially makes the URI Template an IRI [RFC3987] Template. Nits: Section 3.1: "places the port number in an additional *a* prefix" -> "places the port number in an additional prefix" Section 4.1: "All keys specified for use with the HTTPS record": Please add a reference here. Section 4.2: "The client is being used with a DNS server that it trusts not attempt this attack." -> "The client is being used with a DNS server that it trusts not _to_ attempt this attack." Reference [Attrleaf] is an RFC, but is the only RFC that uses a name rather than an RFC number as the reference label. Please streamline. Regards, Martin.
I support Lars's DISCUSS. While I'm at it, a few points on your IANA Considerations: * It's common to group each registry's actions into their own separate subsections. * The first registry you're updating is called "Service Parameter Keys", and the "Change Controller" field is missing. * The second registry you're updating is called "Underscored and Globally Scoped DNS Node Names". Some exposition around that SHOULD and NOT RECOMMENDED at the end of Section 6 might be helpful. When might an implementer legitimately decide to do something different?
Thank you to Joseph Salowey for the SECDIR review. This review noted a few promised changes that were merged into -06. Please merge those in. See https://mailarchive.ietf.org/arch/msg/secdir/kpqucIMv2LCibuUqoK42LhJKyKM/ ** Section 8.1.2 If an endpoint sends an invalid response to a DNS query, the client SHOULD NOT send more queries to that endpoint. Consider the following additional behavior: NEW … the client SHOULD NOT send more queries to that endpoint and MAY log this error.
# GEN AD review of draft-ietf-add-svcb-dns-06 CC @larseggert Thanks to Peter E. Yee for the General Area Review Team (Gen-ART) review (https://mailarchive.ietf.org/arch/msg/gen-art/T4D0gKTp6m_TFw0C26Ib-7PqttY). ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT]. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments [IRT]: https://github.com/larseggert/ietf-reviewtool