NLnetLabs / ldns

LDNS is a DNS library that facilitates DNS tool programming
https://nlnetlabs.nl/ldns
BSD 3-Clause "New" or "Revised" License
295 stars 99 forks source link

Call ldns_dname2canonical for RDATA of NSEC and NSEC3 records #132

Closed adam-stoler closed 3 years ago

adam-stoler commented 3 years ago

Interoperability testing of zone digests revealed a corner case where ldns was not properly canonicalizing NSEC records. According to Section 6.2 of RFC 4034, NSEC records are included in the list of those that should have uppercase US-ASCII letters replaced with the corresponding lowercase ones for DNS names in their RDATA. Not doing so in ldns will cause it to generate an incorrect digest or fail to validate a ZONEMD for a zone that uses NSEC records and has names with uppercase characters.

NSEC3 records potentially have the same issue. These are machine generated and are unlikely to contain uppercase characters, but I believe it is technically legal to do so. Just in case, I think it makes sense to also canonicalize these.

wessels commented 3 years ago

I agree with @adam-stoler that LDNS_RR_TYPE_NSEC should be added to the set of types whose RDATA names need canonicalization for proper ZONEMD digest calculation. I suspect LDNS_RR_TYPE_NSEC3 isn't needed because the hashed next owner name is stored as a binary hash, rather than a domain name.

Test cases have been added to https://github.com/verisign/zonemd-test-cases

wessels commented 3 years ago

After having a discussion with Mark Andrews, I'm not sure this is so simple any more. RFC 6840 section 5.1 updates 4034 to clarify that NSEC is not in the canonical type list. From what I understand it means 'NSEC AAA.example.com ...' and 'NSEC aaa.example.com ...' produce different RRSIGs, and perhaps should also produce different digest values.

adam-stoler commented 3 years ago

That section in RFC 6840 does seem to clear up any ambiguity with this issue. So I now believe that the current ldns behavior is correct, and that it should not canonicalize the RDATA names of NSEC records when generating RRSIGs and calculating ZONEMDs. I will close this Pull Request. Sorry for the trouble.

wtoorop commented 3 years ago

No problem @adam-stoler I'll close it for you :)