insomniacslk / dhcp

DHCPv6 and DHCPv4 packet library, client and server written in Go
BSD 3-Clause "New" or "Revised" License
710 stars 169 forks source link

RFC 1035 Labels: Support Partial Domain Names as per RFC 4704 Section 4.2 #466

Closed rtpt-erikgeiser closed 2 years ago

rtpt-erikgeiser commented 2 years ago

I found that my VM running Windows 10 (Version 1909) in default configuration sends DHCPv6 solicit messages with an FQDN option where the label only contains a length byte and the name without a trailing zero-length byte (note that the zero-byte after win10vm belongs to the vendor class option):

image

When this label is parsed in labelsFromBytes(), the bottom else branch is executed first such that label holds win10vm. Then, however, the whole data is already consumed, pos >= len(buf) is false and the function returns without appending label to labels.

https://github.com/insomniacslk/dhcp/blob/1ca156eafb9f20f7884eddc2cf610bade5dfb560/rfc1035label/label.go#L93-L140

As far as I understand RFC 1035, this library implements label parsing correctly. In this section of RFC 1035 it is explicitly stated that "Since every domain name ends with the null label of the root, a domain name is terminated by a length byte of zero". While Wireshark does parse the supposedly malformed label, it calls it "Partial domain name". I don't know if this is normal or if this is due to the missing zero-length byte.

It would be great if this case would also be supported, as this seems to be the default behavior of Windows 10. I've added a PR (#467) with a failing test as a starting point in but I don't really know how the fix should be implemented correctly.

For anyone running into the same issue, I have implemented the following workaround:

if len(fqdn.DomainName.Labels) == 0 {
    rawLabel := fqdn.DomainName.ToBytes()
    if len(rawLabel) > 2 && int(rawLabel[0]) == (len(rawLabel)-1) {
        fqdn.DomainName.Labels = append(fqdn.DomainName.Labels, string(rawLabel[1:]))
    }
}
rtpt-erikgeiser commented 2 years ago

I did some further research and found that this is indeed a valid label. As per RFC 4704 Section 4.2 labels without trailing zero-length field are indeed partial names:

To send a fully qualified domain name, the Domain Name field is set
to the DNS-encoded domain name including the terminating zero-length
label.  To send a partial name, the Domain Name field is set to 
DNS-encoded domain name without the terminating zero-length label.