Re: [DNSOP] Eric Rescorla's No Objection on draft-ietf-dnsop-session-signal-12: (with COMMENT)

[Eric Rescorla <[e-mail]>]

On Tue, Jul 31, 2018 at 7:28 PM, Ted Lemon <[e-mail]> wrote:

> On Tue, Jul 31, 2018 at 6:28 PM, Eric Rescorla <[e-mail]> wrote:
>
>> IMPORTANT
>> S 5.3.
>> >      field set to zero, and MUST NOT elicit a response.
>> >
>> >      Every DSO request message (QR=0) with a nonzero MESSAGE ID field is
>> >      an acknowledged DSO request, and MUST elicit a corresponding
>> response
>> >      (QR=1), which MUST have the same MESSAGE ID in the DNS message
>> header
>> >      as in the corresponding request.
>>
>> How do I handle duplicate message IDs on the responder? Did I miss
>> where you said this? Is this just an error?
>>
>
> In order for the responder to detect this condition, it has to track
> message IDs.   The existing implementation that I have doesn't track
> message IDs—it just responds with the message ID that it got.   There's no
> hash table of known incoming message IDs, and maintaining such a table
> would add complexity.   I think that this is okay, because it's the
> requester that loses in this case—it's effectively attacking itself.
>  However, if the responder does track message IDs, then it could be an
> issue.   So I've added the following text:
>
>  If a client or server receives a response (QR=1) where the MESSAGE ID is
> zero, or is
>  any other value that does not match the MESSAGE ID of any of its
> outstanding operations,
>  this is a fatal error and the recipient MUST forcibly abort the
> connection immediately.
>
> +If a responder receives a request (QR=0) where the MESSAGE ID is not
> zero, and
> +the responder tracks query MESSAGE IDs, and the MESSAGE ID
> +matches the MESSAGE ID of a query it received for which a response has
> not yet been sent,
> +it MUST forcibly abort the connection immediately.   This behavior is
> required to prevent
> +a hypothetical attack that takes advantage of undefined behavior in this
> case.   However,
> +if the server does not track MESSAGE IDs in this way, no such risk
> exists, so tracking
> +MESSAGE IDs just to implement this sanity check is not required.
>
>  Does this address your concern?
>

Yeah, this seems fine. Didn't mean to make you do a lot of work here, just
noticed as I was reading.

>
>
>> S 9.3.
>> >
>> >      Requests to register additional new DSO Type Codes in the
>> >      "Unassigned" range 0040-F7FF are to be recorded by IANA after
>> Expert
>> >      Review [RFC8126].  At the time of publication of this document, the
>> >      Designated Expert for the newly created DSO Type Code registry is
>> >      [*TBD*].
>>
>> What is the standard for the expert to follow
>>
>
> I added this (the text has changed somewhat as a result of a previous
> comment):
>
>  Requests to register additional new DSO Type Codes
>  in the "Unassigned" range 0040-F7FF
>  are to be recorded by IANA after Expert Review {{!RFC8126}}.
> +The expert review should validate that the requested type code
> +is specified in a way that conforms to this specification, and
> +that the intended use for the code would not be addressed with
> +an experimental/local assignment.
>
>  DSO Type Codes in the "experimental/local" range F800-FBFF
>  may be used as Experimental Use or Private Use values {{!RFC8126}}
>
> OK?
>

LGTM.



>
>> COMMENTS
>> S 1.
>> >      is appended to the end of the DNS message header.  When displayed
>> >      using packet analyzer tools that have not been updated to recognize
>> >      the DSO format, this will result in the DSO data being displayed as
>> >      unknown additional data after the end of the DNS message.  It is
>> >      likely that future updates to these tools will add the ability to
>> >      recognize, decode, and display the DSO data.
>>
>> I'm sure you will get to this soon, but what are the backward
>> compatibility implications for the two endpoints.
>>
>
> Do you mean in terms of requesters and responders that don't implement DSO
> but receive a DSO message?   I think we've specified that adequately—the
> responder is expected to return "Not Implemented" because of the status
> code.   As I mentioned in a different response, this is the behavior of at
> least BIND 9 and Google's 8.8.8.8 server.   1.1.1.1 just didn't reply to
> the message.   So I realized, and maybe this is what you intended me to
> realize, that the text doesn't address several possible outcomes of trying
> to establish a DSO session.   I believe the following changes should do it:
>
>  sending DNS messages on that connection, but the client SHOULD NOT
>  issue further DSO messages on that connection.
>
> +Two other possibilities exist: the server might drop the connection, or
> +the server might send no response to the DSO message.   In the first
> +case, the client SHOULD mark the server as not supporting DSO, and not
> +attempt a DSO connection for some period of time (at least an hour)
> +after the failed attempt.   The client MAY reconnect but not use
> +DSO, if appropriate.
> +
> +In the second case, the client SHOULD set a reasonable timeout, after
> +which time the server will be assumed not to support DSO.   At this
> +point the client MUST drop the connection to the server, since the
> +server's behavior is out of spec, and hence its state is undefined.
> +The client MAY reconnect, but not use DSO, if appropriate.
> +
>  When the server receives a DSO request message
>  from a client, and transmits a successful NOERROR response to that
>  request, the server considers the DSO Session established.
> @@ -477,7 +490,11 @@ clients and servers should behave as described in
> this specification with
>  regard to inactivity timeouts and session termination, not as previously
>  prescribed in the earlier specification for DNS over TCP {{!RFC7766}}.
>
> -Note that for clients that implement only the DSO-TYPEs defined in
> +Because the Keepalive TLV can't fail (that is, can't return an RCODE
> +other than NOERROR), it is an ideal candidate for use in establishing
> +a DSO session.   Any other option that can only succeed MAY also be
> +used to establish a DSO session.
> +For clients that implement only the DSO-TYPEs defined in
>  this base specification, sending a Keepalive TLV is the only
>  DSO request message they have available to initiate a DSO Session.
>  Even for clients that do implement other future DSO-TYPEs, for simplicity
>

You're doing more than I asked for here (thanks!). I just found myself
wondering at this point in the spec how this worked and so was looking for
an overview. But this text is very helpful, so I think it's good to add.



>
>> S 3.
>> >      The unqualified term "session" in the context of this document
>> means
>> >      the exchange of DNS messages over a connection where:
>> >
>> >      o  The connection between client and server is persistent and
>> >         relatively long-lived (i.e., minutes or hours, rather than
>> >         seconds).
>>
>> This is a surprising taxonomy. I would assume that some of the options
>> you are proposing would be relevant with a 30s connection (very long
>> by HTTP standards!)
>>
>
> Hm.   I think this text is motivated by the first and second identified
> use cases for this document: DNS Push and DNSSD Discovery Relay.   Neither
> would be expected to have 30-second connections.   However, in principle I
> think that you are correct, and actually I think specifying the time is
> unnecessary, so I'd propose the following change:
>
>  DNS messages over a connection where:
>
>  - The connection between client and server is persistent and relatively
> -  long-lived (i.e., minutes or hours, rather than seconds).
> +  long-lived.
>  - Either end of the connection may initiate messages to the other.
>

LGTM.


>  In this document the term "session" is used exclusively as described
> above.
>
> S 3.
>> >      Where this specification says, "close gracefully," that means
>> sending
>> >      a TLS close_notify (if TLS is in use) followed by a TCP FIN, or the
>> >      equivalents for other protocols.  Where this specification
>> requires a
>> >      connection to be closed gracefully, the requirement to initiate
>> that
>> >      graceful close is placed on the client, to place the burden of
>> TCP's
>> >      TIME-WAIT state on the client rather than the server.
>>
>> Does this mean that the server will ask the client to close?
>>
>
> Yes, this is specified in 6.6.
>

Yep. I just make these comments as I go and it wasn't clear here. Sorry if
that was confusing.


> S 3.
>> >      connection to be closed gracefully, the requirement to initiate
>> that
>> >      graceful close is placed on the client, to place the burden of
>> TCP's
>> >      TIME-WAIT state on the client rather than the server.
>> >
>> >      Where this specification says, "forcibly abort," that means
>> sending a
>> >      TCP RST, or the equivalent for other protocols.  In the BSD Sockets
>>
>> Because you bother to mention TLS above, what about non-close_notify
>> TLS alerts?
>>
>
> That's the only alert that would be used here.   TLS alerts used in
> connection establishment and in ongoing connections are deferred to the
> DNS-over-TLS spec, which in turn defers to BCP 195.
>

OK.


> S 3.
>> >      the server's listening socket.
>> >
>> >      The terms "initiator" and "responder" correspond respectively to
>> the
>> >      initial sender and subsequent receiver of a DSO request message or
>> >      unacknowledged message, regardless of which was the "client" and
>> >      "server" in the usual DNS sense.
>>
>> Might be helpful to say earlier that this is a request/response
>> protocol
>>
>
> It's not.   Not all requests require responses: Unacknowledged requests
> explicitly don't.
>

Fair enough. I guess where I was going was that you talk about request here
but it's not clear till later that there might be responses. Not a big deal.


>
>> S 3.
>> >
>> >      DNS Stateful Operations uses three kinds of message: "DSO request
>> >      messages", "DSO response messages", and "DSO unacknowledged
>> >      messages".  A DSO request message elicits a DSO response message.
>> >      DSO unacknowledged messages are unidirectional messages and do not
>> >      generate any response.
>>
>> This would be useful further up.
>>
>
> Do you mean in the introduction?
>

Yes.


>
>> S 5.1.
>> >
>> >      DNS over plain UDP [RFC0768] is not appropriate since it fails on
>> the
>> >      requirement for in-order message delivery, and, in the presence of
>> >      NAT gateways and firewalls with short UDP timeouts, it fails to
>> >      provide a persistent bi-directional communication channel unless an
>> >      excessive amount of keepalive traffic is used.
>>
>> Note that this is going to make things not work super-well with DNS-
>> over-QUIC unless you use one stream only.
>>
>
> We don't expect this to work with DNS-over-QUIC without some restriction
> like this, hence the text in paragraph 3 of section 5.1.
>

Sure. I just wanted to point out that that was a consequence of this design


S 5.1.
>> >
>> >      If the RCODE is set to any value other than NOERROR (0) or
>> DSOTYPENI
>> >      ([TBA2] tentatively 11), then the client MUST assume that the
>> server
>> >      does not implement DSO at all.  In this case the client is
>> permitted
>> >      to continue sending DNS messages on that connection, but the client
>> >      SHOULD NOT issue further DSO messages on that connection.
>>
>> Why is this a SHOULD and not a MUST?
>>
>
> Agreed, fixed.
>
>
>> S 5.1.3.
>> >      to any problems that could be result from the inadvertent replay
>> that
>> >      can occur with zero round-trip operation.
>> >
>> >   5.1.3.  Middlebox Considerations
>> >
>> >      Where an application-layer middlebox (e.g., a DNS proxy, forwarder,
>>
>> I'm having trouble with this section. Is it a set of requirements on
>> middleboxes or statements of fact? If the latter, it seems like there
>> are a bunch of ways for middleboxes to mess things up,
>>
>
> This was covered in the gen-art review, and the text was updated to
> address the comment that was made there about this.   We believe that we
> have thought this through and that the text as written is correct.
>

Sorry, has this been changed in a new version?


> S 5.4.
>> >         generate a response, the application-layer software informs the
>> >         TCP implementation that it should go ahead and send the TCP ACK
>> >         and window update immediately, without waiting for the Delayed
>> ACK
>> >         timer.  Unfortunately it is not known at this time which (if
>> any)
>> >         of the widely-available networking APIs currently include this
>> >         capability.
>>
>> Are you going to make a recommendation here?
>>
>
> Out of scope for DNSOP to update TCP stack APIs.
>

Sorry, it wasn't clear from context what I was referring to here. You
discuss a number
of strategies and then say they are all bad. Do you had a recommendation
for what
people ought to do?


>
>> S 6.6.1.2.
>> >      client a Retry Delay message, or by forcibly aborting the
>> underlying
>> >      transport connection) the client SHOULD try to reconnect, to that
>> >      service instance, or to another suitable service instance, if more
>> >      than one is available.  If reconnecting to the same service
>> instance,
>> >      the client MUST respect the indicated delay, if available, before
>> >      attempting to reconnect.
>>
>> Do you want to recommend some sort of randomness around this value to
>> avoid avalanche?
>>
>
> The server is specifying the retry delay, so if the client adds
> randomness, that could result in more collisions rather than fewer.
>

Sure, if the server actually randomizes it itself. Perhaps that's worth
recommending?


>
>> S 8.2.
>> >      The table below indicates, for each of the three TLVs defined in
>> this
>> >      document, whether they are valid in each of ten different contexts.
>> >
>> >      The first five contexts are requests or unacknowledged messages
>> from
>> >      client to server, and the corresponding responses from server back
>> to
>> >      client:
>>
>> Nit. This text is a tiny bit hard to read, because you don't list S-P,
>> etc.
>>
>
> Hm, I believe that this should be using the requester/responder
> vernacular, not the client/server vernacular.   I've asked the team to
> confirm.
>
> S 10.
>> >      messages are subject to the same constraints as any other DNS-over-
>> >      TLS messages and MUST NOT be sent in the clear before the TLS
>> session
>> >      is established.
>> >
>> >      The data field of the "Encryption Padding" TLV could be used as a
>> >      covert channel.
>>
>> Why not require this to be 0, then?
>>
>
> The text describing the option currently says this:
>
> As specified for the EDNS(0) Padding Option {{!RFC7830}}
> the PADDING bytes SHOULD be set to 0x00.  Other values MAY be used,
> for example, in cases where there is a concern that the padded
> message could be subject to compression before encryption.
> PADDING bytes of any value MUST be accepted in the messages received.
>
> Do you believe that this is an invalid concern?
>

Mostly I just missed this. With that said, generally we are discouraging
compression in cryptographic protocols.
OTOH, I'm not that worried about the covert-channel either, so it's kind of
a tossup
-Ekr