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

[Florian Obser <[e-mail]>]

On 2023-06-12 20:41 UTC, Paul Hoffman <[e-mail]> wrote:
> 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.

Excellent. I was not 100% sure what the outcome in the WG was.

>
>> * 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". 
>

you cut of "that differ". If querying with the correct SNI you get a
specific RRset. with the wrong SNI you get the empty set. Those
sets differ. At least that's how I read it.

>> 
>> * 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.
>

I think we are on the right track in the dprive mailing list
discussion. I think we can resolve it over there.

>> 
>> * 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.
>

makes sense.

>> 
>> * 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?
>

I need to go through section 4 again. I was under the impression that
its not just about probing but it also says things about what to do when
we know that a specific nameserver supports E and we send subsequent
queries to it.

So if we know that a.iana-servers.net supports E, we will use E and
*not* Do53. Then that server runs out of resources and starts to answer
SERVFAIL, we would just returns SERVFAIL ourselves and not try
b.iana-servers.net. (It's possible that I misunderstood section 4).

>> 
>> * 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.
>

Let me come back to you on this.

>> 
>> * 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?

So this gave me the impression that it's not just about initial probing
but also about what to do with subsequent queries.

So we successfully probed, we have a connection open, we send a bunch of
queries through and then idle for a bit. The auth will not keep sessions
open indefinitely and after some time without activity shut the session
down.
If we then have another query for the server we should open an encrypted
connection again, because we know it will work. This section seems to
suggest to do one round trip through Do53.

>
>> 
>> * 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
>
>

-- 
In my defence, I have been left unsupervised.