Reviewer: Benjamin Schwartz Review result: Has Issues I think there are some interesting open questions about the structure of this document, and some details about how recommendations are described, but the technical components are sound. Section 1: > When DNS clients wish to use encrypted DNS protocols such as DNS-over-TLS (DoT) [RFC7858], DNS-over-QUIC (DoQ) [RFC9250], or DNS-over-HTTPS (DoH) [RFC8484], they require additional information beyond the IP address of the DNS server, such as the resolver's hostname, non-standard ports, or URI templates. This is arguably true, but none of these things are relevant in the simplest cases for DoT and DoQ using IP addresses (Section 4). In those cases, the purpose of this specification is simply to indicate whether the resolver supports DoT or DoQ. That probably bears mentioning here, ahead of the less common metadata like nonstandard ports. > This document defines two mechanisms I think this is probably a mistake. The fact that there are two very different mechanisms defined here, both known as DDR, has already generated a fair bit of confusion. The second mechanism (Section 5) also seems largely to be repeating the recommendations in draft-ietf-svcb-dns. Given that these two drafts are entering review together, it might make sense to reconsider how material is split between them. Ideally, I think we would adjust the document so that "DDR" means "query _dns.resolver.arpa", as that is what everyone seems to think it means. Section 3: Most of this text seems redundant with draft-ietf-svcb-dns. I understand the desire to avoid making readers read both documents, but some of the text seems especially duplicative, e.g. > If the client encounters a mandatory parameter in an SVCB record it does not understand, it MUST NOT use that record to discover a Designated Resolver. This is repeating the normative definition of "mandatory" from draft-ietf-dnsop-svcb-https. If it must be repeated, the repetition could be flagged (e.g. 'As required by Section 8 of ...'). Section 4: > When a DNS client is configured with an Unencrypted Resolver IP address, it SHOULD query the resolver for SVCB records for the name "resolver.arpa" before making other queries. Specifically, the client issues a query for _dns.resolver.arpa ... This seems contradictory at a glance. It might be more correct to say '... for SVCB records of a service with a scheme of "dns" and an Authority of "resolver.arpa" ...'. Also, SVCB-DNS defines the term "Binding Authority" specifically to enable unambiguous specification of DDR, but DDR doesn't use this term. > Because this query is for an SUDN, which no entity can claim ownership over, the ServiceMode SVCB response MUST NOT use the "." value for the TargetName. This rationale is mysterious to me. We've just told the resolver that they can place SVCB records on "_dns.resolver.arpa", so it's not at all clear why they can't place AAAA records there as well, and this is all that TargetName means in SVCB. It also doesn't quite address the question of whether TargetName can be "resolver.arpa", which would be more natural in SVCB. Personally, I don't see the need for this restriction, which I think is a holdover from previous drafts that imposed different semantics on TargetName. Placing AAAA records on "resolver.arpa" seems like a fine way for the resolver to tell the client what it thinks its own public ingress IPs are. If the authors think this should be prohibited, perhaps they can clarify the rationale (e.g. 'To enable reliable identification of the Designated Resolver for debugging and future extensions, TargetName SHOULD NOT indicate a SUDN.'). Relatedly, if "resolver.arpa" is not the expected TargetName, this section should probably have a sentence noting that the "speculative A and/or AAAA queries" recommended in Section 5 of draft-ietf-svcb-https are not applicable here. > If the recursive resolver that receives this query has no Designated Resolvers, it SHOULD return NODATA for queries to the "resolver.arpa" SUDN. I think this should say something like 'for queries to the "resolver.arpa" zone'. Note that NXDOMAIN means there is nothing underneath (RFC 8020), but NODATA does not. Section 4.1: > A client MUST NOT re-use a designation ... This recommendation is obscure. Perhaps an example of a plausible violation would help. Section 4.1.1: > having no delay in establishing an encrypted DNS connection This seems a bit optimistic. For DoQ with resumption "no delay" might be possible, but for DoT a delay of at least 1 RTT is required. Perhaps 'no delay before establishing' would be more precise. Section 4.2: Perhaps cite RFC5280 for subjectAltName? > Additionally, the client SHOULD suppress any further queries for Designated Resolvers using this Unencrypted Resolver for the length of time indicated by the SVCB record's Time to Live (TTL). Why? I agree that the client shouldn't retry in a fast loop, but consider a passive attacker who can occasionally become active via some difficult mechanism (e.g. a Kaminsky-type attack). If this attacker can inject a single DDR record with a very long TTL, it can disable DDR passively monitor the DNS queries for a long time. It would be better for the client to retry SVCB resolution more frequently than the (potentially attacker-controlled) TTL, to force an active attacker to be more persistent. > If resolving the name of a Designated Resolver from an SVCB record I think this should say 'TargetName'. (This is not "the name of a Designated Resolver" under the standard interpretation of TargetName.) > yields an IP address that was not presented in the Additional Answers section or ipv4hint or ipv6hint fields of the original SVCB query, the connection made to that IP address MUST pass the same TLS certificate checks before being allowed to replace a previously known and validated IP address for the same Designated Resolver name. I find this confusing. There are potentially four classes of IP addresses involved here: A. The IP of the unencrypted resolver, provided out of band (e.g. via DHCP) B. ipv4hint/ipv6hint C. Additional Section records for TargetName D. Answer Section records for TargetName. This text makes it sound like A-C or B+C have something in common that D does not, but that is not true. In fact, only A is treated specially, and B-D are completely equivalent for DDR. Maybe this text is trying to warn against some implementation mistake? If so, better to make that explicit. Section 4.3: > the SVCB record for "resolver.arpa" I find this confusing, since the SVCB record is at "_dns.resolver.arpa", not "resolver.arpa". (Really, it's the SVCB record for "dns://resolver.arpa", but "dns:" URIs are controversial at best.) Section 5: As discussed earlier, I think it might be better to shrink or eliminate this section. Section 6.1 > A DNS forwarder SHOULD NOT forward queries for "resolver.arpa" upstream. '"resolver.arpa" and subdomains'? Are we sure this advice applies to all future uses of the "resolver.arpa" zone? > ... or that the operator expects clients of the unencrypted resolver to use the SVCB information opportunistically. This seems to contradict Section 4.3 (Opportunistic Discovery), which prohibits the use of the SVCB information in this configuration due to differing IPs of the local and upstream resolvers. Perhaps this should match Section 4.1's language about "implementation-specific policy" and "future mechanism", e.g. "... expects clients to validate the connection via some future mechanism". Section 7: Typo: "provides provides" > once a client discovers a compatible Designated Resolver, it SHOULD NOT use unencrypted DNS until the SVCB record expires, unless verification of the resolver fails. I think the caveat needs to be removed. Otherwise, an attacker on the transport path can cause verification to fail, in order to trigger unencrypted fallback. > DoH resolvers that allow discovery using DNS SVCB answers over unencrypted DNS MUST NOT provide differentiated behavior based on the HTTP path alone ... I think the language on this in draft-ietf-add-svcb-dns is more precise and complete. For example, this description does not cover alternate port numbers, and the use of "separate resolver IP addresses" is unclear (it means "separate _unencrypted_ resolver IP addresses"). The language here should probably match, or be removed and rely on the other draft. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
