fix: dnssec::name_to_wire duplicates write_qname and lacks RFC 1035 §5.1 escape handling #55

New Issue

2026-04-10T04:52:43+08:00

razvandimescu commented

2026-04-10 04:52:43 +08:00

(Migrated from github.com)

Bug

`src/dnssec.rs:723` defines `name_to_wire(name: &str) -> Vec`, a parallel implementation of what `BytePacketBuffer::write_qname` does — it splits on `.`, lowercases each label byte, and emits `len` + bytes + trailing zero. It does not understand RFC 1035 §5.1 text escapes.

This was surfaced by the code review for #54 (the read_qname/write_qname escaping fix). Before that PR, `read_qname` emitted raw label bytes without escaping, so `name_to_wire` was already divergent-but-hidden: a label containing a literal `0x2E` would round-trip to bogus DNSSEC signed data, but nobody had noticed because the ambiguity was silent on both sides.

After #54 lands, `read_qname` emits `\.`, `\\`, and `\DDD` escape sequences. If a name that came out of `read_qname` is then handed back to `name_to_wire` for RRSIG canonical form construction, the escape sequences get treated as literal bytes — `name_to_wire` will split on the literal `.` inside `\.`, producing wrong-length labels and a corrupt signed-data blob.

Affected call sites

`name_to_wire` is used in `build_signed_data` and helpers around it:

src/dnssec.rs:765
src/dnssec.rs:786
src/dnssec.rs:809-810
src/dnssec.rs:814
src/dnssec.rs:850
src/dnssec.rs:1012

Any RRSIG canonicalization path that passes an owner name, signer name, next domain, CNAME/NS/MX target, or similar through `name_to_wire`.

Impact

Practical impact is low because:

Numa is not an authoritative signer, only a validator, and upstream answers for zones with escaped labels are extremely rare.
Real-world domains don't contain dots in labels.

But it's a correctness bug in DNSSEC canonical form construction. Adversarial or crafted zones could cause validation mismatches.

Fix

Delete `name_to_wire` and replace the call sites with one of:

A scratch `BytePacketBuffer` + `write_qname` + `filled()` / `get_range()` to extract the wire bytes. Single source of truth for label-to-wire conversion.
Factor the streaming label writer out of `write_qname` into a shared `pub(crate)` function that writes into either a `BytePacketBuffer` or a `Vec`.

Option 1 is the smaller diff; option 2 avoids the scratch-buffer allocation if callers want to write into their own `Vec`.

Either way: the escape logic should live in exactly one place in `buffer.rs`, and `name_to_wire` should be gone.

Discovered during

Code review for #54 (fix: escape DNS label text per RFC 1035 §5.1). Not expanded in that PR to keep scope disciplined.

## Bug \`src/dnssec.rs:723\` defines \`name_to_wire(name: &str) -> Vec<u8>\`, a parallel implementation of what \`BytePacketBuffer::write_qname\` does — it splits on \`.\`, lowercases each label byte, and emits \`len\` + bytes + trailing zero. It does not understand RFC 1035 §5.1 text escapes. This was surfaced by the code review for #54 (the read_qname/write_qname escaping fix). Before that PR, \`read_qname\` emitted raw label bytes without escaping, so \`name_to_wire\` was already divergent-but-hidden: a label containing a literal \`0x2E\` would round-trip to bogus DNSSEC signed data, but nobody had noticed because the ambiguity was silent on both sides. After #54 lands, \`read_qname\` emits \`\\.\`, \`\\\\\`, and \`\\DDD\` escape sequences. If a name that came out of \`read_qname\` is then handed back to \`name_to_wire\` for RRSIG canonical form construction, the escape sequences get treated as literal bytes — \`name_to_wire\` will split on the literal \`.\` inside \`\\.\`, producing wrong-length labels and a corrupt signed-data blob. ## Affected call sites \`name_to_wire\` is used in \`build_signed_data\` and helpers around it: - src/dnssec.rs:765 - src/dnssec.rs:786 - src/dnssec.rs:809-810 - src/dnssec.rs:814 - src/dnssec.rs:850 - src/dnssec.rs:1012 Any RRSIG canonicalization path that passes an owner name, signer name, next domain, CNAME/NS/MX target, or similar through \`name_to_wire\`. ## Impact Practical impact is low because: - Numa is not an authoritative signer, only a validator, and upstream answers for zones with escaped labels are extremely rare. - Real-world domains don't contain dots in labels. But it's a correctness bug in DNSSEC canonical form construction. Adversarial or crafted zones could cause validation mismatches. ## Fix Delete \`name_to_wire\` and replace the call sites with one of: 1. A scratch \`BytePacketBuffer\` + \`write_qname\` + \`filled()\` / \`get_range()\` to extract the wire bytes. Single source of truth for label-to-wire conversion. 2. Factor the streaming label writer out of \`write_qname\` into a shared \`pub(crate)\` function that writes into either a \`BytePacketBuffer\` or a \`Vec<u8>\`. Option 1 is the smaller diff; option 2 avoids the scratch-buffer allocation if callers want to write into their own \`Vec\`. Either way: the escape logic should live in exactly one place in \`buffer.rs\`, and \`name_to_wire\` should be gone. ## Discovered during Code review for #54 (fix: escape DNS label text per RFC 1035 §5.1). Not expanded in that PR to keep scope disciplined.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dearsky/numa#55