Last Call Review of draft-ietf-add-ddr-08
review-ietf-add-ddr-08-genart-lc-sparks-2022-07-08-00

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-add-ddr-08
Reviewer: Robert Sparks
Review Date: 2022-07-08
IETF LC End Date: 2022-07-08
IESG Telechat date: 2022-07-14

Summary: Has issues to address before publication as a Proposed Standard RFC

(Note: I reviewed -07, and noticed -08 while entering this review. I've read
the diffs, and believe all the below to still be relevant).

Issues:

RFC 6761 requires explicit discussion of seven categories of consumers of a new
SUDN. This document does not yet provide that discussion.

There's a lack of discussion of issues around certificate revocation throughout
the document. The end of section 4.1.1, for example, cuts off an mitigation
opportunity should control of a certificate used by the Designated Resolver be
lost.

Please add an explanation for _why_ the requirement in the last sentence of the
first paragraph of 4.2 exists. As written, it seems out of context, and
underspecified (which SVBC result, obtained when, should the client consider
the TTL from?).

The discussion of providing differentiated behavior over unencrypted DNS is
good to call out, but needs more depth. There are many other fields an attacker
might modify, even outside the DNS part of the datagram (say, the source IP
address) that could give the attacker an advantage if the returned results
varied.

Nits:

In the introduction, please reconsider "claims ownership over the IP
addresses". It would be better to simply say "contains the IP addresses in the
SubjectAltName.

There is tension between the normative SHOULD NOT and SHOULD in the first
paragraph of 4.1.1 and the SHOULD in the first paragraph of 4.2. Please clarify
the wording in one place or the other so that an implementer isn't forced to
violate one of those normative requirements to satisfy the other.

The last sentence of the first paragraph of 4.3 is imprecise. It would address
my discomfort to replace "cannot be confirmed" with "cannot be safely
confirmed", or add a pointer to a description somewhere else about why trying
to include such addresses in a certificate is an unworkable idea.

At the end of the second paragraph of 4.3, consider future protocols that might
use something other than TLS as the security layer. The sentence as is takes a
shortcut past the point your are really trying to make.

The use of SHOULD in the second paragraph of section 5 is strange. Do you mean
"are expected to be"? The other side of this coin (that records are NOT
expected to be present) isn't obvious to find in section 4.

Consider explicitly calling out what the implementation MUST do if the
validation in the 4th paragraph of section 7 fails.

(Feel free to ignore this, but): At the 5th paragraph of section 7, consider
discussing the risks of an operator running an Unencrypted Resolver at a given
IP and _not_ running a Designated Resolver there. This adds an attack point for
an adversary to gain enough access to run their own Designated Resolver there,
even if they can't gain enough privilege to affect the Unencrypted Resolver.

Micro-Nits:

Section 3 3rd to last paragraph: s/use others records/use other records/