Re: [DNSOP] Mirja Kühlewind's Discuss on draft-ietf-dnsop-session-signal-12: (with DISCUSS and COMMENT)

[Ted Lemon <[e-mail]>]

On Mon, Jul 30, 2018 at 4:19 PM, Mirja Kühlewind <[e-mail]>
wrote:

[Skipping the DISCUSS, which I hope we have already addressed.]

1) sec 3: I really find it a bit strange that there is normative language
> about
> error handling (as well as in the "same service instance" definition part)
> in
> the terminology section. Maybe move those paragraphs somewhere else...?
> Also
> the part about "long-lived operations" and messages types provides far more
> information than just terminology and I would recommend to also move it
> into an
> own section or maybe just have it as part of the intro.
>

I happen to agree with this criticism, and this issue was discussed, but we
decided not to make this change, because it would have been a fairly
massive overhaul of the document.


> 2) Maybe call section 5 "Protocol specification" instead of "Protocol
> details"...?
>

That would imply that you might not have to read the rest of the document.


> 3) Sec 5.1: "DSO messages MUST be carried in only protocols and in
> environments
>    where a session may be established according to the definition given
>    above in the Terminology section (Section 3)."
> I don't get this. Which part of section 3? Given section 3 is on
> terminology
> and this is a normative statement, I would recommend to spell out here
> explicitly what is meant. Do you mean the protocol must be
> connection-oriented,
> reliable, and providing in-order delivery? Any thing else? However, given
> that
> you say two paragraphs onwards that this spec is only applicable for the
> use
> with TCP and TLS/TCP, do you even need to specify these requirements
> normatively?
>

We've since updated this text, but it didn't address your concern.   I've
updated it further as follows:

DSO messages MUST NOT be carried in protocols and
environments where a session can't be established.   For example,
DNS over plain UDP {{?RFC0768}} is not appropriate since it does not provide
in-order message delivery, and, in the presence of NAT gateways and
firewalls
with short UDP timeouts, it cannot provide a persistent bi-directional
communication channel unless an excessive amount of DSO keepalive traffic
is used.
UDP also doesn't provide a way to mark the start of a session and the end
of a session.


> 4) sec 5.1 "It is a common
>    convention that protocols specified to run over TLS are given IANA
>    service type names ending in "-tls"."
> Not sure this is true. Isn't it usually just an "s" at the end? Or with
> registry are you talking about?
>

https://www.iana.org/assignments/service-names-port-numbers/
service-names-port-numbers.xml

I think the "s" suffix is more for URIs and /etc/services (which is
informal).   I've added a reference to the registry.


> 5) sec 5.1: "In some environments it may be known in advance by external
> means
>    that both client and server support DSO, ..."
> I guess the client and server also need to know if TLS is supported or not.
> Maybe spell this out as well.
>

This is specified in DNS-over-TLS.   Additionally, for example
draft-ietf-dnssd-mdns-relay explicitly requires TLS.   It's also covered in
the paragraph before this one.  This paragraph is actually a separate
topic: whether the client can assume the server supports DSO and vice
versa, as opposed to negotiating and falling back.


> 6) sec 5.1: "... therefore either
>    client or server may be the initiator of a message."
> Maybe s/initiator of a message/initiator of a message exchange/
>

It's not necessarily an exchange.


> 7) sec 5.1.2: "Having initiated a connection to a server, possibly using
> zero
> round-
>    trip TCP Fast Open and/or zero round-trip TLS 1.3, a client MAY send
>    multiple response-requiring DSO request messages to the server in
>    succession without having to wait for a response to the first request
>    message to confirm successful establishment of a DSO session."
> Why is the ability to send more than one request related to TCP Fast
> Open/TLS1.3 0-RTT? These are two independent mechanisms to speed up
> processing.
> Mentioning TCP Fast Open/TLS1.3 0-RTT here is rather confusing.
> Respectively I
> also don't think that the sentence: "Similarly, DSO supports zero
> round-trip
> operation." is describing quite the same.
>
> 8) Further please provide references to TCP Fast Open and TLS1.3 and maybe
> rephrase this paragraph to use normative language: "Caution must be taken
> to
> ensure that DSO messages sent before the
>    first round-trip is completed are idempotent, or are otherwise immune
>    to any problems that could be result from the inadvertent replay that
>    can occur with zero round-trip operation."
> Maybe just:
> "DSO messages sent with TLS1.3 0-RTT before the TLS handshake is completed
> or
> in TCP SYN data with use of TCP Fast Open MUST be idempotent." However,
> this is
> actually already required by TLS1.3 and TFO, so there is after all no need
> to
> just rephrase this requirement here (at least not normatively). I think it
> would be more useful for every DSO message type to specify if it can be
> sent in
> 0-RTT or not and require this for specification of future TLVs.
>

I agree the text is a bit awkward: this section is only talking about 0rtt
sessions.   I've updated it to say this:

DSO permits zero round-trip operation
using TCP Fast Open {{?RFC7413}}
and TLS 1.3 {{?I-D.ietf-tls-tls13}}
to reduce or eliminate
round trips in session establishment.

A client MAY send multiple response-requiring DSO messages using TCP fast
open or TLS 1.3 early data,
without having to wait for a response to the first request message
to confirm successful establishment of a DSO session.

Also, I added the following to security considerations based on ekr's
related comment:

9) sec 5.1.3: "In cases where a DSO session is terminated on one side of a
>    middlebox, and then some session is opened on the other side of the
>    middlebox in order to satisfy requests sent over the first DSO
>    session, any such session MUST be treated as a separate session."
> This sentence seems a bit non-sensical, which probably isn't great for a
> normative sentence. If a session is terminated and open of the other end,
> doesn't that mean that you have two sessions?
>

The point is to exclude the implementation doing something clever, which
would indeed be nonsensical.


> 10) sec 5.1.3: "A middlebox that is not doing a strict pass-through will
> have
> no way to
>    know on which connection to forward a DSO message, and therefore will
>    not be able to behave incorrectly."
> I'm not sure I understand this sentence. Can you clarify?
>

We added this in response to a similar comment:

To illustrate the above, consider a network where a middlebox
terminates one or more TCP connections from clients and multiplexes the
queries therein over a single TCP connection to an upstream server.
The DSO messages and any associated state are specific to the individual
TCP connections.  A DSO-aware middlebox MAY in some circumstances be
able to retain associated state and pass it between the client and
server (or vice versa) but this would be highly TLV-specific.  For
example, the middlebox may be able to maintain a list of which clients
have made Push Notification subscriptions {{?I-D.ietf-dnssd-push}} and
make its own subscription(s) on their behalf, relaying any subsequent
notifications to the client (or clients) that have subscribed to that
particular notification.


> 11) As already briefly mentioned by Ben, there is quite some redundant
> text in
> sec 5 (with 5.2) for handling of message IDs and TLVs. Given this text is
> normative, I would really recommend to only specify it clearly once. Please
> also check the rest of the doc further things that are specified
> normatively
> multiple times. It usually makes it must clearer to specify it only once,
> at
> least normatively, at the appropriate position in the doc.
>

I think you may be seeing two different places where the message id must be
zero as saying the same thing, but they are not.   Or maybe I'm just
confused, but I don't see a change to make here.


> 12) sec 5.3.1: "When a DSO unacknowledged message is unsuccessful for some
> reason, .." What does unsuccessful mean here? Can you clarify?
>

This has been updated in response to a previous comment:

When an unacknowledged DSO message type is received (MESSAGE ID field is
zero), the receiver SHOULD already be expecting this DSO message type.
{{unrecognized}} describes the handling of unknown DSO message types.
Parsing
errors MUST also result in the receiver aborting the connection.
When an unacknowledged DSO
message of an unexpected type is received, the receiver should abort the
connection.
Other internal errors processing the unacknowledged DSO message are
implementation dependent as to whether the connection should be aborted
according to the severity of the error.

13) sec 6.5.2: "A corporate DNS server that knows it is serving only
> clients on
> the
>    internal network, with no intervening NAT gateways or firewalls, can
>    impose a higher keepalive interval, because frequent keepalive
>    traffic is not required."
> I guess in this scenario it is probably most appropriate to not send any
> keep-alives…
>

I think I already addressed this in my response to your DISCUSS—please let
me know if you still see an issue here.


> 14) sec 6.6: "   o  The server application software terminates unexpectedly
> (perhaps
>       due to a bug that makes it crash)."
> This bullet point does not really make sense to me because at that time
> when
> the app is crashed there is no way for the server anymore to perform any
> actions.
>

Right, the connection is terminated as a result of the server crashing, not
as the result of the server noticing that it crashed and deciding to
terminate the connection. :)


> 15) sec 7.1: "When a client is sending its second and subsequent Keepalive
> DSO
>    requests to the server, the client SHOULD continue to request its
>    preferred values each time. "
> I don't understand the SHOULD here.. what else should be client put in
> these
> field instead...?
>

It could repeat back to the server what the server sent it.


> 16) sec 7.1.2: "Once a DSO Session has been established, if either
>    client or server receives a DNS message over the DSO Session that
>    contains an EDNS(0) TCP Keepalive option, this is a fatal error and
>    the receiver of the EDNS(0) TCP Keepalive option MUST forcibly abort
>    the connection immediately."
> This is normatively specified multiple time (3?) in the doc. Please
> consider to
> only specify it once where most appropriate (probably section 7.1.2)
>

It appears in two places, not three, and in both places it makes sense to
say it.   The requirements are consistent in both places.   I would rather
say it in both places than have someone skim the document and miss it.   If
we had two places in the document where that were all that was said, I'd
agree, but both places where this is specified are contextually
appropriate—I don't agree that 7.1.2 is better.


> 16) sec 7.1: "The Keepalive TLV is not used as an Additional TLV."
> This is redundant with the normative sentence in the next paragraph. Maybe
> just
> remove this sentence...?
>

WFM!


> 17) +1 to Ben's discuss regarding the reconnection of clients. A TCP RST
> can be
> sent for many reasons and waiting for an hour seems not appropriate. I
> would
> rather recommend to log an error and directly try to reconnect.
>

 I haven't seen Ben's response to what Tom said about this, but the context
for this is that the client aborted the connection because it detected an
implementation error on the server.   Waiting an hour is actually pretty
optimistic.