Last Call Review of draft-ietf-dnssd-srp-20
review-ietf-dnssd-srp-20-artart-lc-faltstrom-2023-06-11-00
review-ietf-dnssd-srp-20-artart-lc-faltstrom-2023-06-11-00
Most of these comments are nits and related to how the document is written. It might be more clear to someone that already implements mDNS than to someone "only" deals with DNS. I think the document is (as you see below) a bit unclear on issues. Whether they should actually be made more clear or not can be discussed. SRP is an acronym that has been in use many times. For example for Spatial Reuse Protocol / Dynamic Packet Transport (SRP/DPT) that can be found for example at Cisco: <https://www.cisco.com/c/en/us/tech/optical/spatial-reuse-protocol-dynamic-packet-transport-srp-dpt/index.html>. I presume the choice will be SRP although there are many acronym collisions...? In section 3.2.1 there is some text that talks about what RR types that are supported. At least for me the text ends up being a bit unclear on whether it describes what is implemented, or what is required by entities being in accordance with this specification. As specified in many DNS related RFCs, protocols should allow any RR Type be used, and limiting protocols to some is by experience a bad thing. For example all crazy use of TXT records because implementations do not handle RR Types properly. I think 3.2.1 must be more clear, specifically second bullet in second paragraph. I also think the specifically is too wordy. Example is section 3.2.3 that starts by explaining how this specification is NOT working by explaining DNS updates. Then it is later explaining how this spec works. This spec should be about how things according to this spec should work. Maybe it is also explaining differences from other protocols, but it should not explain on what and how this is *not* doing things. Makes it harder to implement. In section 3.2.4 there is a claim "This model does not work for automatic service registration.". I do not understand why it is not. I can guess it is "impractical" as there must be a shared secret deployed and various other things, which implies the zero-conf piece of service registration is messy, but "...does not work..."? I also think wording in section 1 (related to 3.2.4) is unfortunate. It says in sixth paragraph "to authenticate both the initial claim and subsequent updates". I do not think it is authenticating the initial claim. It is using the FCFS security model together with (section 6.1), if TCP is used, inability to inject registrations from outside network locations. In section 3.2.4 there is a statement "The goal is not to provide the level of security of a network managed by a skilled operator." which honestly I do not understand what it means and why it is in this text. In section 3.2.5.3 there is a time constraint of lease time of 14 days that is "typical" but it can be chosen to something else. As this is a choice that might impact the security (and reuse of the same name) my question is whether it is not for security reasons necessary to say it MUST be higher than a certain lowest value. In section 6.3 a reference is made to RFC8624. Should be to RFC9157. In 3.2.5.4 there is a claim that compression saves "substantial space". Although it might do, I think this specification should stay at saying "compression MUST be supported". In 3.3.1.3 it is stated that some A or AAAA records can be ignored by the server. I think it would be more proper to say that the server can ignore ANY request due to contents of the update request be "weird". In section 4 about TTLs it is said the request TTL should only be advisory. I understand why the SRP registrar might believe the TTL is too short, but that should be flagged in the response. A client must according to my view be able to request something short so that the client can honor the use of the service for the protected time period.