Re: [Gen-art] [Last-Call] Genart last call review of draft-ietf-dnsop-svcb-https-07

[Lars Eggert <[e-mail]>]

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 <[e-mail]> 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
> [e-mail]
> https://www.ietf.org/mailman/listinfo/last-call