At Mon, 12 Feb 2018 15:28:50 -0500, Warren Kumari <war...@kumari.net> wrote:
> Anyway, we've finally posted an updated version - > https://datatracker.ietf.org/doc/draft-ietf-dnsop-kskroll-sentinel/ I've read draft-ietf-dnsop-kskroll-sentinel-01 (this is my first careful read of this draft) and found it generally well-written. I have some comments on 01. These are basically editorial. - General I think it's worth pointing out that SERVFAIL can be returned for various other reasons and the detection mechanism relying on it isn't reliable. This is probably okay given the purpose of this detection, but I think it's better to note it explicitly. - Section 1.1 Address Record: Throughout this document we use the term Address Record (AR) to mean an A or AAAA record. [...] I actually didn't find this term other than here and Section 9 (change log). - Section 1.1 and throughout the draft AAAA records and the IPv6 documentation prefix (2001:DB8::/32) as I'd suggest using lower-case letters to show IPv6 addresses and prefixes, following the recommendation of RFC5952. It's not a big deal but it would be better if we can be more consistent on the choice of letter case in RFCs. - Section 2 Charlie's resolvers are validating, but they have not been upgraded [...] resolve it). He is able to fetch both of the other resources - from this he knows (see the logic below) that he is using legacy, non- validating resolvers. [...] Do you mean "he is using legacy validating resolvers"? If it's not a typo, the behind logic isn't obvious to me and it would help if you explain it in more detail (instead of just referring to 'below'). - Section 3 [...] Note that the test is "Is there a key with this KeyID in the resolver's current trust store for the DNS root", not "Is there any key with this KeyID in the trust store", nor "Was a key with this KeyID used to validate this query?". Unless I miss something, the draft doesn't clarify its use is limited to root KSK until the next paragraph of this, so I suspect this statement will confuse a fresh reader who reads this doc from top to bottom without a particular knowledge of background discussion. I'd suggest noting it somewhere before this part, perhaps in the introduction (and maybe also in abstract). - Section 3 [...] Note that the <tag-index> is specified in the DNS label using hexadecimal notation. I suggest clarifying whether '<tag-index>' contains leading 0s somewhere in the document (this may not be the best place to do so, but this is the first reference to 'tag-index' that made the question occur to me). - Section 3 If the resolver has not placed a Root Zone Key Signing Key with tag index value matching the value specified in the query into the local resolver's store of trusted keys, then the resolver should return a response indicating that the response contains authenticated data according to section 5.8 of [RFC6840]. Should we perhaps define "store of trusted keys" more precisely? For example, if a key is in the "AddPend" status (per RFC5011) for the resolver, should it be considered in the "store of trusted keys"? - Section 3 This mechanism is to be applied only by resolvers that are performing DNSSEC validation, and applies only to RRset responses to an A or AAAA query (Query Type value 1 or 28) where the resolver has authenticated the response RRset according to the DNSSEC validation process and where the query name contains either of the labels described in this section as its left-most label. Is the RRset in 'response RRset' intentionally added (or in other words can't it just be 'response')? Perhaps is the intent to exclude negative responses? In any case I think the intent should be clearer here. And if a mere 'response' suffices it's probably less confusing to just say so. BTW, you might want to say 'a query for an Address Record' or 'an Address Record query' instead of 'an A or AAAA query' (I guess that's the intent of the introduction of this term. See also my first comment on Section 1.1 above). - Section 4 o Vleg: A DNSSEC-Validating resolver that does not implement this mechanism will respond with an A or AAAA RRSET response for "kskroll-sentinel-is-ta", an A record response for "kskroll- sentinel-not-ta" and SERVFAIL for the invalid name. + s/RRSET/RRset/? + 'an A record response' should be 'an A or AAAA record response' or it's revised using the "Address Record" term. Same comment applies to other part of this section including the table. -- JINMEI, Tatuya _______________________________________________ DNSOP mailing list DNSOP@ietf.org https://www.ietf.org/mailman/listinfo/dnsop
