Dale, thank you for your review. I have entered a No Objection ballot for this document. Lars > On 2021-8-14, at 13:34, Dale Worley via Datatracker <noreply@xxxxxxxx> wrote: > > 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
Attachment:
signature.asc
Description: Message signed with OpenPGP
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
