-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: make Resolver.PreferGo and GODEBUG=netdns=go use Go code on Windows #33097
Comments
Does Windows stop us from making direct DNS UDP requests? (This is the reason we don't do it on macOS.) Otherwise I don't see why we currently go out of our way to bypass the plain Go code, which was written first. There must be a reason. What is it? Windows-specific name resolution mechanisms like NetBIOS? We could also use the plain UDP DNS if we can detect that there's nothing special about the rest or the host system, which I think we try to do on macOS. More information is needed here. |
Pebble accepts a `--dnsserver` flag, that allows to select a custom DNS resolver to use when validating the ACME challenges. This option is critical to make the certbot integration tests work in particular. Current implementation is to set the `net.DefaultResolver` on a new `net.Resolver` instance with a custom `Dial` property to consume the value from `--dnsserver`. This allows to transparently resolve any host using this custom DNS resolver from a pure-Go implementation of the DNS resolution API. Sadly this approach does not work on Windows, and the `--dnsserver` has no effect on how resolution is done, and it is always the OS mechanism that is used. One can reveal this behavior by running the following piece of code on Linux and on Windows: ```go // test.go package main import ( "context" "fmt" "net" ) func main() { resolver := &net.Resolver{ PreferGo: true, Dial: func(ctx context.Context, _, _ string) (net.Conn, error) { d := net.Dialer{} return d.DialContext(ctx, "udp", "4.3.2.1:404") }, } net.DefaultResolver = resolver ctx, cancelfunc := context.WithTimeout(context.Background(), 30) defer cancelfunc() _, err := resolver.LookupTXT(ctx, "stupid.entry.net") fmt.Printf("Error is: %s\n", err) } ``` This piece of code tries to resolve a non-existent domain on a non-existent DNS server as IP `4.3.2.1:404`. On Linux, you will get the following error: ```bash Error is: lookup stupid.entry.net on 127.0.0.53:53: dial udp 4.3.2.1:404: i/o timeout ``` That indicates that the system tried to reach the non-existent DNS server, and get back a timeout exception on it. However on Windows you will get: ```bash Error is: lookup stupid.entry.net: dnsquery: DNS name does not exist. ``` This indicates that the system ignored the dummy DNS server address, contacted the OS DNS resolver, that responded that the DNS name does not exist. One can see also the reason for this behavior on Windows on the `net` godoc, https://godoc.org/net, in particular this line in the module introduction: ``` On Windows, the resolver always uses C library functions, such as GetAddrInfo and DnsQuery. ``` In fact, the pure Go DNS resolver is not applicable on Windows, the OS DNS resolver will be used, ignoring any customization. Several relevant discussions, in particular a proposal (not developed yet) to make the pure Go DNS resolver available on Windows: * golang/go#29621 * golang/go#33097 * golang/go#33086 To fix this, this PR makes Pebble switch to a different logic: * if `-dnsserver` is not set, use the default API to resolve the names * if `-dnsserver` is set, use a dedicated DNS client, to avoid to use the OS one both on Linux and Windows The DNS client is https://github.com/miekg/dns, a highly used and supported DNS library. With these modifications, integrations tests on Certbot are running correctly both on Linux and Windows.
The previous comment (from Jul 16) asked some questions that still need to be answered. We can't move forward with this proposal without answers. |
Appears this was missed by triage... cc @alexbrainman @zx2c4 @mattn @jordanrh1 @bradfitz |
I don't know. But Windows could certainly restrict DNS UDP. Even external firewall can restrict us.
Maybe I mis-remember. Windows has built in network name resolver. The resolver uses not just DNS to do its work. Go net code just calls into that resolver. This makes Go name resolution consistent with other Windows programs. There is only one external lookup API for net package. And that is why Windows users cannot access our pure Go DNS resolver code. I am pretty sure I complained about this to you, but you decided it was not important at the time.
Quite possible. But I am not Windows network expert - do not take my word for it. Alex |
The Windows resolver is weird and particular, and I doubt we're going to be able to copy that internally from Go. Setting aside the issue of whether we can reliably get out those UDP packets, the Windows resolver has specific behavior with handling several name servers on several interfaces, and it has specific behavior when dealing with things like Active Directory. Think of |
@zx2c4, I agree that the Windows resolver should remain the default, but I think this bug is about permitting the opt-in use of the Go resolver, either at a whole-program (build tag) level or in code by e.g. https://golang.org/pkg/net/#Resolver.PreferGo |
Gotcha. That's fine with me. Is there still a question then of whether udp 53 sockets are possible? If so I can experiment a bit. |
Retitled to "proposal: net: make Resolver.PreferGo and GODEBUG=netdns=go use Go code on Windows" based on @bradfitz's comment above. I originally thought this issue was about using the Go code by default in certain cases. I don't think we know when that would be appropriate, nor how to detect it. But if this is only about making it possible for programs to explicitly say "use the Go resolver" and have that actually take effect (as it does on other operating systems), then this seems very easy to accept - it's not much more than a bug fix. Based on the narrower scope implied by the retitling, this seems like a likely accept. Leaving open for a week for final comments. |
I agree. We should treat this as a bug fix. Alex |
No final comments. Accepted |
Is there an ETA for this? If not, I would check if we can contribute a patch for this, no promises though. |
Change https://go.dev/cl/401754 mentions this issue: |
…an 9" This reverts CL 400654. Reason for revert: broke net.TestGoLookupIP on the darwin-amd64-nocgo builder. Updates #33097. Change-Id: Idaf94eda88c9d4401e667a4d31c00ce376d91909 Reviewed-on: https://go-review.googlesource.com/c/go/+/401754 Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Than McIntosh <thanm@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
Reopened because the implementation CL was reverted. |
Change https://go.dev/cl/409234 mentions this issue: |
Change https://go.dev/cl/412139 mentions this issue: |
For #33097 Change-Id: I6138dc844f0b29b01c78a02efc1e1b1ad719b803 Reviewed-on: https://go-review.googlesource.com/c/go/+/412139 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com>
Change https://go.dev/cl/413458 mentions this issue: |
Windows can call the C DNS lookup routines even without cgo, so don't force it to use the Go routines in that scenario. No test because the test requires building the tools with CGO_ENABLED=0. For #33097 Fixes #53490 Change-Id: I3595a68e788be0d3bbd1bbd431836aca20a7d757 Reviewed-on: https://go-review.googlesource.com/c/go/+/413458 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com>
There has been a recent uptick in the failure rate of |
Thank you for fixing this bug. Our security products on Windows need to take the advantage of the alternate dialer for |
Windows can call the C DNS lookup routines even without cgo, so don't force it to use the Go routines in that scenario. No test because the test requires building the tools with CGO_ENABLED=0. For golang#33097 Fixes golang#53490 Change-Id: I3595a68e788be0d3bbd1bbd431836aca20a7d757 Reviewed-on: https://go-review.googlesource.com/c/go/+/413458 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com>
It seems the implementation for this may have been incomplete; see #57757. |
Go 1.19 introduced Resolver.PreferGo which allows the Go resolver to be used on Windows. See golang/go#33097. Fixes tailscale#5161 Signed-off-by: Thomas Way <thomas@6f.io>
Problem
Currently DNS resolution on Windows always calls the C functions, which means DNS resolution cannot be configured using a custom dialer. This has two primary downsides:
nslookup
.Proposal
Enable the use of the built-in DNS stub resolver on Windows so that custom dialers can be used to control the behavoir of Go's resolution when needed.
See Also
#33086
#29621
The text was updated successfully, but these errors were encountered: