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

[Robert Sparks <[e-mail]>]

inline

On 7/2/19 1:36 PM, Tom Pusateri wrote:
>
>> On Jun 28, 2019, at 4:03 PM, Robert Sparks via Datatracker <[e-mail]> 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.”
wfm
>
>> 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.
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.
>
>> 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."
Sure
>
>> 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
>
>