Reviewer: Dale Worley Review result: Ready with Issues I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at https://trac.ietf.org/trac/gen/wiki/GenArtfaq Document: draft-ietf-dnsop-svcb-https-07 Reviewer: Dale R. Worley Review Date: 2021-08-14 IETF LC End Date: 2021-08-19 IESG Telechat date: unknown Summary: The document is generally in quite good shape and is almost ready for publication, but it has a technical point that should be reviewed and some editorial issues. Technical issues: Whereas SRV records have both a priority and weight field, SVCB records have only a priority field. (This point is not addressed in section C.1.) This point should be reviewed to ensure that is what we want. It may be that the weight field has not proven useful in practice, but my concern is that people may mistakenly think that implementing weight is complex and left it out of SVCB because of that. In fact, SRV records processed in the following way produce results with the specified statistical properties: - Assign each record effective_weight = - weight * log(uniform_random(0, 1)) - Sort the records first by priority in ascending order then by effective_weight in descending order. You may want to add a requirement that all SVCB-compatible RR types have presentation formats that are identical to SVCB (except for the RR type name). (see comments on section 8) Major editorial issue: While I am sure the details of the relationship between SVCB and HTTPS are given correctly, the text isn't as clear as it should be regarding the meaning and requirements of "SVCB-compatible RR types". It would be an improvement to: - Add a section that lays out the rules for SVCB-compatible RR types. - Move/copy the 4th and 5th paragraphs of the Introduction to the new section. - Clarify that the term "SVCB" is used to mean (1) the SVCB RR type, (2) collectively, SVCB and all SVCB-compatible RR types (via the sentence "All behaviors described as applying to the SVCB RR also apply to the HTTPS RR unless explicitly stated otherwise."), and (3) the SVCB conceptual structure (e.g. App. B "a non-normative summary of the HTTP mapping for SVCB"). This should probably be done in the Terminology section, where oddly "SVCB" is mentioned but not specifically listed as a defined term. - "SVCB-compatible" should be listed as a defined term. - Instead of defining "SVCB-compatible" by the example of HTTPS, identify the statements about HTTPS that are applicable to all SVCB-compatible records and state them as such. In particular, revise "All behaviors described as applying to the SVCB RR also apply to the HTTPS RR unless explicitly stated otherwise." - Add a note that when the mapping is first established for a protocol, a choice must be made whether it uses SVCB RRs or a new SVCB-compatible RRs. But once that choice is made, it is impossible (or very difficult) to change it in an upward-compatible way. The current document reads as if HTTPS was devised well into the process based on its ability to reduce the number of DNS queries, and then late in the process it was realized that HTTPS can be generalized to SVCB-compatible, but the document wasn't edited to fully describe "SVCB-compatible". Minor editorial issues: 1. Introduction Some of the text here regarding SVCB-compatible records as a whole seems to be normative (in that it is not stated anywhere else). But that has been covered in the editorial point above. It's may worth mentioning here that while the document defines SVCB and then derives HTTPS from it, the document defines only the mapping for the http: and https: schemes, and defines them as using the HTTPS record. So there are at this point no schemes with mappings for SVCB proper. 1.1. Goals of the SVCB RR This is important, as DNS does not provide any way to tie together multiple RRs for the same name. This is ambiguous, as of course DNS ties together all of the RRs for the same name. Perhaps: This is important, as DNS does not provide any way to specify multiple groups of RRs for the same name. 1.4. Terminology This is usually section 2, which clarifies that it is normative, in that the definitions are needed to understand the normative statements in the rest of the document. 2.2. RDATA wire format When the list of SvcParams is non-empty (ServiceMode) Actually, an AliasMode record can have a non-empty SvcParams, it simply SHOULD NOT. The subtle case is whether an AliasMode record with non-empty SvcParams is governed by this section (which requires the SvcParams to be properly formatted and "If any RRs are malformed, the client MUST reject the entire RRSet") or by section 2.4.2 ("In AliasMode, ... recipients MUST ignore any SvcParams that are present.") I recommend stiffening the requirement so that AliasMode records MUST NOT have SvcParams (and having the zone file processor enforce it). * a 2 octet field containing the length of the SvcParamValue as an integer between 0 and 65535 in network byte order (but constrained by the RDATA and DNS message sizes). It seems redundant to include the parenthesized text, as the same requirement is applied in the 3rd bullet point of this section. * an octet string of this length whose contents are in a format determined by the SvcParamKey. More exactly, "... whose contents are the SvcParamValue in a format determined by the SvcParamKey." 2.3. SVCB query names When a prior CNAME or SVCB record has aliased to a SVCB record, each RR shall be returned under its own owner name. Oddly, I've not heard "owner" applied to a DNS record before, but "owner" is defined in [DNSTerm]. (Parentheses are used to ignore a line break ([RFC1035], Section 5.1).) I would add "... in DNS RR presentation format". 3. Client behavior This could be made clearer by rearranging the information in these paragraphs to: This procedure does not rely on any recursive or authoritative DNS server to comply with this specification or have any awareness of SVCB. [this paragraph is unchanged] A client is called "SVCB-optional" if it can connect without the use of ServiceMode records, and "SVCB-reliant" otherwise. Clients for pre-existing protocols (e.g. HTTPS) SHALL implement SVCB-optional behavior (except as noted in Section 3.1 and Section 9.1). SVCB-optional clients SHOULD issue in parallel any other DNS queries that might be needed for connection establishment using non-SVCB connection modes. Once SVCB resolution has concluded, whether successful or not, SVCB-optional clients SHALL append to the list of known endpoints an endpoint consisting of the final value of $QNAME, the authority endpoint's port number, and no SvcParams. (This endpoint will be attempted before falling back to non-SVCB connection modes. This ensures that SVCB-optional clients will make use of an AliasMode record whose TargetName has A and/or AAAA records but no SVCB records.) The client proceeds with connection establishment via SVCB's list of known endpoints, if any. Clients SHOULD try higher-priority alternatives first, with fallback to lower-priority alternatives. Clients issue AAAA and/or A queries for the selected TargetName, and MAY choose between them using an approach such as Happy Eyeballs [HappyEyeballsV2]. If the client is SVCB-optional and connecting using the list of known endpoints failed, it then attempts non-SVCB connection modes. [continue with "Some important optimizations..."] 3.1. Handling resolution failures If SVCB resolution fails due to [...] This condition isn't actually defined in section 3. A clearer formulation is: If any DNS query specified in the SVCB resolution process fails due to a SERVFAIL error, transport error, or timeout, and DNS exchanges between the client and the recursive resolver are cryptographically protected (e.g. using TLS [DoT] or HTTPS [DoH]), a SVCB client SHOULD abandon the connection even if it is SVCB-optional. 3.2. Clients using a Proxy Providing the proxy with the final TargetName has several benefits: Somewhat clearer as "with the final TargetName rather than its IP address". 4.2. Recursive resolvers Recursive resolvers that are aware of SVCB SHOULD help the client to execute the procedure in Section 3 with minimum overall latency, by incorporating additional useful information into the response. For the initial SVCB record query, this is just the normal response construction process (i.e. unknown RR type resolution under [RFC3597]). For followup resolutions performed during this procedure, we define incorporation as adding all useful RRs from the response to the Additional section without altering the response code. Upon receiving a SVCB query, recursive resolvers SHOULD start with the standard resolution procedure, and then follow this procedure to construct the full response to the stub resolver: The wording could be clarified: Recursive resolvers that are aware of SVCB SHOULD help the client to execute the procedure in Section 3 with minimum overall latency by incorporating additional useful information into the response. The normal response construction process (i.e. unknown RR type resolution under [RFC3597]) generates the Answer section of the query. Upon receiving a SVCB query, recursive resolvers SHOULD start with the standard resolution procedure, and then follow this procedure to obtain additional records to incorporate into the Additional section: This provides the definition of "incorporate" which is used in the following clauses. -- 1. Incorporate the results of SVCB resolution. If the chain length limit has been reached, terminate successfully (i.e. a NOERROR response). Shorten this to: 1. Incorporate the results of SVCB resolution. If the chain length limit has been reached, terminate. -- After this: In this procedure, "resolve" means the resolver's ordinary recursive resolution procedure, as if processing a query for that RRSet. This includes following any aliases that the resolver would ordinarily follow (e.g. CNAME, DNAME [DNAME]). add: In all cases, errors or anomalies in obtaining additional records MAY cause this process to terminate, but MUST NOT themselves cause the resolver to send a failure response. 4.3. General requirements No part of this specification requires recursive resolvers to alter their behavior based on its contents, even if the contents are invalid. It seems like this sentence is redundant and could be omitted. But better would be to start it "This specification does not require ...". Recursive resolvers MAY validate the values of recognized SvcParamKeys and reject records containing values which are invalid according to the SvcParam specification. It might be better to s/reject/ignore/, because "reject" seems to imply a specific error response toward some source of the records, and resolvers don't do that. 8. Using SVCB with HTTPS and HTTP Some of this information is actually part of the general structure of SVCB-compatible records and should be moved to the general discussion of SVCB-compatibility. Clients MUST NOT perform SVCB queries or accept SVCB responses for "https" or "http" schemes. This is true of a client resolving any scheme whose mapping provides a separate RR type. The HTTPS RR wire format and presentation format are identical to SVCB, and both share the SvcParamKey registry. SVCB semantics apply equally to HTTPS RRs unless specified otherwise. The presentation format of the record is: Name TTL IN HTTPS SvcPriority TargetName SvcParams All of the above applies to any SVCB-compatible RR, and so should be documented as such -- except the presentation format requirement. But for sanity's sake, SVCB-compatibility should require that the presentation format be the same as for SVCB (except for the RR type name). 8.1. Query names for HTTPS RRs Reusing the service name also allows ... I assume this means "Using one record for both HTTP and HTTPS allows ..." 10.1. Structuring zones for flexibility Each ServiceForm RRSet can only serve a single scheme. Is this "ServiceMode"? Appendix B. HTTP Mapping Summary This table does not indicate any SvcParamKeys that servers are required to publish, or that clients are required to implement, because there are none in this mapping. It would be better to show these two facts as entries, as other mappings may need to give values, and putting entries in the one existing example will help them remember that: ... +--------------------------+----------------------+ | *Special behaviors* | HTTP to HTTPS | | | upgrade | +--------------------------+----------------------+ | *SvcParamKeys servers | none | | must publish* | | +--------------------------+----------------------+ | *SvcParamKeys clients | none | | must implement* | | +--------------------------+----------------------+ I'm not sure if the asterisks around the items in the first column add any value. 12. Security Considerations A hostile DNS intermediary might forge AliasForm "." records This paragraph uses "AliasForm" twice, which should probably be "AliasMode". D.1. AliasForm Is this "AliasMode"? D.2. ServiceForm Is this "ServiceMode"? This section uses "vector" in three places where "form" or "example" would likely be better. [END] -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
