I was assigned as the dnsdir reviewer for draft-ietf-dnsop-rfc7958bis.

For more information about the DNS Directorate, please see
https://wiki.ietf.org/en/group/dnsdir

Summary: On the Right Track

I've read the document with fresh eyes and implementation experience based on the previous version of the format:
- Removed parts are okay from my perspective.
- The major problem I can see is under-specification related to the XML format.

I remember when I was implementing the previous version I just crossed my fingers and hoped for the best, which worked because it the code was only ever run on root zone XML. But if we are redoing the document I think we should fix it OR declare the format to be root-only. The new PublicKey element also brings new nasty corner cases which need clarification, see below.

Details:

>  2.2. XML Semantics
> The Zone element in the TrustAnchor element states to which DNS zone this container applies. The root zone is indicated by a single period (.) character without any quotation marks.

I understand it does not matter for the root zone, but if we want to make this format generic I think it needs clarification for non-root Zone names. Essentially it needs text which allows implementer to just take the string produced by XML parser and pass it on into a normal DNS parser with confidence. I suggest addition like:
- Zone element contains DNS name in presentation format [RFC reference]
- Escaping applies the same way as in zone file format [RFC reference]
- Names MUST be absolute (?)

An obvious alternative is to remove the element completely and declare the format to be root-only.

Or if backwards compatibility is required then we can declare that the value MUST be equal to . 

BUT If backwards compatibility is a requirement it should be mentioned because I can't see it mentioned anywhere in the current text.


> Note that the validUntil attribute of the KeyDigest element is optional. If the relying party is using a trust anchor that has a KeyDigest element that does not have a validUntil attribute, it can change to a trust anchor with a KeyDigest element that does have a validUntil attribute, as long as that trust anchor's validUntil attribute is in the future and the DNSKEY elements of the KeyDigest are the same as the previous trust anchor.

What are "DNSKEY elements" this paragraph refers to? I think I can guess, but given optional presence of PublicKey field I think we should rephrase to make it absolutely clear what fields MUST match. E.g. something like: "MUST match values for KeyTag, Algorithm, DigestType, Digest" (ignoring id, both timestamps, and PublicKey which is optional and there are no rules for its presence).


> The KeyTag element in the KeyDigest element contains the key tag for the DNSKEY record represented in this KeyDigest.

As courtesy for implementers I recommend a warning about possibility of duplicate KeyTags. Something like "MUST NOT rely on unique KeyTag values". (Again, this might not be really relevant for the Root zone but ... are we still pretending the format is generic?)


>  2.4. Converting from XML to DNSKEY Records 
> The published trust anchor does not provide values for DNSKEY protocol or flags. For the purpose of this mechanism, clients can assume valid trust anchors will be have the ZONE and SEP flag bits set to 1.

I think it is extremely bad idea to ignore fields, especially Flags. There are various proposals for new DNSKEY flag usage in the DD WG.

Even if we say that DD WG will go down in flames before it produces anything I think it's _extremely_ bad idea to omit pieces of information and assume that client can reliably fill in missing pieces with constants. I would say that the missing DNSKEY fields really really must be represented explicitly. (E.g. I don't want to go down the rabbit hole of all REVOKE flag corner cases.)

On high level I also find confusing that the new element is optional - that makes it unreliable for consumers because there are no rules for when it might or might not be present.

Also there are no rules for what to do when reconstructed DS and DNSKEY don't match - which can totally happen given the half-representation we have in the current version.

Is there implementation experience with the new format? What was the implementer feedback?

TL;DR there are issues which can be addressed with smallish amendments.

Petr Špaček
Internet Systems Consortium