On Mon, Dec 23, 2024 at 1:58 PM Patrick Mevzek via Datatracker <
nore...@ietf.org> wrote:
> Reviewer: Patrick Mevzek
> Review result: Ready with Nits
>
Thanks for the review ...
> However, I do find in §3 this to be a little weak:
> " While it could support NSEC3 too, there is no benefit in introducing the
> additional complexity associated with it." Because Motivation in §1 clearly
> explains that this new scheme allows fewer number of NSEC records... and
> mentions 3 of them are needed in NSEC3 case, so the benefit is (should be)
> even
> better here for NSEC3 than NSEC. So I would suggest either giving more
> details
> here on what would be additional complexity for NSEC3, or just removing the
> whole line and stating unambiguously that the document applies only to
> zones
> using NSEC.
>
Please see the separate thread I started on this point, since there are now
already implementations that use NSEC3 and we may need to make some
updates to the draft to acknowledge that and/or describe the small set of
changes needed to support NSEC3.
> I do have some remarks on §3.4:
>
> It defines a new extended code as
> "Invalid Query Type" but it seems to be defined to be used in case of a
> resolver asking for "NXNAME" resource record type. Wouldn't it better to be
> clearer that it applies to the resource record type, as otherwise I fear
> "query
> type" could be understood at what RFC 1035 calls OpCode aka Query vs
> Inverse
> Query vs Notify vs Update, etc.? Specifically since RFC6895 has: " Data
> TYPEs
> are the means of storing data. QTYPES can only be used in
> queries. Meta-TYPEs designate transient data"
> And NXNAME is clearly defined as being of Meta type, not QType.
>
The intention is that the Invalid Query Type EDE has broader applicability
than just this draft.
"Note that this EDE code is generally applicable to any RR type that should
not appear in DNS queries."
So, we could imagine it being used with other meta types too (and that's
what I hope will happen).
> Also, RFC8914 already defines code 20 as:
> "The requested operation or query is not supported."
> Isn't that good enough, or is really a new code needed (30)?
> For a generic code (since document says "Note that this EDE code is
> generally
> applicable to any RR type that should not appear in DNS queries.") maybe
> existing 20 could be enough, or otherwise 30 should be repurposed
> specifically
> about and only NXNAME queries.
>
The EDE space is large and it is better to have more surgical codes
that precisely specify the issue. The "requested operation or query" is
broader than it should be. Was the problem with the query type or
some other "operation"?
Not sure myself, so only in case you find it useful:
> - about caching, RFC2308 has "Negative responses without SOA records
> SHOULD NOT
> be cached", which makes sense when not having the SOA Negative TTL field to
> use, but here with NSEC(NXNAME), there is a TTL on this record, should it
> be
> used (or not) as caching hint?
We don't call it out specifically, but the NODATA responses always have a
SOA
record in the Authority section too. And we expect the NSEC record to have
same
TTL as the SOA min (per RFC 4034, Section 4).
Maybe add a sentence for that, if relevant. -
because of §4.1., shouldn't also RFC6891 be marked as updated by this
> document,
> specifically since it defines Z (which is mutated here) as "Set to zero by
> senders and ignored by receivers, **unless modified
> in a subsequent specification.**
"
(emphasis mine)
>
I don't think an update to 6891 is needed. The phrase you quoted, "unless
modified
in a subsequent specification" takes care of that.
> Also RFC6891 states the following about the fixed part vs the variable one:
> "The fixed part holds some DNS metadata,
> and also a small collection of basic extension elements that we
> expect to be so popular that it would be a waste of wire space to
> encode them as {attribute, value} pairs."
> So this new CO flag being in the fixed part, does it pass the "extension
> elements we expect to be so popular" criteria? If not, and to resolve the
> above
> (no change in RFC6891), instead of a new CO flag, create a new variable
> part,
> by requesting a new EDNS0 Option Code? (with a length of 0, its mere
> presence
> would mean the same as CO=1)
>
This is 1-bit of information, so a good match for an EDNS header flag,
rather
than wasting more space in an option. Secondarily, from experience we know
that EDNS header flags are more likely to traverse broken middleboxes and
deficient DNS actors than options, so that's another reason we chose that.
I cannot answer the question of whether we expect it to pass a popularity
test.
I suspect many in the DNSOP crowd would have already objected to this by now
if it were a concern.
Related, and even if obvious, maybe specify that the flag should be set
> (value
> = 1) to enable the feature, and how it should be in replies and what the
> receiver should do with it (discard I guess, but better to specify - or
> should
> 0/1 on answer give an hint if server supports the feature, even if not
> relevant
> in that specific answer because it was not an NXDOMAIN/NXNAME case?).
>
It is the latter (I understand or support the rcode substitution feature).
Let me
see if we can make that clearer in the draft.
> Nitpicks:
> - there is both "type bitmap" and "Type Bitmaps" and "type bitmaps", so
> both
> variations in casing and singular/plural form. Even if not really
> confusing,
> possibly better to align to just one name? RFC4034 has "Type Bit Maps".
Good point. Let's match the phrasing in existing specs by saying "Type Bit
Maps"
throughout.
> - §3.1
> says creating a response with no records in Answer section, and then says
> to
> create a NSEC record. While normally obvious, just specify that it goes in
> the
> Authority section? (specifically since Authority section is mentioned in
> §3.3)
> - in §6.2 for the new paragraph to add, there is a dangling open ( with no
> ) to
> close it.
>
Ok, yes, we can make that clearer.
Shumon.
_______________________________________________
DNSOP mailing list -- dnsop@ietf.org
To unsubscribe send an email to dnsop-le...@ietf.org
