Subject: [DNSOP] Benjamin Kaduk's Discuss on draft-ietf-dnsop-session-signal-12: (with DISCUSS and COMMENT)

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 27 Jul 2018 08:24:56 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-dnsop-session-signal-12: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-dnsop-session-signal/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Hopefully this first point is simple to resolve (whether by changing text or merely
un-confusing me).  The other ones may require some actual discussion,
though.

Section 6.6.1.2 has:

   After a DSO Session is ended by the server (either by sending the
   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.

which seems to contradict the text from Section 3:

%  [...] Given that these fatal error
%  conditions signify defective software, and given that defective
%  software is likely to remain defective for some time until it is
%  fixed, after forcibly aborting a connection, a client SHOULD refrain
%  from automatically reconnecting to that same service instance for at
%  least one hour.

Given some other mentions in the document about aborting the connection, it
may be that I am just reading the "refrain from reconnecting for an hour"
more strongly than I should be.

Is Section 5.1.2 expected to be considered an "application protocol
profile" that defines the use of TLS 1.3 0-RTT data for DSO?  (If so, I do
not personally feel that it provides adequate treatment to be considered as
such.)

I should probably leave this to my (transport-area?) colleagues to discuss
further, but I'm not sure that the interaction of this mechanism with
high-RTT connections is fully covered -- for example, the inactivity
timeout in Section 6.4(.x) could behave poorly when the timeout is set to a
smaller value than the RTT, as the server would potentially end up starting
the "forcibly abort" process (and potentially causing the client to lose
for an hour) because the server's timer starts when it sends the DSO
response that initiates its idea of the session, and would not recieve
graceful shutdown messages from a properly-behaving client in time.

I'm not sure that the behavior specified in Section 7.1.2 is entirely safe.
Consider when a client sends a DSO keepalive to try to request a DSO
session, but subsequently sends EDNS(0) TCP Keepalive (whether "just in
case" or for some other reason).  The Server will receive the DSO keepalive
and respond, creating a session, but the EDNS(0) TCP Keepalive is already
in flight.  I don't remember seeing any text that prevents this client
behavior explicitly, but that seems like the right sort of thing to do.

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Six authors exceeds five, so "there is likely to be discussion" about this
being too large a number of authors.  What is the justification for the
author count?

Do we need to specify some GREASE (per draft-ietf-tls-grease) for new TLV
types in order to ensure the proper handling of unknown types?

Section-by-section comments follow.

Section 1

A DoH reference seems timely/apt.  (But maybe then it is only "Some such
transports" that can offer persistent sessions.)

Maybe give some examples of advantages of server-initiated messages?  Are
we talking about letting the server push records with larger TTLs or
notifying when the response to a query is changing, or just more mundane
keepalive-type things?

Section 3

   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.

We just defined "client" and "server" explictly (without reference to the
"usual DNS sense"), so probably it's best to have this definition refer to
the previous client/server definitions or clarify that the above
definitions match the "usual DNS sense".

   When an anycast service is configured on a particular IP address and
   port, it must be the case that although there is more than one
   physical server responding on that IP address, each such server can
   be treated as equivalent.  If a change in network topology causes
   packets in a particular TCP connection to be sent to an anycast
   server instance that does not know about the connection, the normal
   keepalive and TCP connection timeout process will allow for recovery.
   If after the connection is re-established, [...]

Perhaps clarifying that "recovery" means "detecting the broken session and
starting a new one" would be useful?  (I guess Spencer's DISCUSS takes this
a different direction.)

   DSO unacknowledged messages are unidirectional messages and do not
   generate any response.

"Do not generate any response" at the DNS layer; any reason to mention that
TCP will still ACK the bytes (or rather, that the "reliable" part of the
data stream will need to do so)?

Section 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).

nit: is this "in only" or "only in"

   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.

I'm confused how the server would still have proper framing for subsequent
DNS messages, since the DSO TLVs would be "spurious extra data" after a
request header and potentially subject to misinterpretation as the start of
another DNS message header.

Section 5.1.3

It is probably worth explicitly noting that a middlebox MUST NOT
make/forward a DSO request with TLVs that it does not implement.

Section 5.2

   If a DSO message is received where any of the count fields are not
   zero, then a FORMERR MUST be returned, unless a future IETF Standard
   specifies otherwise.

This seems like ... not the conventional wording for this behavior, and
subject to large debates about the meaning of "IETF Standard".
(Similar language is used elsewhere, too.)

Section 5.2.2

The start of this section seems to duplicate a lot of Section 3 -- e.g.,
the specification of the "Primary TLV"; request/response/unacknowledged as
the types of messages; etc.  It's unclear to me that this duplication of
content is helpful to the reader.

   Unacknowledged messages MUST NOT be used "speculatively" in cases
   where the sender doesn't know if the receiver supports the Primary
   TLV in the message, because there is no way to receive any response
   to indicate success or failure of the request message (the request
   message does not contain a unique MESSAGE ID with which to associate
   a response with its corresponding request).  Unacknowledged messages
   are only appropriate in cases where the sender already knows that the
   receiver supports, and wishes to receive, these messages.

Having gone to the trouble to explicitly define (twice!) "request",
"response" and "unacknowledged", it's pretty confusing to then use the
English word "request" to refer to an "unacknowledged" message.

Section 5.2.2.1

   The specification for a TLV states whether that DSO-TYPE may be used
   in "Primary", "Additional", "Response Primary", or "Response
   Additional" TLVs.

Perhaps this could be wordsmithed to avoid accidental misreading as
exclusive-or?

Section 5.3.1

   When a DSO unacknowledged message is unsuccessful for some reason,
   the responder immediately aborts the connection.

Doesn't this kill the client/server pairing for an hour?  "For some reason"
is very vague to induce such behavior, and could include transient internal
errors.

Section 6.1

When would it be appropriate for the server to send responses out of order?

Section 6.6.1.1

nit: "RECONNECT DELAY" is used with inconsistent capitalization.

Section 7.1

The description of the two timeout fields is predicated on understanding
that it is only the response's incarnation of them that is authoritative;
as an editorial matter, it might be nice to introduce this fact earlier.

Section 7.1.1

It seems like we could consolidate the "equal to" and "greater than" cases
into "greater than or equal to".

Section 7.2.1

   A client MUST NOT send a Retry Delay DSO request message or DSO
   unacknowledged message to a server. [...]

nit: it must not send it as a response, either, so perhaps "MUST NOT send a
Retry Delay DSO message to a server" is shorter and better.

Section 9.3

I thought IANA liked to see a "registration template" for what subsequent
registrations in a registry being created will need to specify.  (That
said, the IANA state is "IANA OK - Actions Needed", and one might expect
that they know better than I do...)

Section 10

I'm a little surprised to not see some discussion that this mechanism
encourages the maintenance of persistent connections on DNS servers, which
encourages the maintenance of persistent connections on DNS servers, which
has impact on resource consumption/load, but is not expected to be
problematic because the server can tell the clients to go away if needed.

---

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 30 Jul 2018 13:14:55 -0500

Hi Mirja,

On Mon, Jul 30, 2018 at 08:11:52PM +0200, Mirja Kuehlewind (IETF) wrote:
> Hi Ben, hi all,
> 
> as you summoned an TSV AD...
> 
> > Am 27.07.2018 um 17:24 schrieb Benjamin Kaduk <[email-masked]>:
> > 
> My understanding is that they require a minimum time-out of 5 second at the server side, which seems reasonably safe to me. However, maybe this could be further clarified or explained in the doc.

I'm happy to defer to your expertise -- thanks!

-Benjamin

---

From: "Mirja Kuehlewind (IETF)" <[email-masked]>
Date: Mon, 30 Jul 2018 20:11:52 +0200

Hi Ben, hi all,

Mirja

---

From: Tom Pusateri <[email-masked]>
Date: Tue, 31 Jul 2018 15:53:57 -0400

> On Jul 27, 2018, at 11:24 AM, Benjamin Kaduk <[email-masked]> wrote:
> 

Section 3 discusses fatal errors on the server side that aren’t likely to change until the software is modified so there’s no use having the client retry quickly (hence the one hour). Section 6.6.1.2 is about receiving a Retry Delay message and the server having the close the connection because the client didn’t follow the spec and close the connection when asked.

I think the difference can be described as Section 3 talks about how the client deals with server bugs and Section 6.6.1.2 describes how the server deals with client bugs.

Does that help?

> graceful shutdown messages from a properly-behaving client in time.

The 0 RTT discussion is also happening in another thread so I will not address it here.

> behavior explicitly, but that seems like the right sort of thing to do.

Ok, is this more clear?

The inactivity timeout value in the Keepalive TLV (DSO-TYPE=1) has similar intent to the edns-tcp-keepalive EDNS0 Option [RFC7828] <file:///Users/pusateri/doc/draft-bellis-dnsop-session-signal/draft-ietf-dnsop-session-signal.html#RFC7828>. A client/server pair that supports DSO MUST NOT use the edns-tcp-keepalive EDNS0 Option within any message after a DSO Session has been established. A client that has sent a DSO message to establish a session MUST NOT send an edns-tcp-keepalive EDNS0 Option from this point on. Once a DSO Session has been established, if either client or server receives a DNS message over the DSO Session that contains an edns-tcp-keepalive EDNS0 Option, this is a fatal error and the receiver of the edns-tcp-keepalive EDNS0 Option MUST forcibly abort the connection immediately.

We have all contributed significantly.

> types in order to ensure the proper handling of unknown types?

Happy to do this if the other authors are in agreement.

> transports" that can offer persistent sessions.)

We specifically didn’t want to reference DoH since HTTP is unsuitable for long lived connections and DSO wouldn’t apply here. We didn’t want to imply that DoH was suitable by referencing it.

> notifying when the response to a query is changing, or just more mundane

Yes, I will add a reference to DNS Push Notifications (https://tools.ietf.org/html/draft-ietf-dnssd-push-14 <https://tools.ietf.org/html/draft-ietf-dnssd-push-14>)

> definitions match the "usual DNS sense”.

Noted. I will add "the usual DNS sense" to the client and server definitions.

> starting a new one" would be useful?  (I guess Spencer's DISCUSS takes this

I will let this discussion continue with Spencer’s DISCUSS.

> data stream will need to do so)?

Noted. Will add “at the DNS layer”.

> nit: is this "in only" or "only in”

Noted. This will be changed to:

DSO messages MUST be carried only in protocols and environments where a session may be established according to the definition given above in the Terminology section (Section 3).

> another DNS message header.

Yes, this is a serious oversight. I think we are going to need to encode differently to make all the TLVs look like an RR externally so the RDLEN can be used to skip them and add a single count or switch the TLV syntax back to RR syntax. The existing DNS header format / RR format is less than ideal...

> make/forward a DSO request with TLVs that it does not implement.

If a middlebox understands any DSO messages then it should follow section 5.2.2.4 (Unrecognized TLVs) with regard to how to handle TLVs that it does not implement. I can make that forward reference.

> (Similar language is used elsewhere, too.)

Noted. We can remove future IETF Standards.

> Perhaps this could be wordsmithed to avoid accidental misreading as

How about:

When a new TLV is defined, the specification MUST include whether the DSO-TYPE can be used as the Primary TLV, used as an Additional TLV, or used in either context for both requests and responses.

> is very vague to induce such behavior, and could include transient internal

An unacknowledged message can only be unsuccessful from the receivers point of view and typically would be a code error because they typically wouldn’t be sent before some protocol support has already been determined by other DSO messages. (i.e. mDNS relay or PUSH notifications are the use cases in mind).

However, your point is noted. This is too general. How about:

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

> When would it be appropriate for the server to send responses out of order?

An example would be with DNS Push notifications subscriptions. The server may receive a query for which it doesn’t have the information in it’s cache followed by a query where it does have the information in it’s cache. So it may asynchronously perform operations to load it’s cache in the first case but immediately response to the second query out of it’s cache.

> nit: "RECONNECT DELAY" is used with inconsistent capitalization.

Noted.

> as an editorial matter, it might be nice to introduce this fact earlier.

Noted. I will change the first paragraph in 7.1 to:

The Keepalive TLV (DSO-TYPE=1) performs two functions: to reset the keepalive timer for the DSO Session, and to establish the values for the Session Timeouts. The client will request the desired Session timeout values and the server will acknowledge with the final values the client MUST use.

> into "greater than or equal to”.

Noted.

> Retry Delay DSO message to a server" is shorter and better.

Noted.

> problematic because the server can tell the clients to go away if needed.

I will add this to the end of Section 10.

Thanks for the thoughtful review. Your comments will definitely make the document better.

Tom

---

From: Tom Pusateri <[email-masked]>
Date: Tue, 31 Jul 2018 16:14:41 -0400

> On Jul 31, 2018, at 3:53 PM, Tom Pusateri <[email-masked]> wrote:
> 
>> 

My co-authors reminded me about the TCP framing for DNS which gives the length of the DNS message so it can easily be skipped so this isn’t a problem.

Thanks,
Tom

---

From: Ted Lemon <[email-masked]>
Date: Tue, 31 Jul 2018 16:16:07 -0400

> My co-authors reminded me about the TCP framing for DNS which gives the length of the DNS message so it can easily be skipped so this isn’t a problem.

And just as an additional data point, I just now pointed my DNSSD Discovery Relay implementation at BIND 9, and BIND 9 returned "Not Implemented" as expected, and did not drop the connection.

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 31 Jul 2018 15:25:42 -0500

> My co-authors reminded me about the TCP framing for DNS which gives the length of the DNS message so it can easily be skipped so this isn’t a problem.

Ah, that would do the trick.  It looks like I only chased up through the
header format in 1035 and didn't scroll down to the "TCP usage" section.
Sorry for the noise.

-Benjamin