Early Review of draft-ietf-add-resolver-info-02
review-ietf-add-resolver-info-02-dnsdir-early-stenstam-2023-06-30-00

This draft is well written and easy to understand. Don’t be concerned about the
number of nits below. Most of them are trivial typos. There is one clear
suggestion for a change. It is obviously entirely your call whether you agree
with the suggestion or not, it's just a suggestion.

Note: I understand that you may want to discuss some of the comments below, and
preferably before the submission deadline. However, I'm sorry to say that I
will be completely offline (hiking mountains) beginning tomorrow morning.
Hence, please do not feel ignored if I don't respond, it is simply that I will
not have seen the message.

Regards,
Johan

General:

* There is a bit of terminology fuzz that could be improved. Eg. instead of
sometimes talking about “recursive resolver” and in other cases “DNS resolver”,
it would be better to use the same term throughout the document.

Suggestion:

* In Section 6: "Security Considerations" I read:

  “or use local DNSSEC validation to retrieve the resolver information”.

  Question: for the stub resolver to be able to do DNSSEC validation it
  typically needs to either not be a mere stub resolver (i.e. it is a full
  resolver itself; no real need for a full resolver) or have access to some
  other DNSSEC-validating recursive server.

  I suggest expanding this section a bit with clearer scenarios. A better
  alternative may be to put such use cases or examples elsewhere in the
  document to give them sufficient attention and also to avoid complicating the
  Security Section.

Nits:

Section 1: Introduction

* “The options … can be deployed … deployments… “

I strongly suggest breaking up this four-line sentence into multiple shorter
sentences (and perhaps rephrase one of the uses of “deployment”).

* "Retrieved information can be used to feed the server selection procedure.
However, that selection procedure is out of scope.”

I understand that it is implicit “… of this document”, but I suggest spelling
that out to make it clear that it is not a question of being out of scope of
the WG.

Section 2: Terminology

* I suggest removing a couple of spurious whitespace characters: "DNS-
over-HTTPS” —> "DNS-over-HTTPS”
  "DNS- over-QUIC" —> "DNS-over-QUIC”

Section 3: Retrieving Resolver Information

* A word is missing here:

  "The content of the RDATA in a response to RR type query is defined in
  Section 5.”

  should be:

  "The content of the RDATA in a response to a RESINFO RR type query is defined
  in Section 5.”

* Third paragraph: suggest replacing “DNS server” with more precise terminology.

* I also suggest expanding the last two paragraphs (perhaps with an example) to
make them easier to understand. I do understand that the set of documents are
meant to be read together, but it is still important to make each document as
understandable as possible.

Section 5: Resolver Information Keys/Values

* Typo: the key “infourl” is called “resinfourl” in the example.

Section 6: Security Considerations

* To minimise the risk of confusion (on the part of the reader, like myself) I
suggest replacing “DNS server” (unfortunately always a fuzzy term) with
“recursive resolver” (or “DNS resolver”, depending on which term is chosen).

* [See also the suggestion above]

Section 7.2:

* For the key “exterr” I believe that the value type should be “comma separated
list of integers” rather than “number”.

* I also suggest that the Description is changed from "Lists the set of
extended DNS errors” to “List of
  SUPPORTED extended DNS errors”.

* The description of the “infourl” key is currently “Provides an unstructured
resolver information…”. I think this is wrong. The infourl is not unstructured,
it is a URL (which is structured), but the URL *points to* a set of
unstructured information located elsewhere. To some extent this is perhaps
semantic nitpicking, but it would be good if the text could be made more clear
without overly complicating everything.