Last Call Review of draft-ietf-doh-dns-over-https-12
review-ietf-doh-dns-over-https-12-genart-lc-bryant-2018-08-04-01

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-doh-dns-over-https-12
Reviewer: Stewart Bryant
Review Date: 2018-08-04
IETF LC End Date: 2018-08-08
IESG Telechat date: Not scheduled for a telechat

Summary:
Adding Summary. The rest of the comments are identical to my previous review.

This is useful work, but lacks a lot of detail needed by those that are not
pre-existing specialists in the technologies discussed, and even to specialists
there is a need to reverse engineer the specification through a study of the
hex encoding to be certain of the design. Fixing this is easy, but I think it
would be helpful to the reader if it were fixed.

I also think that some more detail is needed in the area of IPv4 vs IPv6.

ID-Nits picks up a downref, but the appropriate warning was included in the
IETF LC email.

4.  Selection of DoH Server

SB> I am surprised that there is no discussion of the need to specify
SB> the raw IP address of the sever as part of the URI, and that there
SB> is no discussion of the server address IP address family in the document.
SB> I would have thought that there should at least be a reference to
SB> "happy eyeballs" and and discussion of an  optimization strategy of
SB>  what to do if there was a response on Ipv4 and not IPv6 etc etc.

==========

  The same DNS query for www.example.com, using the POST method would
   be:

   :method = POST
   :scheme = https
   :authority = dnsserver.example.net
   :path = /dns-query
   accept = application/dns-message
   content-type = application/dns-message
   content-length = 33

   <33 bytes represented by the following hex encoding>
   00 00 01 00 00 01 00 00  00 00 00 00 03 77 77 77
   07 65 78 61 6d 70 6c 65  03 63 6f 6d 00 00 01 00
   01

SB> At this point in the text  am at a loss to understand the 33 bytes.
SB> Having read to the end of the text, I assume that this is a binary encoded
SB> DNS query in "one the wire format" but this point needs to be made clear,
SB> as does the explicit encoding. It would be really useful if there was an
SB> example of both formats side by side.

=========
   The DNS query is 94 bytes represented by the following hex encoding
SB> Again in what format?

=========
   :method = GET
   :scheme = https
   :authority = dnsserver.example.net
   :path = /dns-query? (no space or CR)
           dns=AAABAAABAAAAAAAAAWE-NjJjaGFyYWN0ZXJsYWJl (no space or CR)
           bC1tYWtlcy1iYXNlNjR1cmwtZGlzdGluY3QtZnJvbS1z (no space or CR)
           dGFuZGFyZC1iYXNlNjQHZXhhbXBsZQNjb20AAAEAAQ
   accept = application/dns-message

SB> I am not a DNS or HTTP specialist, so please help me understand
SB> what the string starting "dns=" is.

==========

5.2.2.  HTTP Response Example

SB> Nits says:

  -- The document has examples using IPv4 documentation addresses according
     to RFC6890, but does not use any IPv6 documentation addresses.  Maybe
     there should be IPv6 examples, too?

SB> I am not sure if there are any IPv4 vs IPv6 issues but it would be helpful
to SB> clarify the point.

=========

   <64 bytes represented by the following hex encoding>
   00 00 81 80 00 01 00 01  00 00 00 00 03 77 77 77
   07 65 78 61 6d 70 6c 65  03 63 6f 6d 00 00 01 00
   01 03 77 77 77 07 65 78  61 6d 70 6c 65 03 63 6f
   6d 00 00 01 00 01 00 00  00 80 00 04 C0 00 02 01

SB> At this point in the text I am still wondering what this encoding means, I
assume that SB> it is a hex representation of the payload received SB> back
from a normal DNS query.

=========

7.  Definition of the application/dns-message media type

   The data payload for the application/dns-message media type is a
   single message of the DNS on-the-wire format defined in Section 4.2.1
   of [RFC1035].

SB> That would have been useful to have known earlier
SB> However 4.2.1 just talks about UDP and not the message syntax.
SB> Also presumably we are talking about the payload of the UDP
SB> message excluding the UDP header.

SB> "on-the-wire" is a potentially confusing term, since it gets us
SB> into network byte order etc.
SB> I think the whole thing  would be clearer is you said that your
SB> message was identical to the payload of the UDP payload of an
SB> RFC1035 as seen by a server (and then add some specification
SB> about the payload to hex method you are using)
SB> It would also be helpful to the reader to note whether
SB> any of the 25 updates to RFC1035 impact any of the specification
SB> described in this memo.

=======

   The format was originally for DNS over UDP.  Although
   [RFC1035] says "Messages carried by UDP are restricted to 512 bytes",
   that was later updated by [RFC6891].  This media type restricts the
   maximum size of the DNS message to 65535 bytes.  Note that the wire
   format used in this media type is different than the wire format used
   in [RFC7858] (which uses the format defined in Section 4.2.2 of
   [RFC1035]).

SB> 4.2.2 does not seem to describe a format, unless you are implicitly
SB> saying that 4.2.1 does not have a length 4.2.2 does and you
SB> are encoding the length another way and hence use 4.2.1 format.
SB> Either way neither of these sections of RFC1035 really specify the
SB> format, that is provided earlier in the RFC.

===========

   Encoding considerations: This is a binary format. The contents are a
   DNS message as defined in RFC 1035. The format used here is for DNS
   over UDP, which is the format defined in the diagrams in RFC 1035.

SB> With or without the UDP header?

==========

Minor issues:

Abstract

   This document describes how to make DNS queries over HTTPS.

SB> This is the shortest abstract I have seen in a long while, if
SB> not forever. It would be helpful to the undecided reader if
SB> a little more context from the Introduction was included in the
SB> abstract.

=========

Nits/editorial comments:

These caches may exist beteen the
SB> s/beteen/between/