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

Jump to bottom

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

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

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

pmarks-net opened this issue Aug 31, 2015 · 13 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.8

Comments

@pmarks-net
Copy link
Contributor

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
Copy link

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

@rsc rsc closed this as completed in bb8c2e1 Jan 8, 2016
@mikioh
Copy link
Contributor

mikioh commented Jan 8, 2016

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

@rsc rsc reopened this Jan 8, 2016
@rsc
Copy link
Contributor

rsc commented Jan 8, 2016

Reopened because of #13878.

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

rsc commented Jan 8, 2016

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

@bradfitz bradfitz modified the milestones: Go1.7Maybe, Go1.7Early May 6, 2016
@mdempsky
Copy link
Member

mdempsky 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 Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 26, 2016
@quentinmit
Copy link
Contributor

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
Copy link
Member

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

@rsc
Copy link
Contributor

rsc 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 The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 17, 2016
@mdempsky
Copy link
Member

@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
Copy link
Contributor

rsc commented Oct 18, 2016

OK, well sending the DNS request is OK too.

@mikioh
Copy link
Contributor

mikioh commented Oct 19, 2016 •
edited

@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
Copy link

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

@rsc rsc modified the milestones: Go1.8, Go1.8Early Oct 20, 2016
@gibson042 gibson042 mentioned this issue Oct 22, 2016
Closed
@gibson042
Copy link
Contributor

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).

@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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
9 participants
@bradfitz @mdempsky @mikioh @rsc @quentinmit @gibson042 @ianlancetaylor @pmarks-net @gopherbot