Re: [dnssd] WGLC for draft-ietf-dnssd-srp

[Tom Pusateri <[e-mail]>]

I’m breaking this review for draft-ietf-dnssd-srp into two sections.

  1. overall architecture
  2. latest revision of the document

I like the new distinction between constrained nodes and regular nodes. This helps.


Section 1.
==========
As we’re moving from the local link with mDNS to internet wide service discovery over unicast DNS, I think it is necessary to ensure all DNS messages that exit the local link be encrypted.

	a. This document purports to provide “a reasonably secure mechanism” that mentions no encryption. Encryption is desirable for privacy with both link local and internet wide dns messages. While legacy mDNS does not perform encryption, it is possible to encrypt link-local announcements as well as internet wide area announcements, queries, and responses.

	b. It provides limited authentication through “first come, first serve” semantics for change modifications but not for the the initial registration of any service.

	c. It punts access control to the local administrator based on IP address.

	d. It suggests the use of TCP to ensure client address verification but then also seems to suggest UDP is ok too for RFC 2136 unconstrained nodes.
	e. The use of anycast requires guarantees about network routing operations that cannot be detected by SRP servers or clients. SRP Updates sent to an anycast address can leak outside the domain through a default route and even to another domain’s SRP servers. Accepting these over UDP and requiring access control to be configured by the same administrators that haven’t set up anycast routing correctly seems like a bad idea. Random users can install devices configured to use the anycast address out of the box in a network not anticipating the deployment of SRP.

	f. describing an architecture where the DNS SRP server is not typically on the same link but then requiring it to be so in order for the sleep proxy to function as described suggests maybe the SRP architecture needs to change. (section 2.6.2)



Section 2.
==========
document comments

Section 2, paragraph 2:
	There are restrictions placed on the domain name to use including RFC 2119 terminology requiring it but there is no justification for the imperatives (both in the v4 and v6 case). This leaves the reader wondering why.

Section 2, paragraph 3:
	_dnssd-srp._tcp<zone> is defined (add a period before <zone> as in 2.3.2). Can there also be a _dnssd-srp-tls._tcp.<zone>?

Section 2, paragraph 4:
	The wording makes it seem like “default.services.arpa” is defined in RFC 6761

Section 2.1, paragraph 1:
	The update terminology seems overloaded and a bit confusing in the document overall. In this particular paragraph, the second sentence is confusing with updates in the update that are records.

Section 2.1, paragraph 3:
	the use of “both” follows a list of items making it hard to discern what both refers to. It could be:
		1. the three items in the list (but then both implies 2 items not 3)
		2. one or more TXT RRs (but more than 2 also isn’t usually referred to as both)

Section 2.1, last paragraph:
	KEY records are mentioned above but the details of these aren’t explained in RFC 6763

Section 2.3, first paragraph:
	“it’s possible to do all the work of adding a PTR resource record to the PTR RRset on the Service Name if it already exists, or creating one if it doesn’t”. This is confusing because each service instance adds a PTR Service Name record identically whether the RRset exists or not.

Section 2.3, first and second paragraph:
	First it says “it is possible” and then it says “An SRP update is therefore implemented”. But the logic between these end caps is not explicit and so it reads awkwardly and reader is left wondering what the “therefore” is there for.

Section 2.3, last paragraph:
	The document uses “update constraints” but should in this case use "update pre-requisites" since it’s referring to the way 2136 does things. The second use of constraints referring to section 2.4.2 seems correct.

Section 2.3.2:
	This section seems out of place because it is only for testing (manual client key installation etc.) and so should be moved to an appendix.

Section 2.3.3, paragraph 2:
	“as described earlier using _dns-update._udp” - I don’t see any other references to _dns-update._udp. Also, RFC 2136-compliant servers shouldn’t be restricted to UDP. They can support TLS, TCP, or UDP. (_dns-update-tls._tcp, _dns-update._tcp, and _dns-update._udp).

	But earlier in 2.0, second to last paragraph, you require TCP to prevent off-network spoofing so UDP should not be suggested here.

Section 2.4, last paragraph:
	“what we describe here improves upon the security of mDNS. The goal is not to provide the level of security of a network managed by a skilled operator.”
	In fact, we want to design protocols that provide security, privacy, and protect users regardless of the skill of the operators. mDNS has a level of protection afforded to it by it’s link local nature. Once SRP leaves the link, it is open to a whole new host of vulnerabilities and it has to address each of these in a way mDNS never had to.

Section 2.4.1.1, third paragraph:
	“service description updates” - it’s not clear here what “updates” means. This is part of the overloading of update in the document.

Section 2.4.1.1, fifth paragraph:
	“it makes a claim” - the paragraph discusses the option to use different lease life times and then ends with a declarative that it makes a claim that lasts much longer when it fact it doesn’t have to. I suggest “it could make a claim”.

Section 2.4.2:
	This section contains the meat of the “update” confusion. Service Discovery update, Service Description update, and Host Description update need different terminology to distinguish an update message, and SRP update, and these combination of records, that combined in a specific way, comprise an SRP update. Maybe Service Discovery group or bloc? And we already have resource records and RRsets so maybe that terminology could be used in places?

	“An update is a Service Discovery update if” - When all of the records required must be in an SRP update, what does “update” mean here?

	“one of more TXT RRset updates” - can this “updates” be “resource records”?

	“there is a Service Instance Name update in the SRP update that updates an SRV RR so that it points to the hostname being updated by this update” - ETOOMANYUPDATES in this sentence

	“Note that if the definitions of each of these update types are followed carefully, this means that many things that look very much like SRP updates nevertheless are not” - It seems that if the definitions of each of these updates are followed carefully, you would end up with a valid update. Stating that you don’t seems to imply that the definitions are not descriptive enough.

	“is the same as the KEY record in the update” - again, it’s not clear what update you are referring to. And can there be multiple KEY records in the same SRP update? Again, the Service Description update vs the Host Description update make this confusing.


	“The server MAY add a Reverse Mapping” - (why is Reverse Mapping capitalized?) - Can the client add this? Would this be a standard RFC 2136 update since the zone is different?

	“maintenance functionality” - provide a reference in the document to what you mean here

	How do you delete an SRP announced service before the lease lifetime of the service name PTR record expires? The rules don’t seem to allow that.

Section 2.5:

	If one service is announced via SRP and another service from the same host is announced over SRP, how do you make A and AAAA record TTLs consistent with both service records?

	“when adding RRs to an RRset, … MUST use the same TTL” - Seems like it would be impossible to achieve this with Service PTR records in an RRset. I’m probably not understanding it so it may need clarification.


Section 2.6.1, second to last paragraph:
	How does a dual-use server decide an update is an SRP update or an RFC 2136 update?


Section 3.2:
	Do you need to specify which key to use for server signature validation? Isn’t this already specified outside of SRP?

Section 6.1:
	Throughout the document you reference default.services.arpa and then in the IANA you request the services.arpa domain. Where is default.services.arpa defined and/or managed? Is it a subdomain or a leaf?


Thanks,
Tom