miekg / dns

DNS library in Go
https://miek.nl/2014/august/16/go-dns-package
BSD 3-Clause "New" or "Revised" License
7.86k stars 1.12k forks source link

IsDomainName gives an ok for domains longer than the RFC maximum length #1552

Closed jeffswartzman closed 2 months ago

jeffswartzman commented 3 months ago

This issue is a restatement of issue #1544, expanding greatly on the details.

While purporting to check the length of a domain name, the IsDomainName function gives an "ok" to domain names two octets larger than the largest allowable domain name.

It checks if each label fits in 63 characters and that the entire name will fit into the 255 octet wire format limit.

RFC1035 Section 3.1 states:

Domain names in messages are expressed in terms of a sequence of labels. Each label is represented as a one octet length field followed by that number of octets. Since every domain name ends with the null label of the root, a domain name is terminated by a length byte of zero. The high order two bits of every length octet must be zero, and the remaining six bits of the length field limit the label to 63 octets or less.

To simplify implementations, the total length of a domain name (i.e., label octets and label length octets) is restricted to 255 octets or less.

As all labels require an octet dedicated to their length, in a domain name's fully-qualified form, we can use the label separating "." character as a proxy for those length octets in wire format. However, this leaves us requiring the final termination of "a length byte of zero". Since we start with a maximum of 255 octets to work with, including "label octets and label length octets," we have a total budget of 254 usable octets, in fully-qualified form, once the zero-label is accounted for.

Here's an example of miekg/dns lib performing this length check incorrectly.

Here's an example from dig, with a 255 octet domain name, performing its length check flawlessly.

% dig bcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-0123456789.abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-0123456789.abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-0123456789.abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-0123456789.
dig: 'bcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-0123456789.abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-0123456789.abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-0123456789.abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-0123456789.' is not a legal name (ran out of space)
miekg commented 2 months ago

thanks

Yes 1035 has been not good at exactly specifying the boundaries. It is understood that a lenght that fits an octet (256) is OK

zeroizerz commented 3 weeks ago

@miekg

thanks

Yes 1035 has been not good at exactly specifying the boundaries. It is understood that a lenght that fits an octet (256) is OK

Supposedly the limit was set to 255 (rather than 256) to enable efficient storing of the names in a varchar(255) field in a database. The missing byte is used to store the length of the string, since one byte can represent values from 0 to 255. But if the limit were 256, then it would've required two bytes to store the length instead of just one.

RFC 2181 also states the limit is 255. And isDomainName() from the std lib (https://go.dev/src/net/dnsclient.go) also follows this limit of 255. Also the comment of dns.IsDomainName() itself still says the limit is 255.

Can you reconsider changing the limit to 255? This would be consistent with the specifications, std lib, and user expectations.