Repository navigation
Fix DNSSEC, wire and zone-file conformance bugs - #151
Merged
Merged
Conversation
RFC 4034 §3.2 lets a signature's expiration and inception be written either as seconds since the epoch or as a UTC date YYYYMMDDHHmmSS, and signed zones are usually written with dates. The zone parser kept the date as the 14-digit number, 20210217232440 for example, and the encoder wrote its low 32 bits, so every RRSIG read from such a file went out with signature times that had nothing to do with the dates. A value of 14 digits is now read as the date, which is how §3.2 tells the forms apart, since a 32-bit number never has more than 10. Dates past 2106 wrap modulo 2^32 (§3.1.5), as BIND's dns_time32_fromtext does, and a leap second is read as the first second of the next minute, as BIND reads it. An invalid date, one before 1970, or a number too wide for 32 bits and too short for a date is rejected. Found while making the encoder refuse values that do not fit their field: root.zone and both ZONEMD fixtures stopped parsing. Without the change the new date and invalid-time tests fail. fmt, lint, xref, dialyzer and ct (722) pass.
dns_domain:split/1 measured a label when it reached the dot after it,
so the last label of a name was never measured, and to_wire/1, which
splits the name, wrote a last label of 64 bytes or more with that
length in its length byte. RFC 1035 §2.3.4 limits every label to 63
octets, and on the wire a larger length byte is not a length but
another label type (RFC 1035 §4.1.4, RFC 6891 §5), so the receiver
misreads the name and everything after it.
The encoder writes the names in RDATA it does not compress with
to_wire/1, so an SRV target, an RRSIG signer, a DNAME target and the
like went out malformed. Compressed names, owners among them, already
refused such a label, as to_wire/3 measures the last one itself.
split/1 now raises {label_too_long, Label} for the last label too, and
to_wire/1 with it. Without the change the new split and to_wire tests
fail. fmt, lint, xref, dialyzer and ct (722) pass.
The encoder trusts what it is given and writes each field with bit
syntax, which keeps only the low bits of an integer too wide for its
segment and writes a binary of the wrong size as it is. So whatever
JSON or a zone file let through went out silently wrong: MX preference
70000 as 4464 and -1 as 65535, an A address {300, 1, 2, 3} as
44.1.2.3, a 256-byte CAA tag or NSEC3 salt announced as 0 bytes, a
message ID of 70000 as 4464, a TTL of 2^31 as one resolvers read as
zero (RFC 2181 §8). An SVCB hint that was not an address tuple was
skipped. Other records, such as an EUI64 address of two bytes or an A
record in class CH, loaded fine and then made every response carrying
them fail to encode.
The checks live in a new dns_check module and run at load time only,
keeping the encode path as it is: dns_json:from_map/1 checks every
record it builds and raises {invalid_record, Record}, and the zone
file parser checks each record and returns a semantic error. They
cover the RR header, every fixed-width RDATA field, addresses, LOC
coordinates, 8-bit lengths, HINFO strings, NSEC type numbers, SVCB
params and RDATA over 65535 bytes, and in JSON also message headers,
queries, the OPT pseudo-RR and its options. The RDATA is also encoded
once, in the record's own class, to catch what the encoder refuses
outright: a name the wire cannot hold, which it checks as it writes
each one, or a type it does not write in that class, such as A in CH.
fits/1 has no catch-all for records, so a new type without a clause is
refused rather than left unchecked, and a property checks that every
record the wire properties generate is accepted.
Three JSON tests used values the wire cannot carry: RSA DNSKEYs with
three and one key components, which the encoder could never write, and
type 99999, which went out as 34463. They now use an exponent and
modulus, and type 65280 from the private use range. The zone tests of
class parsing used A records in CH, HS, CS and CLASS255, and the zone
properties generated them too; they now use TXT, and the properties
keep the types the encoder writes only in IN there.
Without the change the new JSON and zone file tests fail. fmt, lint,
xref, dialyzer and ct (738) pass, and cover stays at 92%.
DNSSEC canonical form lowercased the names in the RDATA of NSEC, IPSECKEY, DSYNC, SVCB and HTTPS records. RFC 4034 §6.2 lists the types whose RDATA names are lowercased, and RFC 3597 §7 closes that list to the types published before RFC 3597, so IPSECKEY, DSYNC, SVCB and HTTPS keep the case of their names. RFC 6840 §5.1 then takes NSEC off the list. BIND digests all five as they are. With an uppercase letter in such a name, as in an HTTPS record pointing at Svc.Example.Test., the signature input differed from everyone else's: a signature made here did not verify in BIND, and one made by BIND did not verify here. Checked both ways against BIND 9.20: its dnssec-verify refused the RRSIGs signed here with "No correct ED25519 signature", and verify_rrsig/4 returned false for BIND's. erldns signs zones with this library, and when it loads a zone it lowercases owner names but not RDATA names, so such a record in a signed zone would be served with a signature validating resolvers reject. The NSEC records erldns builds are not affected, as it lowercases the query name they come from. The test for canonical form now asserts these types are left as they are, and a new test holds BIND's signatures over mixed-case records of the five types, made with an Ed25519 key: each must verify, and signing the record again must give the same signature, Ed25519 being deterministic. Without the change both tests fail. fmt, lint, xref, dialyzer and ct (739) pass, and cover stays at 92%.
The zone parser lowercased every name it read, the origin and the names in RDATA as well as the owners. RFC 1035 §2.3.3 has names keep the case they are written in, and DNSSEC relies on it: the names in the RDATA of HTTPS, SVCB, IPSECKEY, DSYNC and NSEC are signed with their case (RFC 3597 §7, RFC 6840 §5.1), as are those of any type defined since. So a signature BIND made over an HTTPS record pointing at Svc.Example.Test. did not verify against the record read from the same zone file, and one made here over the record read did not verify in BIND, the very mismatch the canonical form now avoids. The names in RDATA and the origin they are resolved against now keep their case. Owner names are still lowercased, as before, which DNSSEC does to them anyway. The types whose RDATA names DNSSEC lowercases sign the same either way. The new test reads a zone holding a DNSKEY, an HTTPS record pointing at Svc.Example.Test. and BIND's RRSIG over it, finds the target as written, and verifies the signature. The dyn.zone test now expects the MX exchanges spelled as the file spells them. Without the change both fail. fmt, lint, xref, dialyzer and ct (740) pass, and cover stays at 92%.
RFC 4034 §3.1.8.1 builds the signed data from the RRSIG RDATA "with the Signer's Name field in canonical form", and RFC 6840 §5.1 confirms that the signer's name is lowercased there, while the names of the other types it takes off the RFC 4034 §6.2 list are not. The RRSIG RDATA was encoded with the signer's name as given, so its case reached the signature. Signing with a mixed-case zone name, Example.Test, gave a signature that differed from the one for example.test and that other validators rejected, and an RRSIG received with a mixed-case signer, which the wire allows, did not verify here although the signature was valid. The signer's name is now lowercased where the RRSIG RDATA is built, which serves signing and verifying both, and a signed RRSIG carries it lowercased, as BIND writes it. The new test signs an HTTPS record with Example.TEST as the signer and gets BIND's signature for example.test, then verifies that signature with Example.TEST as the signer. The key BIND signed with is now shared between the two tests that use it. Without the change the new test fails. fmt, lint, xref, dialyzer and ct (741) pass, and cover stays at 92%.
The encoder compressed the names in the RDATA of DNAME, KX, NXT and RT records. RFC 3597 §4 says servers "MUST NOT compress domain names embedded in the RDATA of types that are class-specific or not well-known", the well-known types being those of RFC 1035, and none of these four is. RFC 6672 §2.5 says the same of the DNAME target. BIND writes all four uncompressed. A compression pointer only means something inside the message it is in, so a receiver that does not know the type cannot follow it, and copies the pointer as it is when it passes the record on, corrupting the name. RFC 3597 does not even ask receivers to try for KX. The decoder still follows pointers in all four, as RFC 3597 asks of receivers for RT and NXT. The names RFC 1035's types carry are still compressed, and so are the other names in the message. The new test encodes each of the four with a name sharing a suffix with the owner, which compression would have pointed at, and finds the whole name in the message; an MX record in the same position still gets a pointer. Without the change the test fails. fmt, lint, xref, dialyzer and ct (742) pass, and cover stays at 92%.
NelsonVides
force-pushed
the
fix/rfc-conformance
branch
from
October 6, 2026 10:50
d8875f1 to
e2da626
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug fixes only, so this can ship as a v5.0.x patch release:
YYYYMMDDHHmmSSin zone files went out wrong on the wire.🔍 QA
Scenario: Test suite
make test: fmt, lint, xref, dialyzer and all 738 CT tests pass, with coverage at 92%.📋 Deployment Pre/Post tasks
N/A
N/A