golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.46k stars 17.59k forks source link

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

Open sajal opened 9 years ago

sajal commented 9 years ago

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 commented 9 years ago

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

bradfitz commented 9 years ago

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 commented 9 years ago

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.

benburkert commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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] https://github.com/syncthing/syncthing-android/issues/412#issuecomment-115376055

sajal commented 8 years ago

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

jabley commented 8 years ago

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 commented 8 years ago

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

adg commented 8 years ago

This needs a proper proposal document to move forward.

adg commented 8 years ago

An extension to the work done in #16672

bradfitz commented 8 years ago

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

adg commented 7 years ago

Need design work, but should be good for Go 1.9.

polomsky commented 7 years ago

I have another use case for which this feature would be usable. I need to make some app firewall. I have proxy which uses http.Request. Urls are specified by user and I want to block access to several ip addresses, e.g. localhost. Currently I do not know any way how to do it except usage of Host header. But it is not sufficient, because I can not specify multiple ip addresses for request (in case when DNS server returns more addresses for same name). So custom dns resolver (with removing wrong ips) would be very great.

sajal commented 7 years ago

@polomsky I am doing something similar. In my Transport I am using my own dialer which is basically clone of the default dialer but applies the IP policy before making the connection usable. The drawback is a TCP connection was made but the client cant do anything with it if islocalip returns true.

sajal commented 7 years ago

So, currently net.Dialer accepts a net.Resolver which is a struct and not an interface. As I understand it, there is no way to override the Lookup* methods. Are there any plans to make it an interface? Just like how http.Client uses http.RoundTripper interface instead of explicit http.Transport struct.

bradfitz commented 7 years ago

@sajal, yes, we left room for in the design for that: https://github.com/golang/go/blob/ecc4474341504f5893c8333dbb68c520dbe93ca5/src/net/lookup.go#L100

sheerun commented 7 years ago

@bradfitz I'd like to point out that it would be nice if LookupPort could accept a hostname to connect to as well. For now it seems you expect port number as service argument, as shown for example by ReverseProxy implementation.

Use Case: Resolving to custom port with ReverseProxy (e.g. example.com should go to 127.0.0.1:3456)

mikegleasonjr commented 7 years ago

Hi guys, just wondering if go 1.9 beta drops tomorrow would it means this feature won't make its way into go 1.9?

ianlancetaylor commented 7 years ago

@mikegleasonjr As far as I know nobody has written the code for this. At this point I think it is unlikely to get into Go 1.9, but it is still possible if somebody sends in a patch very soon.

excavador commented 7 years ago

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

mauricio commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

jfesler commented 6 years ago

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 commented 6 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

benburkert commented 5 years ago

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

adamramadhan commented 5 years ago

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

Dirbaio commented 5 years ago

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.

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

anacrolix commented 5 years ago

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 commented 5 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Sure, I'll take a look.

cevatbarisyilmaz commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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

TyeMcQueen commented 1 year ago

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.