Subject: [dns-privacy] review of dprive-unilateral-probing

From: Alexander Mayrhofer <[email-masked]>
Date: Wed, 6 Apr 2022 17:52:00 +0200

Hello dkg, all,

as requested by the WG chairs, I've reviewed draft-ietf-dprive-unilateral-probing. Here are my comments:

- I think the "DNS camel" comment in the introduction would be hard to understand for any uninitiated person. While it fits the text perfectly now, I think it should be reworded (or a reference should be added) once we move towards publication.
- In minimizing negative impacts, the third bullet should probable be "the potential for amplification attacks.. ", and the second "excessive use of computational..." (NIT). I think many implementors are concerned about excessive requirements to keep state, but I guess that's covered by the mem/CPU bullet, even on intermediaries (Firewalls, etc.)
- I agree to the prococol choices considerations. Towards publication, we should shorten the paragraph about the DoH path problem.
- As noted in the WG discussion, the X.509 certificate with common names of all the names an authoritative server is known under could be problematic, because customers are usually "creating" their own nameserver names without necessarily informing the op of the nameserver. Side note - this might be an interesting research topic - to look at the names requested in SNI to the DoT port of such servers ... (once we have deployment!)
- I guess implementors have more to say about the client-side probing strategy proposed, but it does look very reasonable to me. Maybe we can get feedback from early implementations on optimizing the resource-use of state information required.
- The ALPNs under section 4.2 should be quoted.
- The description of the connection state (4.3, first section, and the state mangling in 4.5.*) is very close to implementation specifications - do we really need that great detail for the protocol to be interopable? Given the connection to the requirements in 4.5.1, I do admit it's tricky. Maybe we should move that to an Appendix? Section 4.3.1, though, is more like an abstract requirement, and should be kept in. Maybe re-order? I agree on the 4.4. "per IP address" recommendation. It might even be worse for v4/v6 deployments.
- in 4.5.3, regarding the FIXME, I think the client SHOULD keep using the existing connection for the non-preferred transport, but MAY re-probe for the preferred transport, and upgrade to that if probing succeeds - (maybe pretty complicated to implement, though?)
- All the "identifier" strings in the draft should be quoted. (such as "early", "unsent", etc..) - maybe personal preference, but I find it clearer. Latest, the RFC editors will comment on that anyways.
- For the Padding - as with DoQ I would recommend that the server SHOULD use padding according to RFC 7830, and may (lowercase) use the experimental policies set in RFC 8467. However, the individual transports may override this - DoQ eg. recommends QUIC-based padding over EDNS level padding.

To sum it up - my comments are minor tweaks. It's a structurally good protocol and draft, thanks for the work!

Best,
Alex

---

From: Daniel Kahn Gillmor <[email-masked]>
Date: Sat, 09 Apr 2022 08:01:48 -0700

Hi Alex--

thanks for this thoughtful review!

I've adopted most of the changes you highlighted (until we submit a new
draft to the datatracker, they can be seen in the editor's draft
https://dkg.gitlab.io/dprive-unilateral-probing/), but wanted to note a
few of them that I didn't fully incorporate yet:

On Wed 2022-04-06 17:52:00 +0200, Alexander Mayrhofer wrote:
> - I agree to the prococol choices considerations. Towards publication,
> we should shorten the paragraph about the DoH path problem.

noted, and recorded here so we don't lose it:
https://gitlab.com/dkg/dprive-unilateral-probing/-/issues/16

> - As noted in the WG discussion, the X.509 certificate with common
> names of all the names an authoritative server is known under could be
> problematic, because customers are usually "creating" their own
> nameserver names without necessarily informing the op of the
> nameserver. Side note - this might be an interesting research topic -
> to look at the names requested in SNI to the DoT port of such servers
> ... (once we have deployment!)

> - I guess implementors have more to say about the client-side probing
> strategy proposed, but it does look very reasonable to me. Maybe we
> can get feedback from early implementations on optimizing the
> resource-use of state information required.

I agree with these two comments, but i'm not sure there's anything to
put in the draft about either of them.  if you have text you want to
suggest, and a place to put it, please let me know!

> - The description of the connection state (4.3, first section, and the
> state mangling in 4.5.*) is very close to implementation
> specifications - do we really need that great detail for the protocol
> to be interopable? Given the connection to the requirements in 4.5.1,
> I do admit it's tricky. Maybe we should move that to an Appendix?
> Section 4.3.1, though, is more like an abstract requirement, and
> should be kept in. Maybe re-order? I agree on the 4.4. "per IP
> address" recommendation. It might even be worse for v4/v6 deployments.

This draft is definitely "close to implementation specifications" -- but
that's kind of the goal here.  We're trying to describe what specific
policies each server needs to take to avoid harming resolution and
disincentivizing other players from themselves adopting these
strategies.

I've added a high-level overview to section 4 so that an implementer
gets the general idea before diving into the specifics:
https://dkg.gitlab.io/dprive-unilateral-probing/#name-high-level-overview
but i'm kind of reluctant to move the state tables or the specific
transition steps into an appendix.

If those state tables or transition steps are misguided, incomplete for
the stated purpose, or problematic, i'd like to know about it from
implementers so we can correct them.

I fully expect some implementers to keep *additional* state beyond what
is described here, because they may have additional goals.  And of
course some implementers might use their own data structures that are
the equivalent of the state described here without being named or
indexed in the exact way described in the document.  But i don't see how
to perform the recursive resolver side without keeping something
equivalent to this state.

Would it help to explicitly acknowledge that this description isn't a
requirement for variable names, data structures, etc?  I'm happy to
accept text.

Many of the documents coming out of the WHATWG and W3C offer this level
of implementation guidance or even more. For example:

  https://www.w3.org/TR/webdriver/
  https://encoding.spec.whatwg.org/

They do this to enhance interoperability; when each party knows that the
other conformant party will behave in a certain way, it becomes simpler
to make a decision about how to interact.

I think this is not an unreasonable posture for a document like this in
the IETF too.

> - All the "identifier" strings in the draft should be quoted. (such as
> "early", "unsent", etc..) - maybe personal preference, but I find it
> clearer. Latest, the RFC editors will comment on that anyways.

In the HTML version (which i'm thinking of as the canonical form) all
those identifier strings are set off as <code> elements so they're
already visually distinct.  Do you (or does the WG) want quotation marks
as well?

Another possibility would be to rely on <code> in the text/html form,
and quotation marks in the text/plain form, but the xml2rfc tooling
doesn't really permit that.  Here are some links that describe that
decision (i'm not sure i agree with it, but i'm also not sure i have the
capacity to fight it):

   https://github.com/ietf-tools/xml2rfc/issues/600
   https://github.com/ietf-tools/xml2rfc/issues/647
   https://notes.ietf.org/cmt-20210222#

I could do an automated replacement in the source to wrap all literal
strings (`foo`) with quotation marks ("`foo`"), using something like:

   sed -e 's/\([^"]\|^\)`\([^`]*\)`\([^"]\|$\)/\1"`\2`"\3/g'

But i'm not sure whether it improves the document -- perhaps that's
overkill?  Or maybe I'm not sure what you think counts as an
"identifier" string.  is `E-foo[X]` an identifier string?

You can get a list of all the backtick-wrapped strings in the document
(and their counts) with:

    grep '`' unilateral-probing.md | \
    sed -e 's/[^`]*`\([^`]*\)`[^`]*/\1\n/g' | \
    grep -v '^$' | sort | uniq -c | sort -n

In the editor's copy, i've quoted the ALPN strings as "`doq`" so you can
see what that looks like in both html and text, but I haven't done any
more changes.

Editor's copy:

 text/html: https://dkg.gitlab.io/dprive-unilateral-probing/
text/plain: https://dkg.gitlab.io/dprive-unilateral-probing/unilateral-probing.txt

    --dkg