Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: go/cgo resolvers report malformed domains differently #12421

Closed
pmarks-net opened this issue Aug 31, 2015 · 13 comments

Comments

Projects
None yet
9 participants
@pmarks-net
Copy link
Contributor

commented Aug 31, 2015

Given the following program:

package main

import (
        "net"
        "fmt"
)

func main() {
        ips, err := net.LookupIP("!!!.local")
        fmt.Printf("%v %v\n", ips, err)
}

A different error message is produced, depending on whether the go or cgo resolver is used:

~/src/go/src$ GODEBUG=netdns=go ../bin/go run ~/test/lookup.go
[] lookup !!!.local: invalid domain name
~/src/go/src$ GODEBUG=netdns=cgo ../bin/go run ~/test/lookup.go
[] lookup !!!.local: no such host

This discrepancy exists because src/net/dnsclient.go uses isDomainName as a filter before starting the lookup, while getaddrinfo has no such filter, and returns the same error for malformed and nonexistent (NXDOMAIN) names.

I think it's debatable whether this qualifies as a bug, but in theory it could be fixed by having the cgo implementation check isDomainName.

@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Aug 31, 2015

@gopherbot

This comment has been minimized.

Copy link

commented Jan 7, 2016

CL https://golang.org/cl/18383 mentions this issue.

@rsc rsc closed this in bb8c2e1 Jan 8, 2016

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2016

Note that "!!!.local" is valid for multicast DNS.

@rsc rsc reopened this Jan 8, 2016

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2016

Reopened because of #13878.

@rsc rsc modified the milestones: Go1.7, Go1.6, Go1.7Early Jan 8, 2016

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2016

Reverted the earlier fix. These will keep being inconsistent in Go 1.6.

@mdempsky

This comment has been minimized.

Copy link
Member

commented May 6, 2016

Fixing this seems easy, but we need a Decider to pick the right fix.

The only restrictions on domain names imposed by DNS are: labels must be 1 to 63 bytes, and total name length (including label length bytes and terminating zero byte) must be <= 255 bytes (or 256 for mDNS). When comparing names for equality, labels are compared ASCII case-insensitively, but otherwise they can contain arbitrary byte values. For example, running host '!!!.google.com' and dig '!!!.google.com' both end up dispatching DNS queries for "!!!.google.com" over the network.

Beyond that, it's not uncommon for DNS utilities to support escape sequences: host '\033\033\033.google.com' and dig '\033\033\033.google.com' both work the same as using '!!!.google.com'. (Most programs use 3-byte decimal escape sequences like Go; djbdns uses variable-length octal escape sequences like C.)

On the other hand, RFC 952 restricts domain names to match the grammar:

  <hname> ::= <name>*["."<name>]
  <name>  ::= <let>[*[<let-or-digit-or-hyphen>]<let-or-digit>]

RFC 1123 relaxes the initial <let> to <let-or-digit>. This relaxed form is also what's specified for use in URIs by RFC 3986. RFC 5321 imposes the same restriction on email address domains. (Off hand, I'm not sure how strictly these rules are enforced in practice.)

mDNS relaxes the rules for .local names to allow UTF-8 in RFC 6762 section 16.

I'd rather have more time to think about the best solution for Go. Punting to 1.8.

@mdempsky mdempsky modified the milestones: Go1.8Early, Go1.7Maybe May 6, 2016

@rsc rsc added the NeedsDecision label Sep 26, 2016

@quentinmit

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2016

My opinion is that we should allow DNS queries for any string, and thus the bug here is that the Go resolver isn't dispatching the query for !!!.local.

@mdempsky

This comment has been minimized.

Copy link
Member

commented Sep 26, 2016

What about backslashes? Do we handle them as escape sequences like libresolv does?

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2016

There are two questions here:

  1. Should the Go DNS implementation reject !!!.local or try a lookup? My understanding is that it should reject !!!.local and NOT try the lookup.

  2. Should the rejection say 'invalid domain name' or 'no such host'? It seems fine to change to say 'no such host' to match cgo.

So I would suggest for Go 1.8 we simply change the path that fails on isDomainName to return 'no such host' instead of 'invalid domain name', to match netdns=cgo mode.

@rsc rsc unassigned mdempsky and rsc Oct 17, 2016

@rsc rsc added NeedsFix and removed NeedsDecision labels Oct 17, 2016

@mdempsky

This comment has been minimized.

Copy link
Member

commented Oct 17, 2016

@rsc To be clear, the cgo path does send an actual DNS query for !!!.local. The "no such host" error isn't synthetic: it's because the DNS server sent an NXDOMAIN response.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 18, 2016

OK, well sending the DNS request is OK too.

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Oct 19, 2016

@mdempsky,

I personally don't put my oar in, but a bit.

The only restrictions on domain names imposed by DNS are: [...]

There are use cases of Net-Unicode for unicast DNS in the wild. I noticed that "ah, Active Directory and a few directory service implementations do support Net-Unicode for unicast DNS" when I saw #16739. As far as I know, https://tools.ietf.org/html/draft-ietf-dnssd-mdns-dns-interop is the active informational document that addresses interoperability issues with multiple types of DNS-labels. Of course, the existing built-in DNS stub resolver doesn't support unicast DNS labels encoded in Net-Unicode from its brith. So just FYI.

@gopherbot

This comment has been minimized.

Copy link

commented Oct 19, 2016

CL https://golang.org/cl/31468 mentions this issue.

@gibson042

This comment has been minimized.

Copy link
Contributor

commented Oct 22, 2016

isDomainName also falsely rejects wildcard domains like *.golang.org, see results in #17549. The bottom line, as pointed out six years ago in #1168, is that the function is far too restrictive for DNS lookups (as opposed to hostname connections). It should accept all strings that are valid DNS names per RFC 1035, including those with non-LDH labels, and should probably also accept strings that are valid per RFC 6762 (which allows up to 256 wire-format octets for ".local" domains—you might even opt to allow that many for all domains, even though 255 is the interoperability limit for public DNS).

@gopherbot gopherbot closed this in fa90f9b Oct 24, 2016

@golang golang locked and limited conversation to collaborators Oct 24, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.