Subject: Re: [dnssd] Genart last call review of draft-ietf-dnssd-push-20

From: Tom Pusateri <[email-masked]>
Date: Tue, 2 Jul 2019 14:36:16 -0400

> On Jun 28, 2019, at 4:03 PM, Robert Sparks via Datatracker <[email-masked]> wrote:
> 
> Reviewer: Robert Sparks
> Review result: Ready with Issues
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-dnssd-push-20
> Reviewer: Robert Sparks
> Review Date: 2019-06-28
> IETF LC End Date: 2019-07-05
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: Ready for publication as a Proposed Standard but with an Issue to
> consider before publication,
> 
> Issue:
> 
> The discussion of recursive resolvers in section 6.1 may need additional
> consideration. In particular, the recommendation to pass a received error code
> along to a client has, I think, some unintended consequences for the client. If
> the recursive server receives a NOTIMP, for example, passing that to the client
> tells the client the wrong thing about the server it is connected to. Perhaps
> it would be better for the recursive server to return SERVFAIL in this
> circumstance? (Similar to what it would do if it couldn't connect to the next
> server as described at the bottom of page 10).

Let me think about this some more before I respond.

> 
> Nits:
> 
> Page 5, Section 3, 3rd paragraph, last sentence: NOT REQUIRED is not a
> 2119/8174 keyword. I suggest using lowercase 'not required' in this sentence.

Fixed.

> 
> Page 7, Section 4, 3rd paragraph: The first sentence alludes to concerns about
> anonymous subscriptions, saying TCP alleviates those concerns. As written this
> is pretty vague. Can you expand on what you mean by an anonymous subscription
> in this context?

Now says:

"Connection setup over TCP ensures return reachability and alleviates concerns of state overload at the server which is a potential problem with connection-less protocols using spoofed source addresses. All subscribers are guaranteed to be reachable by the server by virtue of the TCP three-way handshake.”

> 
> Page 10, Section 6.1, first sentence: Suggest s/first step in DNS Push/first
> step in a DNS Push/
> 

Fixed.

> Page 15, last paragraph: Why MUST the server immediately terminate a connection
> in this situation? Just accepting the request seems safe - having subscription
> requests show up for the same name seems nearly idempotent, and only one PUSH
> would result from having multiple such subscriptions. Is this close an attempt
> to avoid resource denial attacks buy some node subscribing many times to the
> same thing? That feels extreme, especially since tearing down the connection
> would cancel other subscriptions the client already has established on that
> connection.

The discussion about case insensitivity here is a side note. The main point is that if you receive a subscription for something you already have a subscription for, the two sides are out of sync. There is no protocol mechanism built in to regain synchronization and the best way is then to close the connection and try again. The most common reason that the two ends are out of sync is likely software bugs and if this keeps occurring, the administrator will begin to see a pattern of connections closing and can report the problem.

> 
> Page 16, second paragraph: I suggest replacing the second sentence with
> something like "A name in a SUBSCRIBE message that matches only a literal CNAME
> in the zone will only receive notifications of changes to the CNAME (assuming
> the subscription asks for that type), and nothing else."
> 

The SUBSCRIBE contains the record type not just a name. The point is that a subscription of a CNAME is only to the CNAME record and not to the record it points to. How about:

"Aliasing is not supported. That is, a CNAME in a SUBSCRIBE message matches only a literal CNAME record in the zone, and not to any records referenced by the owner name."

> Page 23, top of page: Since section 4 restricts this protocol to TLS over TCP,
> the "(or equivalent for other protocols)" phrase should be removed.

Good catch. I removed all instances of "(or equivalent for other protocols)”

Thanks!

Tom

---

From: Robert Sparks <[email-masked]>
Date: Tue, 2 Jul 2019 14:55:42 -0500

inline

> "Connection setup over TCP ensures return reachability and alleviates concerns of state overload at the server which is a potential problem with connection-less protocols using spoofed source addre…
wfm
> The discussion about case insensitivity here is a side note. The main point is that if you receive a subscription for something you already have a subscription for, the two sides are out of sync. T…
At a high level, we're talking about what to do when something violates 
protocol. I'm not going to argue for a change, but I find issuing an 
error when asked to make things like X when they are already like X 
suspect. The idea that you can treat this as a symptom of being out of 
sync and that a reset would fix it would be more compelling if you 
weren't basing this on TLS over TCP - I can only see it being a problem 
that must recur and you're choosing to have it be noisy and thrashy to 
expose the problem. I guess that's ok, though I worry about the intended 
deployment environments not really having an administrator to notice 
there's a problem.
> "Aliasing is not supported. That is, a CNAME in a SUBSCRIBE message matches only a literal CNAME record in the zone, and not to any records referenced by the owner name."
Sure

---

From: Tom Pusateri <[email-masked]>
Date: Tue, 2 Jul 2019 17:24:10 -0400

> server as described at the bottom of page 10).

The only time NOTIMP is returned is in the case where DSO stateful operations (over which DNS PUSH runs) is not supported. This error code is due to the fact that DSO uses a different Opcode than the standard Query/Response Opcode. DSO defines a new opcode. The effect of this is that NOTIMP is the correct response if a server doesn’t support the new Opcode.

Similarly, if DSO is supported but DNS Push TLV type is not supported, we return a new RCODE for DSO type not implemented DSOTYPENI.

These are both required by existing specifications.

If a client begins the connection with a DSO Keepalive to a resolver and the resolver accepts the connection, subsequent SUBSCRIBE operations that return NOTIMP will be obvious that the authoritative server does not support DSO.

However, if the client begins with a SUBSCRIBE and receives NOTIMP from the resolver, it’s not clear if the NOTIMP was originated by the resolver or the authoritative server.

Right now, we allow either DSO TLV to setup the state. In the case that the client begins with a SUBSCRIBE and the resolver returns NOTIMP, the client should attempt a connection with the authoritative server directly as described in 6.1. If the authoritative server returns NOTIMP, then PUSH notifications are not possible.

While we could be more explicit about every error case, I don’t think the document says anything wrong here.

Thanks,
Tom

---

From: Robert Sparks <[email-masked]>
Date: Wed, 3 Jul 2019 09:45:32 -0500

> However, if the client begins with a SUBSCRIBE and receives NOTIMP from the resolver, it’s not clear if the NOTIMP was originated by the resolver or the authoritative server.

And I think that's a problem.

What does a NOTIMP mean to the client? Most of the draft says "The 
server doesn't implement DSO". It doesn't say "doesn't implement DSO for 
this particular set of bits in this query". Section 6.2.2 says the 
client should assume a retry delay of 1 hour before talking to the 
server (the resolver) again.

Now, other parts of the document imply "for this particular set of bits" 
- in the overview, near the bottom of page 5, it says to use NOTIMP 
(actually it says NOTIMPL, maybe those are different things and I'm 
confused?) if a message is received for a class other than "IN" and the 
server has only implemented push for "IN". Again, that "assume a retry 
delay" kicks in.

> Right now, we allow either DSO TLV to setup the state. In the case that the client begins with a SUBSCRIBE and the resolver returns NOTIMP, the client should attempt a connection with the authorita…
But there's fate sharing, at least as I'm reading the document now. That 
resolver could have been authoritative (or could reach an authoritative 
server) for some other subscription. But you've told the client that 
it's not ok to talk to this resolver for any subscription (for an hour 
at least).
> While we could be more explicit about every error case, I don’t think the document says anything wrong here.
And I used NOTIMP as an example. I think there may be fate sharing 
issues with other response codes as well.

---

From: Ted Lemon <[email-masked]>
Date: Wed, 3 Jul 2019 11:12:11 -0400

On Jul 3, 2019, at 10:45 AM, Robert Sparks <[email-masked]> wrote:
> What does a NOTIMP mean to the client? Most of the draft says "The server doesn't implement DSO". It doesn't say "doesn't implement DSO for this particular set of bits in this query". Section 6.2.2 says the client should assume a retry delay of 1 hour before talking to the server (the resolver) again.
> 
> Now, other parts of the document imply "for this particular set of bits" - in the overview, near the bottom of page 5, it says to use NOTIMP (actually it says NOTIMPL, maybe those are different things and I'm confused?) if a message is received for a class other than "IN" and the server has only implemented push for "IN". Again, that "assume a retry delay" kicks in.

Robert, first, thanks for doing a really thorough review of this document.  This is much appreciated.   This particular insight is one that I think is really important.   I think your initial take on this is correct: if a resolver receives NOTIMP or DSONI from upstream, what this means is “fail,” not “not implemented,” from the perspective of the resolver.   The resolver knows what the client is asking, and can’t do it.  That’s SERVFAIL, not NOTIMP or DSONI.

I think your subsequent point about terminating connections is also correct: we do not want a billion broken clients hammering on servers.  However, the DSO document actually specifies how to deal with this: the server can tell the client to shut up for a period of time, and this is explicitly recommended for situations like this.   The advantage of failing in cases like this is that it causes the implementation to not work, which then motivates the implementor to fix it.

And thanks for the advice about how to terminate TLS connections—I had missed that nuance.  Are TLS implementations actually able to do this (to reject RST packets)?

---

From: Tom Pusateri <[email-masked]>
Date: Thu, 4 Jul 2019 12:52:25 -0400

> On Jul 3, 2019, at 11:12 AM, Ted Lemon <[email-masked]> wrote:
> 
> On Jul 3, 2019, at 10:45 AM, Robert Sparks <[email-masked] <mailto:[email-masked]>> wrote:

a. If the client sends SUBSCRIBE or KEEPALIVE to the resolver and the resolver doesn’t support DSO, it will send NOTIMP whether you want it to or not because it doesn’t know about the opcode.

b. If the client sends SUBSCRIBE and gets back DSONI from the resolver because it supports DSO but not PUSH Notifications, the resolver is following the DSO spec.

c. If the client sends KEEPALIVE and successfully establishes a DSO connection to the resolver and then sends SUBSCRIBE and the resolver gets back NOTIMP from the authoritative, and returns this to the client, then the client knows exactly that the problem is that the authoritative doesn’t support DSO.

To solve the ambiguity:

1. The client should only send SUBSCRIBE to set up the DSO session if it has gone through the discovery process and wants to talk to the authoritative directly. If it wants to try to subscribe to push notifications through a resolver, then it should use KEEPALIVE first to establish the DSO connection with the resolver.

2. If the resolver supports SUBSCRIBE but the authoritative doesn’t, the resolver should not send DSONI back to the client because the client can’t tell the difference between the resolver not supporting SUBSCRIBE and the authoritative not supporting SUBSCRIBE. In this case, the resolver should return SERVFAIL. This should inform the client that the authoritative doesn’t support SUBSCRIBE. If there are multiple authoritative servers supporting _dns-push._tcp, the resolver may want to try all of them before returning SERVFAIL.

> And thanks for the advice about how to terminate TLS connections—I had missed that nuance.  Are TLS implementations actually able to do this (to reject RST packets)?

This was actually a comment from David Schinazi (and a good one). I’ve adjusted the working copy on github but there’s still one section I’m wrestling with regarding TCP RST.

Thanks,
Tom

---

From: Ted Lemon <[email-masked]>
Date: Thu, 4 Jul 2019 14:13:16 -0400

> 1. The client should only send SUBSCRIBE to set up the DSO session if it has gone through the discovery process and wants to talk to the authoritative directly. If it wants to try to subscribe to p…

This creates a new semantic connection between KEEPALIVE and SUBSCRIBE.  This means that DSO implementations now have to track more state.  I don’t think there’s a benefit to doing this.  Why do you think it’s necessary?

> 2. If the resolver supports SUBSCRIBE but the authoritative doesn’t, the resolver should not send DSONI back to the client because the client can’t tell the difference between the resolver not supp…

There’s no need to mention DSONI here.  Just say what the right behavior is.  If you mention DSONI here, someone might read this to suggest that in some case DSONI would make sense.

SERVFAIL just means that the server is unable to support the requested behavior. It doesn’t signal why. Unless there’s something the client would do differently, I don’t think it’s necessary for the client to know why the request failed.

---

From: Tom Pusateri <[email-masked]>
Date: Thu, 4 Jul 2019 15:32:52 -0400

> On Jul 4, 2019, at 2:13 PM, Ted Lemon <[email-masked]> wrote:
> 
> On Jul 4, 2019, at 12:52 PM, Tom Pusateri <[email-masked] <mailto:[email-masked]>> wrote:

Do I already have a session or not with my resolver is not a lot of extra state. Especially when it’s over TLS and you are likely to keep the session up for some time and then let it expire. This state is necessary regardless.

I was trying to give the client enough information to determine WHY it failed. If we determine this isn’t important, we can just let the client try to figure it out but it will be more work for the client.

> SERVFAIL just means that the server is unable to support the requested behavior. It doesn’t signal why. Unless there’s something the client would do differently, I don’t think it’s necessary for th…

It’s likely that resolvers aren’t going to support this before authoritative servers are and clients will quickly learn their resolver isn’t capable and go directly to the authoritative for some period of time.

Tom

---

From: Ted Lemon <[email-masked]>
Date: Thu, 4 Jul 2019 15:54:04 -0400

> I was trying to give the client enough information to determine WHY it failed. If we determine this isn’t important, we can just let the client try to figure it out but it will be more work for the…

The client is a computer program, not a person, so there is no chance that it will be able to figure out what went wrong! :)

Seriously, though, what’s the strategy the client should follow in this case?  I think we generally say “try again in an hour” but I’m not sure if we said that explicitly here or just in the DSO document.

> It’s likely that resolvers aren’t going to support this before authoritative servers are and clients will quickly learn their resolver isn’t capable and go directly to the authoritative for some pe…

I think that how resolver support for this will work is an open question right now, which will probably have to be addressed in a follow-on document.  At present, the implementation I’ve done doesn’t even attempt the local resolver, because I couldn’t figure out how to implement that.  I’m assuming that in most cases there’s no particular benefit to using the local resolver, because the auth server will also be local.

---

From: Tom Pusateri <[email-masked]>
Date: Thu, 4 Jul 2019 16:09:09 -0400

Maybe we could have a conference call and hash this out. Anyone is welcome. 

I’m ok with removing resolver support for now to simplify this and tackle resolvers in another document. But if there’s a good solution we’re all happy with, we can fix it now. 

Tom

> On Jul 4, 2019, at 3:54 PM, Ted Lemon <[email-masked]> wrote:
> 
> _______________________________________________
> dnssd mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dnssd

---

From: Ted Lemon <[email-masked]>
Date: Thu, 4 Jul 2019 17:30:39 -0400

> I’m ok with removing resolver support for now to simplify this and tackle resolvers in another document. But if there’s a good solution we’re all happy with, we can fix it now. 

I think we’ve specified the parts of resolver support that we understand, and it would be a shame to take those out.   I don’t think we need to make this any more complicated than that—we’d talked about the resolver support issue during WGLC, and were satisfied.   Simply addressing the point Robert raised should be sufficient.