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

[Esko Dijk <[e-mail]>]

Thanks,

I reviewed the new (Github) text.  Most comments are well addressed, here a few remaining points:


  *   New section “Subdomains of 'service.arpa.'” -> this section reads as if it should be placed within the “IANA Considerations” section as a sub-section, or at least be referenced from the IANA section?  Now it is outside it and not referenced.


  *   “increase the power required” -> “increase the energy required”

>> -> “present in the Host Description Instruction to which the Service Description Instruction refers”
> why
Oops, maybe my text proposal was actually not good! We can leave it as is.  One Instruction also doesn’t reference another Instruction, but references data.
One trigger was that the terms “Host Description” and “Service Description” are used and gradually introduced in the document, but there’s not a formal/specific definition. The draft overall suggests that a “Host Description” can mean the data in the Host Description Instruction, or that it can be data in the SRP Registrar’s local database (which resides there as a result of processing previous Host Description Instruction(s)). Similar for “Service Description” – may be the data of a Service Description Instruction, or data about a service already present in the database.


> However, you’re right that this is a problem, because if it does a pattern match, EXPLETIVE-4387 is going to fail just as badly as EXPLETIVE did. So maybe it should return REFUSED in this case.
Yes, I agree with “Refused” in case the adding of new numbers at the end by the SRP Requestor (e.g. EXPLETIVE-3, -4, -4387 etc) would not make any difference for the name filter.  Seeing “Refused”, a Requestor knows that it could retry with a completely different name, in this case. (Contrasting to YXDomain:  that signals the requestor to try with changing/adding some number at the end of the name i.e. retry with a similar name.)

I do think that if the server has a very specific name filter (e.g. on “EXPLETIVE-1*” only) it could just configure for that the YXDomain answer, triggering the Requestor to try again with another name (e.g. “EXPLETIVE-2”) and that would likely succeed.   So that would be allowed, but seems a corner case that we don’t need to spend any text on in the draft; something an implementer could just use if required.


> That title doesn’t make sense. We aren’t cleaning up the lease time. I this this is fine as is.
Right, it can easily be read in a way that makes no sense. (I was intending the other way.)  So let’s keep as is.


> Send text?
I don’t think we need additional text for 6LoWPAN Border Routers (6LBRs) here, the phrasing is generic currently for “routers” so IP routers, which includes such 6LBRs.


Regards
Esko

From: Ted Lemon <[e-mail]>
Sent: Tuesday, August 23, 2022 06:47
To: Esko Dijk <[e-mail]>
Cc: David Schinazi <[e-mail]>; DNSSD <[e-mail]>
Subject: Re: [dnssd] Second WGLC for draft-ietf-dnssd-srp

On Aug 18, 2022, at 9:16 AM, Esko Dijk <[e-mail]<mailto:[e-mail]>> wrote:

Thanks, Esko! Comments below, pull request is here:

https://github.com/dnssd-wg/draft-ietf-dnssd-srp/pull/10


*** Open issues

2.2.1
“The Service Instance Name MUST be referenced by one or more
      Service Discovery PTR records, unless it is a placeholder service
      registration for an intentionally non-discoverable service name.”
-> This bit is a fairly detailed requirement, with a hard-to-grasp exception case also. Why not include such requirements only in 2.3.x which anyway has all the formal rules? I thought 2.2.1 was more like an intro sketching at high level what it looks like, and 2.3.x has details. The MUST requirement is already present in 2.3.2.
The exception case seems not listed there – is that a bug?

Yikes. Actually this whole section says way too much, and some of it is wrong, including this text that you’ve called out.



“The SRP requestor SHOULD follow the advice given in Section 5.5.2 of [RFC8766]” -> this automatically makes RFC 8766 a normative reference, per the IETF rules for that. (Even if it was a MAY this would be the case. The point is that an implementer OPTIONALLY will use the rules and if so, 8766 becomes normative on how to do these rules. ) So, ok to make RFC 8766 normative?

I think given that we have applications where this advice is not what’s specified (e.g. Thread), we shouldn’t make the 8766 behavior normative. I’ve updated this text to refer to it as an example rather than normatively.


2.2.3
“so there is no need for the server sending the SRP Update” -> “no need for the SRP requestor” ? “server” seems wrong here. Sorry this is almost a nit but I don’t think we should rely here on reader’s internal autocorrect…

This escaped my review of the server->requestor changes. I think it should say “SRP requestor” rather than “server sending the SRP update.”


2.2.5.2
“So for instance, it might try
   host.service.arpa, then host-1.service.arpa, then host-
   2.service.arpa, then host-31773.service.arpa.”
-> here the domain should be “default.service.arpa”? If so, it must be fixed to avoid interpretation errors (does the implementation also need to be fixed for this?). If not, then there’s a bunch of clarification missing why “default” can be omitted here. (But I assume that doesn’t apply here.)

Oops, nice catch. Not really a serious problem, but you’re right to ask for consistency here.


2.2.5.4
“Although [RFC2782] requires that the target name in the SRV record
   not be compressed, an SRP requestor SHOULD compress the target in the
   SRV record.”
-> This means RFC 2782 is formally Updated. The draft should indicate this in the usual IETF way at the top as “Updates: 2782”.

It actually doesn’t mean that. It means that SRP requestors and registrars have to support compressing of SRV record targets. No other DNS requestor or responder is so constrained. I think this has come up before, actually. I think that the RFC should be updated, but it’s not really our bailiwick.


2.2.5.5.1.
The Method 2 of only removing the Host record is still a bit unclear – as explained in my previous email about “Host instructions with no address”.
Text says “Therefore, it is sufficient for the requestor to send the Host Description Instruction” but the latter requires a “Service Description Instruction” to be present which contradicts the stated.

This has been addressed in an earlier pull request.


2.3.1.3
Last bullet - See previous email on “Host instructions with no address” ; it would seem we do have a case where the Service Description Instruction can be absent in the SRP Update – if lease-time=0.  All the host’s services would then get auto-removed.

Also I think addressed in the earlier pull request, but let me know if you disagree.


2.3.5
“and can be either SUCCESS or SERVFAIL” ->
In the IANA registry it’s listed as NoError instead of SUCCESS. RFC 1035 calls it “No error condition”. So we could adopt ‘NoError’ ? (Since the original spec doesn’t use CAPS for the name)

Ouch, there were actually a lot of those. I think I got them all. Thanks for pointing this out.


4.1
See section 11 ref update.

“SRP registrars SHOULD also track a lease time per service instance”
-> this seems problematic to have as a ‘SHOULD’ because you can’t count on the SRP Registrar doing it properly.
E.g. if this is not done, you could get the following situation:


  *   At time t=0, requestor registers service X with lease = 10 mins
  *   At time t=5 min, requestor registers service Y with lease = 8 hours

     *   At this time the Registrar extends the life of the host and all its services to 8 hours (since it doesn’t track lease time per service instance)

  *   (Requestor expects at time t=10, that service X will time out)
  *   At times t=10 min – t=8 hours, service X remains in the Registry unintentionally.

So better is to either do it (MUST) or don’t do it at all. In the first case, partial updates like in the example above and below are possible. When not doing it, partial updates should typically be discouraged since it gets tricky to get it right.

Argh. You’re right. This MUST be a MUST.



Another example of ‘partial update’ enabled – this situation works even when the SRP Registrar doesn’t track lease time per service :


  *   requestor registers service X at time t=0 for a lease time of 8 hours ;
  *   at t=1 hour it registers service Y for a lease time of 1 hour.
  *   at times t=1 to 2 hours, both services X/Y are active. And at  t=2 hours, both services X/Y and the host will time out at the same time if the requestor doesn’t renew before that.

So in this case the first lease time 8 hours applies to the host and to service X, and the second 1-hour lease time applies to the host and to service Y, not directly to service X (which still has 7 hours to go) but due to the host removal service X is also removed at time T=2 hours.

What is perhaps missing/unclear in the spec if the SRP requestor is even allowed to do such fractional updates where the SRP Update doesn’t contain all the registered services.  I don’t think this is mentioned anywhere.

Matter is assuming that this is allowed, and I think the text you quoted admits that it is allowed, so I think we’re okay here. If you think we need to say more, can you suggest text?


“The times may be
   shorter or longer than those specified in the SRP Update; the SRP
   requestor must honor them in either case.”
-> Is it an idea to also require (“SHOULD”) a configuration mechanism here just like for the TTL to set a Min and Max ?  For example, in a network with sleepy devices a longer allowed lease time Max could be configured on the SRP Registrar.
And if a network is getting busy with registration traffic, the Registrar could be reconfigured to have a longer Min lease time.

This all seems clearly to be within bounds based on the text you’re quoting. I’ve added “by the registrar” after “shortened or lengthened” to make this clear. I don’t think this document should specify behavior, but I’m open to arguments to the contrary.


8.1
Should we say anything about the subdomains under “service.arpa”? In this document we use “default.service.arpa”, but what about in the future when someone wants to use “myname.service.arpa” etc ? Is there a registry, or is an RFC needed to define these names?
Maybe this is more of a question than an open issue for the document.  But if we know how this is handled we could just mention it briefly.  I had a case in the past in IETF where a registry was forgotten where it would have been handy.

Ick. Yeah.


11.
Ref update https://datatracker.ietf.org/doc/html/draft-sekar-dns-ul-03  by https://datatracker.ietf.org/doc/html/draft-ietf-dnssd-update-lease-02

Yup.


12.
RFC 8310 is informative, however it defines a profile that updates 7858 normatively. So shouldn’t it be in ‘normative’ section 11? If not, does that mean that nothing of the profile of 8310 is used in any implementation?
(Maybe it doesn’t matter much, since 7858 is normative and that one is being automatically updated by 8310 whether we like it or not.)

This is a bit touchy. We don’t actually require (nor can we) any of the profiles in 8310, since our SRP registrar may not even be able to use ACME to get a validatable CERT. I’ve taken our the reference to 8310.


RFC 1035 needs to be normative – impossible to implement SRP without using e.g. the record formats defined there.
Same for 2782 (need SRV records to implement), and 2181.
(Again I may be wrong here – maybe 2181 is automatically normative if we make 1035 normative and it doesn’t matter where it’s placed.)

No, you’re right.


*** Improvements/nits/editorial
Entire doc
“SRP Update” / “SRP update” used interchangeably without apparent reason(?)

True, but I’m going to leave for the RFC editor, so that this pull request isn’t full of chaff.

1.
First occurrence of “registrar” does not define the term. There’s also no terminology section to look it up. This term should be defined in Section 1 ideally. Section 2.0 doesn’t really define it either, it only mentions it in terms of “most likely”.

I added a parenthetical. I think adding a terminology section at this point is too big a change unless we have a serious problem.



2.
“SRP requests” -> better to use SRP Updates here, since “SRP requests” is only used once so is probably not the intended term. Or, “SRP Update requests”.

Yeah, that’s just a mistake.

2.1.1
Mention “default registration domain” first and then calls it “default registration zone”. Is this correct? 6763 calls it only “domain”.

Good point. Should be domain.


Nit: “using SOA queries [RFC8765]” -> it would be good to refer to a specific section here, e.g. 6.1

Yup.


2.1.3
“The reason for these different assumptions” -> “The reason for these different variants”
Nit: The standalone words “power” can all be changed to “energy” to be more correct.
“By requiring the use of TCP,” -> clarify that this applies to full-featured hosts only. Is that intended, or do constrained devices also require TCP?
I’ve rephrased that. The reason to require TCP is to prevent spoofing when the SRP update is traveling across multiple hops.


2.2
“We will discuss … and how to maintain the information … “ -> part of the “maintain” part is in section 4, so it’s a bit strange to announce it here as part of 2.2.x. Maybe add a forward pointer to 4 here also.

Ooh, good point.



2.2.3.1.
Last bullet -> add “.” at end

check


2.2.4
“TSIG protocol” -> add informative reference to [rfc8945]

Good point!


2.2.4.1
“using DNS-SD Registration protocol” -> “using the DNS-SD Service Registration Protocol”
“First-Come First-Serve” -> “First-Come First-Serve (FCFS)” – there’s a need to introduce the acronym here

Yup.


2.2.5
Title “Service Behavior”  -> shouldn’t this be “SRP Requestor Behavior” ? Since we just before talked about “registration service” the term “service” here is unclear.

Sure.


2.2.5.1
“"factory reset."" -> “"factory reset".”

The former is correct.


2.2.5.2
“Service Description Records” -> set ‘R’ to lowercase

OK


2.2.5.3
“ The lifetime of the DNS-SD PTR, SRV, A, AAAA and TXT records [RFC6763] uses the LEASE field”
-> we could clarify that it also holds for other record types. E.g. for everything that’s not KEY, the LEASE field is used.
Since 2.2.1 specifies that other record types may be used for Service Description and 4.1 specifies this too. Maybe it’s enough we say here “DNS-SD PTR, SRV, A, AAAA, TXT and other non-KEY records” or give a pointer to 4.1.

We don’t actually support this, so not needed.


2.2.5.5.2
“these will be seen as a single Service Description Instruction” -> could clarify what “these” refers to here. I didn’t immediately parse it. I assume it refers to both mentioned *-RRset updates. So “these updates will be seen … “

How about:

The difference is that because the target for the PTR record in the Service Discovery Instruction is the same for both the Delete An RR From An RRset update and the Add To An RRSet update, there is no way to tell whether they were intended to be one or two Instructions.

2.3.1.3
“hostname being updated by this update” -> more correct seems “hostname being updated by this instruction”.

That text got deleted by a previous PR.

2.3.2
“Every Service Description
   Instruction must have that Host Description Instruction as the target”
-> MUST

Also deleted


“The target of the SRV record in every
   Service Description Instruction MUST be the single Host Description
   Instruction”
-> “ … MUST be the hostname used in the single Host Description Instruction”  (more correct)

Also deleted.



“RCODE=REFUSED”
-> for style consistency, call it “REFUSED RCODE”.


OK


2.3.3
“present in the Host Description to which the Service Description refers”
-> “present in the Host Description Instruction to which the Service Description Instruction refers”

why


2.3.6
“In order for the registrar
   to add a reverse mapping update, it must be authoritative for the
   zone or have credentials to do the update.”
-> “the zone” here is a bit unclear. We could say “the associated zone” to indicate the zone that’s to be used for the reverse mapping update? Without going into unnecessary details.
Otherwise it could mean “the zone of the SRP Update” which isn’t intended I think.

In order for the registrar to do a reverse mapping update, it must be authoritative for the zone that would need to be updated, or have credentials to do the update.


“The registrar may also have a dictionary of names”
-> MAY
Making it normative is better here to force a requestor implementation to consider this case of (unexpectedly?) getting RCODE YXDOMAIN for a valid SRP Update.

MAY is not normative. The response is the same you’d get for a duplicate name, so the requestor is already required to handle it. However, you’re right that this is a problem, because if it does a pattern match, EXPLETIVE-4387 is going to fail just as badly as EXPLETIVE did. So maybe it should return REFUSED in this case.


4.1
Section title should also mention the “lease time” concept – it is such an important part of this section.
E.g. “Lease Time and Stale Data Cleanup” or “Cleaning Up Stale Data and Lease Time”

That title doesn’t make sense. We aren’t cleaning up the lease time. I this this is fine as is.

5.1
“validation must be enforced by every router on the path from the
   Constrained-Device Network to the unconstrained portion of the
   Network”
-> it’s strange this this only considers constrained-network scenarios. Needs some rewording e.g.
“validation must be enforced by every router on the path from any potential SRP requestor to the SRP registrar.
   In case of a router that is a constrained device, this validation is recommended but not required.”

I deleted that sentence and added a paragraph referencing RFC7084.

Perhaps 6lowpan Routers generally won’t validate all this; while 6lowpan Border Routers could do more extensive validation.

Send text?


8.2
Second paragraph -> still a grammar error in there

8.3
Second paragraph -> same grammar error. There’s also an extra dot ‘.’ After <domain> here and not in 8.2, why’s that?

These sections were just wrong. See pull request for fix.