Re: [dns-privacy] [Ext] Dnsdir early review of draft-ietf-dprive-unilateral-probing-07

[Paul Hoffman <[e-mail]>]

Thank you for the extensive review! Comments below.

> This is an early review and I am aware of ongoing discussions in the
> dprive working group about the intended status of the document, the
> probing policy and what to do about recursive resolvers implementing
> DoT collocated with authoritative servers implementing Do53. This
> review mostly focuses on what is in revision 07 of the draft but it is
> possible that information from the mailing list snuck in.
> 
> * Intended status: Standards Track
> 
> There is ongoing discussion in dprive about this, for now I think the
> status should be "Experimental".

Yep, that was already agreed to in the WG. The -08 will reflect that.

> * 3.1.  Pooled Authoritative Servers Behind a Single IP Address
> |   A recursive resolver following the guidance in Section 4 that
> |   interacts with such a pool likely does not know that it is a pool.
> |   If some members of the pool follow this guidance while others do not,
> |   the recursive client might see the pool as a single authoritative
> |   server that sometimes offers and sometimes refuses encrypted
> |   transport.
> 
> The second occurrence of "guidance" refers to something else than the
> first occurrence. Suggested fix for the second "guidance":
> 
> |   If some members of the pool follow the protocol specified in
> |   this document while others do not,

Fixed.

> 
> * 3.3.  Server Name Indication
> | MUST NOT serve resource records that differ based on SNI
> 
> The MUST NOT adds a strong restriction on future protocols that
> eliminate opportunistic encryption and unilateral probing.  I can
> envision a desire to at least respond REFUSED when using the wrong
> name.
> 
> As B.3. puts it:
> |   Any new stronger protocol should consider how it interacts with the
> |   opportunistic protocol defined here, so that operators are not faced
> |   with the choice between widespread opportunistic protection against
> |   passive attackers (this document) and more narrowly-targeted
> |   protection against active attackers.
> 
> I would say this protocol should also consider how it might interact with
> a new stronger protocol. That is, how can it be forward compatible.

This future-proofing is a good concern, but doesn't the current wording already cover it? The full text is:
   An authoritative DNS server that wants to handle unilateral queries
   MAY rely on Server Name Indication (SNI) to select alternate server
   credentials.  However, such a server MUST NOT serve resource records
   that differ based on SNI (or on the lack of SNI) provided by the
   client, because a probing recursive resolver that offers SNI might or
   might not have used the right server name to get the records it is
   looking for.
Returning REFUSED is certainly one way to meet "MUST NOT serve resource records". 

> 
> * 3.4.  Resource Exhaustion
> Wrong section:
> | Section 6.5 of [RFC9250] ("Connection Handling")
> 
> should be
> 
> | Section 5.5 of [RFC9250] ("Connection Handling")

Fixed.

> 
> * 4.1.  High-level Overview
> "If the encrypted transport protocol fails"
> 
> We really need to spell out what "fails" means, somewhere.
> 
> I am under the impression that draft is only concerned with successful
> / unsuccessful probes on the transport layer. As long as something
> answers it is a successful probe, it would not even need to be a valid
> DNS answer.

In the previous message to the mailing list, I pointed out that this is covered in Section 4.6.4. Having said that, possibly moving the definition up in the document seems fine. However, I think you want a different definition; if so, please specify.

> 
> * 4.2.  Maintaining Authoritative State by IP Address
> |    By associating state with the IP address, the recursive client is
> |    most able to avoid reaching a heterogeneous deployment.
> 
> I do not understand what this is trying to say. Well, I understood it
> retroactively after reading the following example. I think this needs
> better wording. How about dropping that paragraph and pulling the last
> one up, thusly:
> 
> |   respond with an ICMP port closed message).
> |
> |   By associating the state with the authoritative IP address, the
> |   client can minimize the number of accidental delays introduced (see
> |   also Section 4.5.1 and Section 3.1).
> |
> |   For example, consider an authoritative server named ns0.example.com
> |   that is served by two installations (with two A records), one at
> |   192.0.2.7 that follows this guidance, and one at 192.0.2.8 that is a
> |   legacy (cleartext port 53-only) deployment.  A recursive client who
> |   associates state with the NS name and reaches .7 first will "learn"
> |   that ns0.example.com supports encrypted transport.  A subsequent
> |   query over encrypted transport dispatched to .8 would fail,
> |   potentially delaying the response.

Yes, that's definitely better!

> 
> * 4.3.  Overall Recursive Resolver Settings
> |   A recursive resolver implementing this document
> 
> maybe this is better:
> 
> |   A recursive resolver implementing this protocol

Yes.

> 
> |   Implementers or deployers of DNS recursive resolvers that follow the
> |   strategies in this document are encouraged to report their preferred
> |   values of these parameters.
> 
> report where? should this be deleted before publishing?

Given the experimental status of the protocol, knowing this information will help people determine the status of the experiment. I have changed "report" to "publish", and hope we can find the published values.

> 
> * 4.5.  Authoritative Server Encrypted Transport Connection State
> Table 2:
> | Retain Across Reset
> 
> should be
> 
> | Retain Across Restart
> 
> because later on the text uses "restart" as well.

Oh, my. Yes, definitely.

> 
> * 4.6.  Probing Policy
> |   When a recursive resolver discovers the need for an authoritative
> |   lookup to an authoritative DNS server using IP address X, it
> |   retrieves the records associated with X from its cache.
> 
> Since recursive resolvers deal with DNS resource records and usually
> implement a cache of DNS RRs it took me a moment to figure out that
> "record" and "cache" refer to the things from table 2. Using different
> terminology might improve this section.

Fixed.

> 
> Since the policy is currently under discussion in the dprive-wg I did
> not fully check the correctness of the algorithm. I can do that once
> the discussion settles down.

We welcome all review of the algorithm.

> 
> * 4.6.9.  Receiving a Response over Encrypted Transport
> |    But if R is unsuccessful (e.g. timeout or connection closed):
> 
> If there is a timeout or the connection is closed there is no response
> R at all. I think the draft needs to be more explicit in defining
> successful and unsuccessful outcomes. Using "e.g." in brackets
> suggests the list is not exhaustive for unsuccessful. What if an
> answer is received but it is REFUSED (unsuccessful?), SERVFAIL
> (unsuccessful?), NXDOMAIN (obviously successful, but from the point of
> view of the person trying to reach a website they were unsuccessful).

Please see proposed text in the previous message on the mailing list.

> 
> |      -  Return SERVFAIL to the requesting client
> 
> This does not seem to be correct. Consider this:
> example.com. 86400 IN NS a.iana-servers.net.
> example.com. 86400 IN NS b.iana-servers.net.
> 
> If a.iana-servers.net is unreachable we will short-circuit to SERVFAIL
> without trying b.iana-servers.net. That is not how recursive resolvers
> work. 4.6.2 has the same issue.

This is the action for "*  If Q is not in Do53-queries[X] or in any of *-queries[X]:". This means that resolver chose not to send a query on Do53 but had a reason to send a probe to "a". To me, a SERVFAIL makes more sense than the overhead of trying DoE on all the other NS records. If you disagree, what logic would you substitute here?

> 
> * 4.6.6.  Encrypted Transport Failure
> 
> Should we first try another encrypted transport if available and not
> fall back to Do53 immediately?

We don't know if the resolver is trying over multiple encrypted transports in succession (as compared to in parallel): that's optional. We could add more state throughout 4.6 to store and handle that information; in such a case, yes, this would be the logic here. That feels overly prescriptive to me, but others might disagree.

> 
> * 4.6.7.  Handling Clean Shutdown of an Encrypted Transport Connection
> Should we immediately retry E for the outstanding queries?

I don't think so. What is your logic here?

> 
> * 4.6.10.  Resource Exhaustion
> | Section 6.5 of [RFC9250] ("Connection Handling")
> 
> should be
> 
> | Section 5.5 of [RFC9250] ("Connection Handling")

Fixed.

> 
> * 4.6.11.  Maintaining Connections
> |   Some recursive resolvers looking to amortize connection costs, and to
> |   minimize latency MAY choose to synthesize queries to a particular
> |   resolver to keep an encrypted transport session active.
> 
> s/particular resolver/particular authoritative server/

Fixed.

> 
> * 5.  IANA Considerations
> odd boiler plate

There is no standard for NXIANA boiler plate. IANA always double-checks the section when documents move to IETF Last Call.

Thanks again for the thorough review!

--Paul Hoffman