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

[Esko Dijk <[e-mail]>]

Hello,

Here my response to the WGLC for draft-ietf-dnssd-srp accompanied by review comments for the draft.
In summary, I believe the document apart from Section 5 is ready to be published after the review comments are addressed: as these comments are mostly on editorial issues, text location and resolving potential unclarities. The SRP protocol overall looks useful, solid and well-described.

The specific part of Section 5 on Sleep Proxy has had less work it seems, as a few of my original -04 review comments are still open and I don’t see how this function could lead to interoperable implementations. To resolve this, possible actions are:

  1.  Move the Sleep Proxy definition/details out of this draft, into a separate one. This could progress SRP in the best way.
  2.  Add more details on the Sleep Proxy, to a fully interoperable implementation becomes possible.
  3.  Keep current Section 5 text (only minor additions/changes/fixes), and add a sentence saying that more details can be defined in a future document (e.g. after implementations have been tested – clients vs servers of different parties)
Related question: are any implementations using Sleep Proxy?

Best regards
Esko

Detailed review comments list:

*** Abstract
802.11 (Wi-Fi) and 802.15.4 (IoT) networks
-> add 'IEEE' so "IEEE 802.11 (Wi-Fi) and 802.15.4 (IoT) networks"
As an editorial comment, in general the abstract should not contain more details than the main text.
So typically one would see back these terms again in an introduction.

*** 2.2.1
"RFC 6763 describes the details of what each of these types of updates contains"
-> not everything contained in these updates is explained in RFC 6763. Specifically, the "Key RR" is not in that RFC. Another RFC ref can be added here that defines Key RR as definitive source of information.

*** 2.2.3.1
"SRP clients are allowed to send updates to the generic domain "default.service.arpa""
-> In section 2.1.* it looks as if only constrained hosts can do this. Can full-featured hosts also do this? If not we can say "Constrained-Node SRP clients are ..."   If yes, then I would expect section 2.1.1 to mention this use for the full-featured hosts.

*** 2.2.5.1
duplicated sentence "This key pair MUST be unique to the device."

*** 2.2.5.4
Does the statement here imply that this draft formally updates RFC 2782?
(Question to the WG. Maybe not: since the update only applies in SRP context not for SRV records in general. But maybe yes: RFC 2782 says "Unless and until permitted by future standards action, name compression is not to be used for this field." which would make it proper to refer to the SRP draft as such future action.)

*** 2.2.5.5.2
"update that deletes the PTR record whose target is the service instance name."
-> This looks like a single PTR record. Prior section 2.2.1 mentions that the service instance name may be referred to by multiple PTR records.  So in case of multiple, it is not required to have multiple instructions to delete these multiple PTR records?
(I.e. there's no risk of lingering PTR records that would point to a service instance that's deleted?)

*** 2.3 title
[Nit] -> may give the impression that all SRP server behavior is specified in here.
Somewhere in 2.3 / 2.3.* it could mention that more normative behavior is specified in other sections too, like Sections 3 and 4.

*** 2.3.1
Grammar to be fixed: "Each instruction consists some combination"

*** 2.3.1.1
"if the Service Discovery update is an "Add to an RRSet" instruction, the Service Description Instruction does not match if"
-> for me unclear, see separate mail thread on this section.

*** 2.3.1.2
[Nit] Bullet:
"*  Service Descriptions Instructions do not modify any other RRs."
-> best rephrased to
 "*  no RR updates other than the ones defined above."

That would fit better in the style of the previous bullet points in the list.

*** 2.3.1.3
"If a link-scope address or IPv4 autoconfiguration address is provided by the SRP client, the
  SRP server MUST treat this as if no address records were received; that is, the Host Description is not valid."
-> This seems rather strict. If the client provides 1 routable IP address and 1 link-local address, couldn't the SRP server just ignore the link-local address?
This might have some benefits for future/backwards compatibility , in case of future cases where link-local addresses are to be used for some reason. (If only for debug/diagnostic purposes)
I agree that if the client sends only link-local address(es) then the SRP server treats it as if no address records were received.

*** 2.3.2
[Nit] "must have that Host Description instruction as the target of its SRV record"
-> could rephrase to "MUST have the hostname of that Host Description Instruction as the target of its SRV record"  to be more precise.
Similar comments can be given for the remainder of the section text that talks about instructions pointing to instructions.  (It's more like names in instructions being equal to names that are used in other instructions)
(not sure if MUST or must is desired here by the way)

"For example, a DNS update that contains an RRset
   Add to a Service Name and an RRset Add to a Service Instance Name,
   where the Service Name does not reference the Service Instance Name,
   is not a valid SRP update message"
-> 'RRset Add' refers probably to the "Add To An RRset" semantics of RFC 2136? If so it would be good to use that exact same naming "Add To An RRset" including the quotes here.
With present text I'm wondering if I missed something in the DNS specs that's called "RRset Add".

*** 2.3.3
"If any existing KEY record corresponding to a
   KEY record in the SRP Update does not match the KEY same record in
   the SRP Update "
-> grammar issue. 'match the KEY same record'    And a bit confusing: can a KEY record in an update correspond to a stored KEY record but not match at the same time?
  Perhaps the 'name' concept should be introduced here to clarify. (Presumably the server will check if an existing Service Instance Name or existing Hostname is stored and look up the associated KEY for that.)

"KEY record updates omitted from Service Description update are
   processed as if they had been explicitly present: every Service
   Description that is updated MUST, after the update, have a KEY RR,
   and it must be the same KEY RR that is present in the Host
   Description to which the Service Description refers."
-> Can we replace here 'Service Description update' by 'Service Description Instruction' ?
E.g.
"KEY record updates omitted from Service Description Instructions are
   processed as if they had been explicitly present: every Service
   Description that is updated through a Service Description Instruction MUST, after the update, have a KEY RR,
   and it must be the same KEY RR that is present in the Host
   Description Instruction to which the Service Description Instruction refers."

"Otherwise, the server validates the SRP Update using SIG(0) on the
   public key in the KEY record of the Host Description update."
-> what about:
"Otherwise, the server validates the SRP Update using SIG(0) against the
   public key in the KEY record of the Host Description Instruction."

(Using 'Instruction' and 'validates X against Y' instead of 'validations X on Y' )

*** 3
"SRP update servers MUST check"
->"SRP servers MUST check"

*** 4.1
[Nit] " MUST be removed at the same time-it is never valid for a service"
-> " MUST be removed at the same time -- it is never valid for a service " or
-> " MUST be removed at the same time: it is never valid for a service "

3rd paragraph mentions that a SRP is by default configured with a 2 hour Lease limit and 14-day KEY retaining time limit; and then it says that constrained devices may negotiate longer leases. This looks incorrect, as leases beyond configured upper limits won't be given by the server.
Probably something else is meant here with 'negotiate'? For example if one configures 'default values' for Lease and Key-Lease in the server, the serve may extend to these default values if the client asks for something shorter. If the client asks for something longer than the 'default values', the server grants the longer values up to a hard limit that is also configured in the server.

*** 5 Sleep Proxy
The introduction text should clarify whether Sleep Proxy support for an SRP server is OPTIONAL or MANDATORY.
Also to make more clear that for clients the use of this function is optional/OPTIONAL. (The last paragraph says so for constrained hosts, but nothing for the full-featured hosts.)

In relation to above, the same questions from my review of -04 of 2020-10-13 still apply:
- How would a service’s host know that the server is capable to do this? If it doesn't know then it cannot trust the mechanism to work and go to sleep.
- Or, is there a way for the SRP server to reject an update with EDNS(0) OWNER option such that the service’s host knows not to use it again?

[Nit] "Another use of SRP is for devices that sleep to reduce power consumption"
-> ideally it should mention somewhere that the device that sleeps is an SRP client. Otherwise the statement is a bit too generic. E.g.
"Another use of SRP is for SRP client devices that sleep to reduce power consumption, while being able to offer registered service(s) without interruption."

"the device includes an EDNS(0) OWNER Option [I-D.cheshire-edns0-owner-option]"
-> the reference here is Informational; how can this be? To implement the sleep proxy function it is normative to include the option.

"When the DNS server receives a TCP SYN or UDP packet addressed to"
-> "When the Sleep Proxy receives a TCP SYN or UDP packet addressed to"
(see also below comment)

"the Sleep Proxy must be on the same link"
-> the term "Sleep Proxy" is not introduced. In the prior text when it says "should set up a proxy" the name could be formally introduced e.g. "should set up a Sleep Proxy" or "should set up a proxy, called the Sleep Proxy, "

Overall, I miss some details about what the Sleep Proxy does once it receives a TCP SYN or UDP packet and it has woken up the sleepy device. Does it then stop the ARP/ND proxying for the sleepy device right away? And when would it again resume proxying?
What does the proxy do with the receive TCP SYN or UDP packet, discard or forward to the woken sleepy device? (After how much waiting time?)

*** 6.1
"Service Discovery Protocol servers"
-> is this term defined? Should we say SRP servers here?

"a promise made by the registration protocol"
-> "a promise made by the service registration protocol"

"replacing all or part of the service registration information with information provided by an SRP client"
-> the "SRP client" mentioned here I think should be "DNS client". Because the case mentioned looked like an authenticated (non-SRP) DNS client making DNS updates to records that were added by an SRP client.

*** 7 "Privacy Considerations"
This text mentions the requirements for TLS. Perhaps the TLS bits are better placed in Section 6, and Section 7 if needed can refer back to that. To me TLS seems one of the key ingredients to secure the protocol so I'd expect to read something / most-things about this in Section 6.   (Maybe I'm wrong here in case TLS can't provide the authentication - since in some deployments the client may have no trust anchor of the domain installed a priori, so it can't authenticate the SRP server even when TLS is used.)

*** 9.2
"Availability of DNS Service Discovery Service Registration Protocol
   Service for a given domain is advertised using the
   "_dnssd-srp._tcp.<domain>."  SRV record gives the target host and
   port where DNSSD Service Registration Service is provided for the
   named domain."

-> seems like a grammar issue here. Was this meant as a single sentence, or as 2 sentences separated by a dot before the "SRV record" text?
E.g. I could interpret as "using the X SRV record which gives the Y" or as "using the X name. The SRV record gives the Y".
Similar point for 9.3.





From: David Schinazi <[e-mail]>
Sent: Monday, August 16, 2021 20:09
To: Esko Dijk <[e-mail]>
Cc: DNSSD <[e-mail]>
Subject: Re: [dnssd] WGLC for draft-ietf-dnssd-srp

Hi Esko,

That's reasonable, we can wait a few more days for you to review.

David

On Sun, Aug 15, 2021 at 1:37 PM Esko Dijk <[e-mail]<mailto:[e-mail]>> wrote:
Hello David,

Due to holiday I missed this last call. Having reviewed an earlier version I would like to check if the issues are solved now; and can answer the question by Tuesday or Wednesday if that’s ok. (Instead of today)

Best regards
Esko

From: dnssd <[e-mail]<mailto:[e-mail]>> On Behalf Of David Schinazi
Sent: Wednesday, July 28, 2021 23:57
To: DNSSD <[e-mail]<mailto:[e-mail]>>
Subject: [dnssd] WGLC for draft-ietf-dnssd-srp

Hi DNSSD enthusiasts,

This email starts an official Working Group Last Call for draft-ietf-dnssd-srp. This call asks whether we believe the document is ready for publication. As such we are asking two questions:

1) if you believe this document is not ready to publish, please speak up now

2) if you have read the document and think it is ready, please say so as well

For this call to succeed, we'll need statements of explicit support from people who have read the draft. Please send statements in either direction as responses to this email. This call will be open until 2021-08-15 23:59 UTC.

Thanks,
David