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: Moving listenerBacklog from init interferes with use of OpenBSD's pledge syscall #31927

Closed
esote opened this issue May 9, 2019 · 5 comments
Closed

Comments

@esote
Copy link

@esote esote commented May 9, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.1 openbsd/amd64

Does this issue reproduce with the latest release?

Most likely, since the code that causes the issue is still there, but 1.12.1 is the latest version in ports for OpenBSD 6.5.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="openbsd"
GOOS="openbsd"
GOPATH="/home/user/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/openbsd_amd64"
GCCGO="gccgo"
CC="cc"
CXX="c++"
CGO_ENABLED="1"
GOMOD=""
CGO_FLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build867684327=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Use the pledge(2) system call before something which reaches line 57 of socket() in src/net/sock_posix.go calling maxListenerBacklog().

package main

import (
	"io"
	"net/http"

	"golang.org/x/sys/unix"
)

func main() {
	unix.Pledge("stdio inet", "")
	h := func(w http.ResponseWriter, r *http.Request) {
		io.WriteString(w, "hi\n")
	}
	http.HandleFunc("/", h)
	http.ListenAndServe(":8080", nil)
}

What did you expect to see?

A functioning web server listening on port 8080.

What did you see instead?

signal: abort trap (core dumped)

Details

This is caused by @bradfitz's commit (related to #26775). The minimal example above would work fine prior to this commit.

In OpenBSD the pledge(2) system call is used to reduce the operating mode of the running process. It is common to reduce the permitted operations during the lifetime of the process. For example: allowing calls to read(2) and socket(2) then reducing to allow only socket(2) from then on out --- done through first pledging "rpath inet" then "inet" later. This greatly improves security during runtime and is used everywhere in OpenBSD.

When pledge(2) is first called it immediately restricts which sysctl(2) information can be queried (see here). This includes disallowing kern.somaxconn ie [CTL_KERN, KERN_SOMAXCONN].

Before the commit above, kern.somaxconn would be queried during initialization and thus would not interfere with any calls to pledge(2) during run time. After the commit, kern.somaxconn is queried in socket() causing pledge to shoot the disobeying process in the head.

This greatly restricts how pledge(2) can be used with certain aspects of the net package. For example, to bypass this issue one would need to do something like below (and still no calls to pledge(2) can happen before the call to http.ListenAndServe().

package main

import (
	"io"
	"net/http"
	"sync"
	"time"

	"golang.org/x/sys/unix"
)

func main() {
	h := func(w http.ResponseWriter, r *http.Request) {
		io.WriteString(w, "hi\n")
	}
	http.HandleFunc("/", h)

	var w sync.WaitGroup
	w.Add(1)

	go func() {
		defer w.Done()
		http.ListenAndServe(":8080", nil)
	}()

	time.Sleep(time.Millisecond)

	unix.Pledge("stdio inet", "")

	w.Wait()
}

I think we should opt to move the call to maxListenerBacklog() back into initialization. Perhaps it is possible to only have this done on OpenBSD systems where it's an issue. But I am open to other suggestions which are cleaner than my workaround above.

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented May 9, 2019

You can add these lines before your unix.Pledge():

      // Force initialization of lazy sysctls in package net:
      if ln, err := net.Listen("tcp", "localhost:0"); err != nil {
            log.Fatal(err)
      } else {
            ln.Close()
      }
@esote

This comment has been minimized.

Copy link
Author

@esote esote commented May 9, 2019

@bradfitz Haha, I guess that'll work for now because it's kept in a cache. Still feels like a hack though since it's relying on the underlying implementation.

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented May 9, 2019

One could also argue that dropping a random system call into a random lifetime of a program is kinda hacky. Or you could counter-argue that it's a low-level thing and you know what you're doing. If so, you can also know how the net package works. You have tests, right? If OpenBSD doesn't kill your test, that's a good sign.

@esote

This comment has been minimized.

Copy link
Author

@esote esote commented May 9, 2019

Yeah fair point, it's a pretty easy thing to test for, even if only to test that the net implementation hasn't changed to no longer justify lazy init in a program.

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented May 9, 2019

I'm doing to close this for now. We're not ready to commit to a supported place to make pledges but we can revisit in the future if this gets more painful or gets to be a more common request (especially over more operating systems).

@bradfitz bradfitz closed this May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.