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: DNS resolution can use the wrong server in race condition (resolv.conf stale information) #14072

Closed
andrew-edgar opened this issue Jan 22, 2016 · 5 comments

Comments

Projects
None yet
6 participants
@andrew-edgar
Copy link

commented Jan 22, 2016

There can be a race condition on reading resolv.conf that can allow go programs to contain stale information to perform DNS lookups.

resolv.conf is read in net/conf.go initConfVal() and stored in systemConf().resolv

confVal.resolv = dnsReadConfig("/etc/resolv.conf")

In dnsclient_unix.go the "init" function will get that information from systemConf().

conf.dnsConfig = systemConf().resolv

The function then does a stat on the file to set what the conf.modTime is to check for updates to the file.

if fi, err := os.Stat("/etc/resolv.conf"); err == nil {

The race can occur if /etc/resolv.conf is written to between when conf.go reads the file and when the stat() is done in the init() function because the saved modification time of the conf file is set on the first time we call tryUpdate, but not when we call the initConfVal() function mentioned above.

func (conf *resolverConfig) tryUpdate(name string) {

The tryUpdate() function will now not re-read /etc/resolv.conf until it changes again.

We are working around this by setting GODEBUG=netdns=cgo which does not cache the resolv.conf information.

@ianlancetaylor ianlancetaylor changed the title DNS resolution can use the wrong server in race condition (resolv.conf stale information) net: DNS resolution can use the wrong server in race condition (resolv.conf stale information) Jan 22, 2016

@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Jan 22, 2016

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2016

@mdempsky

This comment has been minimized.

Copy link
Member

commented Jan 22, 2016

I think https://golang.org/cl/18860/ should fix this, but I say wait for 1.7. I believe this issue was present in 1.5, so it's not a regression in 1.6.

(Also, does gopherbot not link to CLs if you reply "R=go1.7" immediately after uploading? It also never linked https://golang.org/cl/18623/ to its issue.)

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jan 23, 2016

(Also, does gopherbot not link to CLs if you reply "R=go1.7" immediately after uploading? It also never linked https://golang.org/cl/18623/ to its issue.)

That's @rsc's script somewhere. It doesn't run too often, but it's been plenty of time. Maybe it's confused by the period at the end of the Fixes line?

@luan

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2016

I'm surprised this isn't being ported to 1.6. Seems like a serious enough issue, also I'd be nice to start using the new DNS code ASAP rather than sticking to cgo for it. I'd even argue that it should be on a patch to 1.5. Even though this technically isn't a regression since the code is all new, it is a regression in functionality since 1.4 worked perfectly (albeit not using that code).

@gopherbot

This comment has been minimized.

Copy link

commented Feb 18, 2016

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

@gopherbot gopherbot closed this in 5efbdd9 Feb 20, 2016

eswierk added a commit to skyportsystems/go that referenced this issue Aug 17, 2016

net: fix race in (*resolverConfig).tryUpdate
Fixes golang#14072.

Change-Id: Ie31caa06690ac621906fc5acd34da2efa4e2049f
Reviewed-on: https://go-review.googlesource.com/18860
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>

@golang golang locked and limited conversation to collaborators Feb 28, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.