Re: [dnssd] Dnsdir last call review of draft-ietf-dnssd-update-lease-07

[Ted Lemon <[e-mail]>]

Dave, thanks for the review!

On Thu, Jun 15, 2023 at 2:56 AM David Lawrence via Datatracker <
[e-mail]> wrote:

> For substance, this specification leaves to inference what a server
> should do if it receives multiple dynamic update 'add' operations in
> one message that carries this option.  I expect that you would say
> that obviously the lease time should be applied to all data added in
> the update, but it did take a little extra cognitive overhead for me
> to come to that conclusion.
>
> Along those lines, to be comprehensive you should explicitly specify
> that the data change caused by a 'delete' operation is not covered by
> the Update Lease option.  (I don't think an implementer would really
> do that, but still, explicit is good.)


Good catch. I've added the following:

      <t>In the case of a KEY record and some other record, obviously the
KEY LEASE applies to
      the key, and the LEASE applies to the other record. If more than one
record that is not
      a KEY record is added by the update, the LEASE (not the KEY LEASE) is
applied to all such
      records. Records that are removed are permanently removed.</t>

>
>
>
The rest of this is nits, mostly stylistic, and it is not expected
> that you have to do anything about them.  I think an implementor could
> work with the existing document well enough, and your disagreement
> with any of my style preferences is, of course, fine.
>
> While I personally like seeing the rationale for design decisions in
> an RFC, and realize there is some disagreement about that, the
> discussion of why you didn't use TTL is a distraction in the
> introduction.  I'd move it toward the end, after the main body and
> before Security Considerations.  I'd also rephrase the final sentence
> to drop "short enough to minimize stale cached data" because it felt
> awkward to me and you had just immediately prior already indicated
> that there were short TTLs to avoid stale data in caches.
>

I agree with you, and I've deleted this paragraph.


> Section 2.1: s/Mechanism/Mechanisms/.  Though I see that RFC 8490 has
> used the same singular, the referenced RFC 6891 and its predecessor
> RFC 2671 have been consistent in that the name via the initialism is
> with the plural.
>

OK


> I'd go one step further and also just use EDNS instead of EDNS(0) as
> the shorthand to clean up the typography, though I see that we've been
> inconsistent about this across documents.  For example, 8490 uses
> EDNS(0) as you have, I used EDNS0 in RFC 7871, and Mark Andrews used
> just EDNS in RFC 7314 -- which is how the DNS Terminology RFCs (7719
> and 8499) label it with a nod to both EDNS0 and EDNS(0).  Call it
> futureproofing; some day after the universal deployment of DNSSEC and
> IPv6 we will have an EDNS(1) which will be backward compatible with
> EDNS(0).  Right?  Right??
>

I'd rather not change this this late in the process, although I agree with
you in principle.


> DNS-SD is only used once, in section 4, so could just have its
> expansion and RFC reference provided there.
>

I think this isn't worth fixing, because it would bloat the place where the
term is used in a confusing way, and I lack enthusiasm for coming up with a
way to address that.


> Section 4: Regarding TSIG, strictly speaking the RR need not appear
> after the OPT RR; the OPT data need only be included it in the digest
> but the message ordering is not defined in RFC 8945.  This is also
> more generally about EDNS and TSIG and not specifically about
> UPDATE-LEASE, and so it shouldn't be necessary to describe that in
> section 4.
>

I've deleted that text. We shouldn't be specifying how to do TSIG here
anyway, as you say.


> 4.2: Because the value type is specified in seconds, I'd make the
> number of seconds the main part of the "no shorter than" sentence and
> move 30 minutes into the parenthetical.  The second mention of 30
> minutes is fine as-is.
>

Done.


> s/are for example 100/are, for example, 100/ per common style guides.
>

Let's leave that to the RFC editor.


> Section 5.1: s/records affected the previous/records affected by the
> previous/
>

Yup.


> Is there a reference (RFC 6763 section?) that could be included to how
> dnssd typically does dynamic update and prerequisite handling?


We reference SRP, which is how DNSSD does DNS updates.


>   I
> realized as I was reading section 5.1 about refresh messages that I
> didn't really see how a refresh message would be different from a
> registration message, particularly as the section says that a refresh
> message can act as a registration message and also says that it's
> formatted the same except maybe with regard to prerequisites.
>
> Naively, I'd just do the equivalent of:
>
>   update delete client.example.com a
>   update add client.example.com a 192.168.1.102
>
> .. but I'm pretty sure that is too simplistic.
>

I think this falls into the "don't specify the other protocol here." We're
just specifying the update-lease option, really. I agree that the text
about refreshes is a bit challenging, but I would have had to completely
rewrite the document to fix that, and I don't think there's much point
since this is better specified in the SRP document anyway. Stuart wanted to
keep this text because you might use update-lease in some other way, but I
think we'd need another document to clarify how that would work anyway, so
it's okay if this isn't perfect--it's probably good enough.


> I was left wondering just how "Refresh Message Format" (5.1) was
> different from "Registration Message Format" (no section heading).
> Maybe it shouldn't have a section heading that seems to call it
> out as its own format rather than maybe having an additional
> constraint on prerequisites. (I'm not even sure this is the right
> interpretation.)
>

This would require a document rewrite. I think it's okay as is. Yes, we
could be more consistent, but it gets the point across.


> Also in that section, "the response from the server can be used to
> determine how to proceed when the Refresh fails" could helpfully
> describe the basic strategy or maybe use a "For example, ..."  Perhaps
> the reference to how dnssd usually does dynamic update already has
> discussion of handling failures?
>

This document isn't a protocol specification for DNS update. We could
remove that suggestion, but again I'd rather not make big changes at this
point.


> I'd like to see a section added with at least one example
> request/response pair, demonstrating the multiple 'add' situation,
> presumably of a client staking a name claim on its A and AAAA
> addresses.
>

Again, that would make sense in a DNS Update protocol spec, but not here.

Thanks for the review! Sorry I'm not being more cooperative. Possibly the
ADs will school me... :)