Fix domain name encoding to Latin1 and canonical case folding to ASCII-only - #63
Open
Hemsby wants to merge 1 commit into
Open
Conversation
…I-only SerializeDomainName/DeserializeDomainName used Encoding.ASCII, which silently corrupts any label byte >= 0x80 to '?' on write and throws on read. DNS wire-format labels are octet strings (RFC 1035), not restricted to ASCII, so a well-formed name containing such a byte was previously mishandled. Encoding.Latin1 maps codepoints/bytes 0-255 one to one, a strict superset of ASCII, so this is a no-op for every name that worked before and only changes behavior for the previously-broken case. Also fixes a related, currently-latent correctness bug this would have newly exposed: RFC 4034 section 6.2 canonical form requires lowercasing ASCII A-Z only, but CanonicallySerializedResourceRecord.Create and DnsNSECRecordData.CanonicalComparison both used string.ToLowerInvariant(), which performs full Unicode case folding and would change the byte value of Latin-1 supplement codepoints (e.g. 0xC0 to 0xE0), corrupting the bytes that feed into RRSIG generation and NSEC canonical ordering for such names. Added DnsDatagram.ToLowerInvariantAscii, which folds only A-Z and leaves every other byte untouched, and used it at both call sites.
Member
|
Thanks for the PR. This fix looks good at a glance. Will study it soon and get it merged. |
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
SerializeDomainName/DeserializeDomainNameuseEncoding.ASCII, whichsilently corrupts any label byte >= 0x80 to
?on write and throws onread. DNS wire-format labels are octet strings (RFC 1035), not restricted
to ASCII, so a well-formed name containing such a byte is currently
mishandled rather than round-tripped faithfully.
Fix
Encoding.Latin1maps codepoints/bytes 0-255 one to one, a strictsuperset of ASCII, so this is a no-op for every name that worked before
and only changes behavior for the previously-broken case.
A related, currently-latent bug this would otherwise expose
RFC 4034 section 6.2 canonical form requires lowercasing ASCII A-Z only,
but
CanonicallySerializedResourceRecord.CreateandDnsNSECRecordData.CanonicalComparisonboth usedstring.ToLowerInvariant(), which performs full Unicode case folding andwould change the byte value of Latin-1 supplement codepoints (e.g. 0xC0 to
0xE0). That corrupts the bytes that feed into RRSIG generation and NSEC
canonical ordering for such names. This has been dormant because those
bytes could never survive the ASCII round-trip to reach this code - fixing
the encoding without also fixing this would have introduced a live
signature-correctness bug for real inbound names.
Added
DnsDatagram.ToLowerInvariantAscii, which folds only A-Z and leavesevery other byte untouched, and used it at both call sites.
Testing
Verified against RFC 4470's own worked example and several edge cases via
direct testing of the serialize/deserialize round trip and canonical
comparison for names containing bytes outside the ASCII range.