Skip to content

Don't compress or downcase the FQDN in LP rdata - #1282

Closed
chuenchen309 wants to merge 1 commit into
rthalley:mainfrom
chuenchen309:fix/lp-rdata-no-compress-no-downcase
Closed

Don't compress or downcase the FQDN in LP rdata#1282
chuenchen309 wants to merge 1 commit into
rthalley:mainfrom
chuenchen309:fix/lp-rdata-no-compress-no-downcase

Conversation

@chuenchen309

Copy link
Copy Markdown

LP._to_wire (dns/rdtypes/ANY/LP.py) forwards the message compression table and the canonicalize flag to its embedded FQDN:

self.fqdn.to_wire(file, compress, origin, canonicalize)

So an LP record's name gets name-compressed on the wire and downcased in the DNSSEC canonical form. Both are wrong for LP (RFC 6742), which is not an RFC 1035 type:

  • RFC 3597 §4: names embedded in the RDATA of non-RFC-1035 types MUST NOT be compressed; new RR types MUST NOT allow name compression.
  • RFC 4034 §6.2, as amended by RFC 6840 §5.1 (the downcasing list only ever shrinks): LP is not on the list, so its name is not downcased in the canonical form.

Every other modern name-bearing type in dnspython (DSYNC, SVCB/HTTPS, HIP, IPSECKEY, NSEC) already uses to_wire(file, None, origin, False). LP is the lone outlier — it was written from the MX template, where compression is grandfathered.

# owner "elsewhere." + LP "10 elseWhere." (case-insensitive compression):
before: 000a c000              # preference + a compression pointer
after:  000a 09elseWhere00     # full, uncompressed, case preserved

# to_digestable() of "LP 10 L64.EXAMPLE.":
before: ...l64...   (downcased)
after:  ...L64...   (preserved, like the sibling DSYNC)

Fix: use the same idiom as the sibling types — to_wire(file, None, origin, False).

Verification (re-runnable from the diff): added test_LP_name_not_compressed in tests/test_rdata.py, modeled on the existing test_svcb.py::test_alias_not_compressed; it fails on the current tree and passes with the fix. tests/test_rdata.py + tests/test_wire.py → 108 passed; the wider suite is green apart from an unrelated offline live-network test.

(Left out of this PR: DNAME/UncompressedNS does not downcase even though DNAME is on the RFC 4034 list — but that's arguably intentional (RFC 6672) and debated, so I scoped this to the clear-cut LP case.)


Disclosure: This PR was authored by an AI coding agent (Claude Code) running on this account: the AI found the bug, ran the repro, wrote the test, and wrote this description. The human account holder reviews every change and is accountable for it. The verification above is real and re-runnable from the diff. If this isn't the kind of contribution you want, say so and I'll close it.

`LP._to_wire` forwarded the message compression table and the canonicalize flag
to its embedded FQDN, so the name got name-compressed and downcased in the
DNSSEC canonical form. LP (RFC 6742) is not an RFC 1035 type: per RFC 3597
section 4 its embedded name must not be compressed, and per RFC 4034
section 6.2 (as amended by RFC 6840 section 5.1) it is not on the downcasing
list. Every other modern name-bearing type (DSYNC, SVCB/HTTPS, HIP, IPSECKEY,
NSEC) already uses `to_wire(file, None, origin, False)`; LP was the outlier,
copied from the MX template where compression is grandfathered.
@pspacek

pspacek commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

I agree it is a bug. Perhaps the code could get rid of the unnecessary comments and the test could be generalized to all types with uncompressable names?

@rthalley

Copy link
Copy Markdown
Owner

I agree with Petr.

@rthalley

Copy link
Copy Markdown
Owner

I found a few other similar issues implementing the suggested testing, so I'll just fix everything for downcasing and canonicalization, include updated tests, and be sure to add a Co-Authored-By for you to the commit.

@rthalley

Copy link
Copy Markdown
Owner

Closing in favor of [#1284]. You are a co-author.

@rthalley rthalley closed this Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants