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 · 48 comments
Open

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

sajal opened this issue Sep 4, 2015 · 48 comments

Comments

@sajal
Copy link

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

@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 commented Sep 9, 2015

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

@bradfitz
Copy link
Contributor

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

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

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

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

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

@adg
Copy link
Contributor

adg commented Jul 19, 2016

This needs a proper proposal document to move forward.

@adg
Copy link
Contributor

adg commented Oct 3, 2016

An extension to the work done in #16672

@bradfitz
Copy link
Contributor

bradfitz commented Oct 3, 2016

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

@gopherbot
Copy link

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

@jfesler
Copy link

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.

@mrwiora
Copy link

mrwiora 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

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

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

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

@benburkert
Copy link
Contributor

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

@adamramadhan
Copy link

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

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

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

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

Sure, I'll take a look.

@cevatbarisyilmaz
Copy link

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
Contributor

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

@codeskyblue
Copy link

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

I figured out the code bellow.

package main

import (
	"log"
	"net"
	"time"

	"github.com/benburkert/dns"
)

func init() {
	zone := &dns.Zone{
		Origin: "example.org.",
		TTL:    5 * time.Minute,
		RRs: dns.RRSet{
			"foo": {
				dns.TypeA: []dns.Record{
					&dns.A{A: net.ParseIP("1.2.3.4")},
				},
			},
		},
	}
	mux := new(dns.ResolveMux)
	mux.Handle(dns.TypeANY, zone.Origin, zone)

	net.DefaultResolver = &net.Resolver{
		PreferGo: true,
		Dial: (&dns.Client{
			Resolver: mux,
		}).Dial,
	}
}

func main() {
	log.Println(net.LookupHost("foo.example.org")) // it's working. output: [1.2.3.4] <nil>
	log.Println(net.LookupHost("www.example.org")) // error no such host. I don't known how to fallback to DNS query
}

@aojea
Copy link
Contributor

aojea commented Sep 7, 2021

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

@bradfitz check if this similar to what you had in mind https://go-review.googlesource.com/c/net/+/347850

@ns-jisorce
Copy link

Hi @aojea Hi @bradfitz , any update on this ? Thx

@TyeMcQueen
Copy link

I've prototyped a fairly simple change that significantly narrows the required customization to a single-function interface that doesn't abstract everything that Resolver does, just the part that is used by net.Dialer (and thus by net/http), namely the internetAddrList() private method.

In the net package, I add (in dial.go in my prototype):

type InternetAddrLister interface {
    InternetAddrList(ctx context.Context, net, addr string) ([]Addr, error)
}

And allow users to provide an alternate implementation for net.Dialer to use.

For maximum usefulness, it is best to also allow a *net.Resolver to be used as an InternetAddrLister so people can provide a replacement by just wrapping around the existing net.Resolver implementation (for cases where they want to do light customization rather than provide a whole custom resolution implementation).

I think this function is a very useful abstraction (and the Go authors appear to agree since this is used quite a few places) and we should just make it a public method of Resolver.

I can submit my changes but I wanted to align on the approach before doing so.

dmke added a commit to dmke/caddy-remote-host that referenced this issue Apr 22, 2024
This mocks net.LookupIP with a wrapper type. In future, this might
be easier to accomplish with the help of the standard library (see
discussion in golang/go#12503).

With muety#1 in the
pipeline, I might consider swapping the stdlib resolver with a
full fledged DNS client (github.com/miekg/dns), as the stdlib
resolver does not return TTL values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests