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: allow custom Resolver method implementation(s) #12503

Open
sajal opened this issue Sep 4, 2015 · 44 comments
Open

net: allow custom Resolver method implementation(s) #12503

sajal opened this issue Sep 4, 2015 · 44 comments

Comments

@sajal
Copy link

@sajal sajal commented Sep 4, 2015

I mentioned in #12476 that I would like to detect the time it took for DNS resolution phase of the Dial process in Dialer.Dial . The solution posted there was very hacky and adds unnecessarily to the API.

@bradfitz suggested

Perhaps net.Dialer could have an optional Resolver, akin to how http.Client has an optional Transport, or http.Server has an optional ErrorLog, etc.

This seems like an excellent idea. Here is how I propose we go about it by adding minimal complexity and preserving code compatibility.

I propose net package adds a new Resolver interface.

type Resolver interface {
    Resolve(host string, deadline time.Time) (addrs []IPAddr, err error)
}

The signature of Resolver.Resolve is same as lookupIPDeadline which Dial eventually uses. Dialer gets an optional field CustomResolver of type Resolver.

The Resolver object (or nil) gets passed around thru the resolution process.

Dialer.Dial -> resolveAddrList -> internetAddrList .

internetAddrList currently always uses lookupIPDeadline, it would need to be changed such that if the passed custom resolver is not nil then use it, otherwise use lookupIPDeadline.

Other functions calling resolveAddrList or internetAddrList would need to be modified to add an extra nil argument . This does not break code compatibility because they are unexported functions.

Benefits of allowing a custom Resolver

  • Allowing me to collect timing information as mentioned in #12476
  • Allowing users to implement their own DNS logic. Failovers, etc.
  • Mocking for tests.
  • Client side caching, pre-fetching, etc.
  • In time, other packages ( like the superb github.com/miekg/dns ) could provide their own Resolver implementations.
  • Great for people like me who rely on Go to write network debugging tools.
@sajal sajal changed the title proposal: Allow passing of custom Resolver to Dialer.dial in package net proposal: Allow passing of custom Resolver to Dialer.Dial in package net Sep 4, 2015
@sajal sajal changed the title proposal: Allow passing of custom Resolver to Dialer.Dial in package net proposal: Allow passing of custom Resolver to net.Dialer Sep 4, 2015
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Sep 4, 2015
@sajal
Copy link
Author

@sajal sajal commented Sep 9, 2015

Should I implement and submit the change for codereview? Or wait for some comments here?

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Sep 9, 2015

No need to prototype it yet. The code will be relatively easy compared to getting the design right.

I suspect that signature isn't general enough. Maybe it's good enough for a dialer, but perhaps it needs a different name.

I bet we don't want to define an interface in the net package. If anything, it could just be an optional func type on the Dialer, similar to funcs on http://golang.org/pkg/net/http/#Transport

@sajal
Copy link
Author

@sajal sajal commented Sep 9, 2015

Perhaps call it Lookupfunc(or better name) and deadline-ing is handled inside net package. It might mirror signature of net.LookupIP which is used by default if Lookupfunc is nil.

Anything that does a lookup could ask for optional field for Lookupfunc to allow user to provide their own implementation.

@adg adg added Proposal and removed Proposal labels Sep 25, 2015
@rsc rsc modified the milestones: Proposal, Unplanned Oct 24, 2015
@rsc rsc changed the title proposal: Allow passing of custom Resolver to net.Dialer proposal: allow net.Dialer to use custom resolver Oct 24, 2015
@rsc rsc changed the title proposal: allow net.Dialer to use custom resolver proposal: net: allow Dialer to use custom resolver Oct 24, 2015
@benburkert
Copy link
Contributor

@benburkert benburkert commented Nov 5, 2015

I would also like to see a Resolver interface but with multiple methods that match the net.Lookup* funcs.

type Resolver interface {
  LookupAddr(addr string) (names []string, err error)
  LookupCNAME(name string) (cname string, err error)
  LookupHost(host string) (addrs []string, err error)
  LookupIP(host string) (ips []IP, err error)
  LookupMX(name string) (mxs []*MX, err error)
  LookupNS(name string) (nss []*NS, err error)
  LookupPort(network, service string) (port int, err error)
  LookupSRV(service, proto, name string) (cname string, addrs []*SRV, err error)
  LookupTXT(name string) (txts []string, err error)
}

The timeout & deadline functionality could be configured when the resolver is created:

func NewResolver(options ResolverOption...) (Resolver, error)

type ResolverOption func(*resolver) error

func ResolverTimeout(duration time.Duration) ResolverOption
func ResolverDeadline(deadline time.Time) ResolverOption
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Nov 6, 2015

@benburkert, that is not a Go-style (small) interface. Once you have 9 methods, surely somebody would want to add a tenth later, but they can't for compatibility reasons. 9 methods is also hard to implement. We'd probably have to add some sort of EmptyResolver type that people could embed which just returned errors for everything.

I'd start with looking at which interfaces are actually needed by the things this bug is about. Maybe you'd have 9 interfaces instead (maybe starting with 3?) and combine them as needed like io.ReadWriteCloser? I don't know. I haven't given this much thought yet.

@davecheney
Copy link
Contributor

@davecheney davecheney commented Nov 6, 2015

What about Lookup(recordtype, query string) ...

It's similar to our Dial(network, address string) function, and would permit wildcard, ANY, and lookups for types not yet added to the dns spec.

Just spitballing...

@theckman
Copy link
Contributor

@theckman theckman commented Jan 14, 2016

I just ran in to this issue myself, except a little bit abstracted away from net.Dialer. My use-case may be a little weird, but this would come in extremely handy for me if it was also exposed within the net/http package.

I'm writing a utility that's going to talk over TLS to backend systems (HTTP + JSON) and I'm using Consul to discover the individual backend nodes. The biggest issue is that I don't have all of my system's DNS requests being serviced by Consul, so pulling a configuration from /etc/resolv.conf won't really work. I plan on using their port 8600 DNS interface.

So I'll end up needing to first obtain a list of IP addresses from the Consul DNS endpoint and then use that IP address in the URL. Following that, I'll need to set the Host field on the request so that the TLS validation works. The only downside here is that I end up having to do a network operation at the creation time of the http.Request struct instead of when actually invoking the request.

If the http.Transport struct was modified to support a custom DNS resolver code path, it would make the code cleaner and avoid the upfront network call.

@mikioh
Copy link
Contributor

@mikioh mikioh commented Mar 11, 2016

At the moment, Dial runs the following processes serially for simplicity:

  1. multiple host and service discovery racers
  2. making a short list of target addresses
  3. multiple connection setup racers, and picking a single winner

In near future, when we want more performance on some circumstances, we probably run:

  1. multiple host/service discovery+connection setup racers
    • per address family, per service {name,port}, etc
  2. picking a single winner

For both cases, the Resolver interface needs to take information for host and service filters. Moreover, it would probably need certificates for supporting upcoming DNS over TLS and DANE.

Looks like this proposal makes it possible to place complicated DNS-related packages at x/net repository. I'm happy if we have fancy name/service discovery functionality without replumbing of packages in standard library.

@anatol
Copy link

@anatol anatol commented Mar 29, 2016

I am trying to run a network application at Android arm64 system and http.Get fails because of DNS resolution failed. It turned out Android uses custom dns resolver interface. https://android.googlesource.com/platform/bionic/+/master/libc/dns/net/gethnamaddr.c#564 An application opens /dev/socket/dnsproxyd socket and uses it to resolve names. I tried GODEBUG=netdns=cgo and for some reason it does not work on Android.

It would be nice if I can implement a custom dns resolver and tell my application to use it. Here is similar issue from another project [1].

[1] syncthing/syncthing-android#412 (comment)

@sajal
Copy link
Author

@sajal sajal commented Mar 30, 2016

Valid use-case for custom resolver, but have you tried using the Android NDK to build your binary?

@jabley
Copy link

@jabley jabley commented May 11, 2016

I'd similarly be interested in having timing information available, similar to time_* variables in curl. A monitoring tool that can periodically probe networks would be very handy.

Happy to open up a separate proposal if it's felt to be off-topic for this one?

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented May 11, 2016

@jabley, that already happened for Go 1.7. See #12580

@adg
Copy link
Contributor

@adg adg commented Jul 19, 2016

This needs a proper proposal document to move forward.

@adg
Copy link
Contributor

@adg adg commented Oct 3, 2016

An extension to the work done in #16672

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Oct 3, 2016

In particular, this got submitted: https://go-review.googlesource.com/29440

@bradfitz bradfitz removed this from the Go1.9 milestone Jul 22, 2017
@excavador
Copy link

@excavador excavador commented Sep 1, 2017

I definitely net some simple way to hook LookupIP / LookupHost / LookupAddr for unit-testing purposes

@ianlancetaylor ianlancetaylor modified the milestones: Go1.10, Unplanned Dec 8, 2017
@mauricio
Copy link

@mauricio mauricio commented Jan 19, 2018

So is there something being planned for this? With net.Resolver being a struct it's still impossible to collect any data about what is going on internally on the DNS layer.

@benburkert
Copy link
Contributor

@benburkert benburkert commented Jan 19, 2018

It's possible to intercept DNS requests generated by the net package by modifying the Dial func of DefaultResolver. For example, this package automatically adds cacheing of DNS queries: https://godoc.org/github.com/benburkert/dns/init

@polomsky
Copy link

@polomsky polomsky commented Jan 22, 2018

It's possible to intercept DNS requests generated by the net package by modifying the Dial func of DefaultResolver

@benburkert it works only for platforms where build in go resolver is present. E.g. unusable for windows.

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 1, 2018

Change https://golang.org/cl/115855 mentions this issue: net: Implement hooks to override Resolver's lookup methods

@jfesler
Copy link

@jfesler jfesler commented Jun 22, 2018

Override functions as in https://golang.org/cl/115855 would be "ok". Doesn't seem consistent with other things that use interfaces; but it would allow for shenanigans (dns lookup metrics; alternatives to DNS; caching; and so forth). This would also allow people to avoid custom dialers; instead using the battle tested dual stack happy eyeball code.

@mwiora
Copy link

@mwiora mwiora commented Aug 20, 2018

hi all,

this topic seems to be implemented and still open so I'd like to try it ;)
As described on stackoverflow I would like to directly use the new resolver configuration options.

The only option I miss is to avoid the fallback to another DNS - I thought about a switch.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Oct 24, 2018

Copying my comment from https://go-review.googlesource.com/c/go/+/115855#message-d80f076d91f28a2e5aa2f1eb6fdd88a33aec9502 ....

I can't think of a name or pattern I like here. The closest is naming them all LookupFooFunc, but then there's too many of them.

I do want this behavior, but we kinda already have the hook we need for testing purposes: Resolver.Dial.

We just need an easy way to wire up high-level Lookup func literals into fake in-memory DNS-speaking func implementations to assign to the Resolver.Dial field.

Before we add a bunch of stuff, I'd like to see a package (we could even put it in golang.org/x/net or nettest) that does the in-memory fake-DNS stuff and answers the DNS queries using the test/etc-provided LookupFoo funcs. Such a package should be even easier lately given the recent work on golang.org/x/net/dns.

(The user would probably also have to set Resolver.PreferGo to true to force the Dial to be used instead of cgo?)

So let's put this on hold for now until we see what such a package might look & feel like.

/cc @iangudger

@iangudger
Copy link
Contributor

@iangudger iangudger commented Oct 24, 2018

Another way to achieve this with the current interface would be to use the PacketResolver from golang.org/cl/107306, wrap it in a net.Conn implantation, and return it from Resolver.Dial.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Oct 24, 2018

Oh, nice. PacketResolver makes such a glue package even easier.

@benburkert
Copy link
Contributor

@benburkert benburkert commented Oct 24, 2018

Check out https://godoc.org/github.com/benburkert/dns for some prior art of that glue package.

@adamramadhan
Copy link

@adamramadhan adamramadhan commented Jan 19, 2019

how is this going? is there a current working example to force using 8.8.8.8 resolver and somesort of timeout?

@Dirbaio
Copy link

@Dirbaio Dirbaio commented Jan 20, 2019

I'm also hitting this with a different use case: making a service that sends webhooks. I need a way to avoid posting webhooks to internal IPs.

  • Parsing the URL is not an option, since an external domain can resolve to internal IPs.
  • Resolving the host on my own and then doing the request if it's OK is vulnerable to races.
  • Resolving and rewriting the URL to contain the IP breaks Host header, SNI, round-robin DNS, Happy Eyeballs...

Ideally there would be a way to "MITM" the Resolver call, to inject my own error if a returned IP is internal.

@anacrolix
Copy link
Contributor

@anacrolix anacrolix commented Jan 21, 2019

I've had to do this kind of manual resolution with both torrent trackers and DHT global bootstrap resolution in order to implement IP blocklists. In both cases I'd rather just provide a resolution hook and filter or disallow IPs from the blocked ranges in the results.

@sajal
Copy link
Author

@sajal sajal commented Feb 11, 2019

@Dirbaio I did something similar in the past, but without doing any resolver stuff. (This is very old code, not sure if still valid)

func dialContext(ctx context.Context, network, address string) (net.Conn, error) {
	con, err := (&net.Dialer{
		Timeout:   dialtimeout, //DNS + Connect
		KeepAlive: keepalive,
	}).DialContext(ctx, network, address)
	if err == nil {
		//If a connection could be established, ensure its not local
		a, _ := con.RemoteAddr().(*net.TCPAddr)

		if islocalip(a.IP) {
			fmt.Println(a.IP)
			con.Close()
			return nil, securityerr
		}
	}
	return con, err
}

Usage

	//Configure our transport, new one for each request
	transport := &http.Transport{
		DialContext:           dialContext,
                ......
	}
	client := http.Client{
		Transport: transport,
                .....
	}

It does allow dialing to internal IP, but not able to send any requests to that.

@cevatbarisyilmaz
Copy link

@cevatbarisyilmaz cevatbarisyilmaz commented Nov 6, 2019

For anyone needs this functionality until a patch comes (if it comes at all), I recently wrote a partial net.Dialer replacement that accepts an interface Resolver. After using the custom resolver, it still uses net.Dialer internally and tries to mimic the original net.Dialer's behaviour when possible. It can also be used with http.Transport and http.Client.

Check it out here: https://github.com/cevatbarisyilmaz/ara

@ayanamist
Copy link

@ayanamist ayanamist commented Nov 6, 2019

For anyone needs this functionality until a patch comes (if it comes at all), I recently wrote a partial net.Dialer replacement that accepts an interface Resolver. After using the custom resolver, it still uses net.Dialer internally and tries to mimic the original net.Dialer's behaviour when possible. It can also be used with http.Transport and http.Client.

Check it out here: https://github.com/cevatbarisyilmaz/ara

I appreciate your work, can you provide dialParallel for dual stack dialing?

@cevatbarisyilmaz
Copy link

@cevatbarisyilmaz cevatbarisyilmaz commented Nov 6, 2019

Sure, I'll take a look.

@cevatbarisyilmaz
Copy link

@cevatbarisyilmaz cevatbarisyilmaz commented Nov 6, 2019

I pushed the commit though I didn't really test it yet (at least it didn't disrupt the current tests). Feel free to open an issue in the repo for any further things.

@jmalloc
Copy link

@jmalloc jmalloc commented Nov 6, 2019

Is there any chance that whatever approach is taken here (in the net package I mean) could/would allow for building a resolver that supports multicast DNS (even if it's only legacy queries)? It seems that native Go implementation does not support mDNS, and hence it is unavailable when building with CGO disabled.

@ncruces
Copy link
Contributor

@ncruces ncruces commented May 29, 2020

See github.com/ncruces/go-dns for more "prior art" on hooking into net.Resolver.Dial. Implemented caching, opportunistic encryption and DoH/DoS.

This bit implements the strategy mentioned in the above comment (a fake net.Conn that you can return from Dial). With this in place, implementing the DoH exchange is a relatively simple matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.