net: acquireThread might block for a long time

[mateusz834]

This was previously reported to the go security team.

When the cgo resolver is being used, we call the acquireThread function to limit the amount of concurrent cgo calls running and also to limit the amount of threads that will be ever created (by the net package) by the runtime for cgo calls. The limit is currently capped to 500.

go/src/net/cgo_unix.go

Lines 148 to 151 in 9d836d4

	func cgoLookupHostIP(network, name string) (addrs []IPAddr, err error) {
	acquireThread()
	defer releaseThread()

go/src/net/net.go

Lines 674 to 689 in 9d836d4

	// Limit the number of concurrent cgo-using goroutines, because
	// each will block an entire operating system thread. The usual culprit
	// is resolving many DNS names in separate goroutines but the DNS
	// server is not responding. Then the many lookups each use a different
	// thread, and the system or the program runs out of threads.
	var threadLimit chan struct{}
	var threadOnce sync.Once
	func acquireThread() {
	threadOnce.Do(func() {
	threadLimit = make(chan struct{}, concurrentThreadsLimit())
	})
	threadLimit <- struct{}{}
	}

go/src/net/rlimit_unix.go

Lines 11 to 33 in 9d836d4

	// concurrentThreadsLimit returns the number of threads we permit to
	// run concurrently doing DNS lookups via cgo. A DNS lookup may use a
	// file descriptor so we limit this to less than the number of
	// permitted open files. On some systems, notably Darwin, if
	// getaddrinfo is unable to open a file descriptor it simply returns
	// EAI_NONAME rather than a useful error. Limiting the number of
	// concurrent getaddrinfo calls to less than the permitted number of
	// file descriptors makes that error less likely. We don't bother to
	// apply the same limit to DNS lookups run directly from Go, because
	// there we will return a meaningful "too many open files" error.
	func concurrentThreadsLimit() int {
	var rlim syscall.Rlimit
	if err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rlim); err != nil {
	return 500
	}
	r := rlim.Cur
	if r > 500 {
	r = 500
	} else if r > 30 {
	r -= 30
	}
	return int(r)
	}

This might be problematic for services that connect to a user-provided hostnames.
The user-provided domain might be running on an "evil" nameserver that might trip timeouts (respond with SERVFAIL after ~5s). I have not tested this deeply, but I assume it would be possible to make the getaddrinfo block for 5-30s (assuming mostly default configuration in resolv.conf).

1-3 (Resolvers Count) * 2 (default attempts count) * 5s (default timeout).

Obviously with MITM it is simpler to trip timeouts.

We don't use the cgo resolver much these days on unix systems (except desktop linux, because of systemd nsswitch modules we don't support in the go resolver). On windows and darwin the cgo resolver is the default.

• I think this should be at least made clear by the docs, currently we only note that:
  

  go/src/net/net.go

  Lines 49 to 50 in 9d836d4

  	By default the pure Go resolver is used, because a blocked DNS request consumes
  	only a goroutine, while a blocked C call consumes an operating system thread.

  
  This should probably mention that we have an internal cgo threads limit.

• Also we probably should not force the use of cgo resolver for ".local" subdomains, so that use of the cgo resolver cannot be forced by an external user (consider a service that connects to a user-provided hostname). The go resolver should be able to resolve ".local" domains when there are no other nss modules in use than files and dns.
  

  go/src/net/conf.go

  Lines 341 to 347 in 9d836d4

  	if canUseCgo && stringsHasSuffixFold(hostname, ".local") {
  	// Per RFC 6762, the ".local" TLD is special. And
  	// because Go's native resolver doesn't do mDNS or
  	// similar local resolution mechanisms, assume that
  	// libc might (via Avahi, etc) and use cgo.
  	return hostLookupCgo, dnsConf
  	}

• Support context cancellation in acquireThread, so that when the thread limit is reached we don't call cgo stuff when the context is already cancelled (don't queue bunch of cgo calls when context is done).

I will send CLs for this.

CC @ianlancetaylor @golang/security @rolandshoemaker

[gopherbot]

Change https://go.dev/cl/539360 mentions this issue: net: support context cancellation in acquireThread

[gopherbot]

Change https://go.dev/cl/540555 mentions this issue: net: don't force cgo resolver for .local subdomain queries

[gopherbot]

Change https://go.dev/cl/539361 mentions this issue: net: mention the cgo thread limit in package docs

[mknyszek]

@bcmills Can you provide more context on why you added the compiler/runtime label? In triage I'm not sure we're seeing how this should be addressed in the compiler and/or runtime. Thanks.

[bcmills]

Huh — I don't remember. 😅

I think I misinterpreted acquireThread as a thing that acquires actual threads from the runtime and/or poller, but in practice it appears to only acquire semaphore tokens limiting implicitly-aquired threads. Sorry for the triage noise!

[rolandshoemaker]

@mateusz834 was there anything left to do here? I think all of the proposed changes ended up landing.

[mateusz834]

@rolandshoemaker right, closing the issue