diff --git a/fd_linux.go b/fd_linux.go new file mode 100644 index 0000000..3a3408b --- /dev/null +++ b/fd_linux.go @@ -0,0 +1,25 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build linux + +package main + +import "syscall" + +// socketpair returns a new AF_LOCAL SOCK_STREAM socketpair whose ends +// are both close-on-exec from birth, so spawned processes do not +// inherit them. Linux creates the pair atomically with SOCK_CLOEXEC. +func socketpair() ([2]int, error) { + return syscall.Socketpair(syscall.AF_LOCAL, syscall.SOCK_STREAM|syscall.SOCK_CLOEXEC, 0) +} + +// recvFlags are the flags TsnetAccept passes to Recvmsg: MSG_WAITALL +// to keep listener socketpair record framing exact, plus +// MSG_CMSG_CLOEXEC so the connection fd installed by SCM_RIGHTS is +// close-on-exec without a race window. +const recvFlags = syscall.MSG_WAITALL | syscall.MSG_CMSG_CLOEXEC + +// closeOnExec marks fd close-on-exec. It is a no-op on Linux, where +// recvFlags already made received fds close-on-exec. +func closeOnExec(fd int) {} diff --git a/fd_other.go b/fd_other.go new file mode 100644 index 0000000..1834355 --- /dev/null +++ b/fd_other.go @@ -0,0 +1,42 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !linux + +package main + +import "syscall" + +// socketpair returns a new AF_LOCAL SOCK_STREAM socketpair whose ends +// are both close-on-exec, so spawned processes do not inherit them. +// +// Platforms without SOCK_CLOEXEC (darwin) create the pair while +// holding ForkLock's read lock, which delays any concurrent fork +// (which takes the write lock) until both ends are marked +// close-on-exec. This is the same dance package net does for +// accept(2). +func socketpair() ([2]int, error) { + syscall.ForkLock.RLock() + defer syscall.ForkLock.RUnlock() + + fds, err := syscall.Socketpair(syscall.AF_LOCAL, syscall.SOCK_STREAM, 0) + if err != nil { + return fds, err + } + syscall.CloseOnExec(fds[0]) + syscall.CloseOnExec(fds[1]) + return fds, nil +} + +// recvFlags are the flags TsnetAccept passes to Recvmsg: MSG_WAITALL +// to keep listener socketpair record framing exact. Platforms without +// MSG_CMSG_CLOEXEC mark the fd received via SCM_RIGHTS close-on-exec +// with closeOnExec instead. +const recvFlags = syscall.MSG_WAITALL + +// closeOnExec marks fd close-on-exec. It runs right after Recvmsg +// installed fd, closing the window in which a concurrent fork could +// inherit it as far as the kernel allows. +func closeOnExec(fd int) { + syscall.CloseOnExec(fd) +} diff --git a/sourcepkg/Makefile b/sourcepkg/Makefile index e33dbcb..2016df0 100644 --- a/sourcepkg/Makefile +++ b/sourcepkg/Makefile @@ -24,8 +24,8 @@ check: libtailscale.tar.zst clean: rm -rf ./libtailscale.tar.zst ../vendor ./libtailscale -../vendor: ../go.mod ../go.sum ../tailscale.go Makefile.src Makefile +../vendor: ../go.mod ../go.sum ../tailscale.go ../fd_linux.go ../fd_other.go Makefile.src Makefile go mod vendor -libtailscale.tar.zst: Makefile.src configure ../vendor ../LICENSE ../tailscale.go ../go.mod ../go.sum - $(TAR) --transform 's#^#libtailscale/#' --transform 's#Makefile.src#Makefile#' -acf $@ Makefile.src configure ../vendor ../LICENSE ../tailscale.go ../go.mod ../go.sum \ No newline at end of file +libtailscale.tar.zst: Makefile.src configure ../vendor ../LICENSE ../tailscale.go ../fd_linux.go ../fd_other.go ../go.mod ../go.sum + $(TAR) --transform 's#^#libtailscale/#' --transform 's#Makefile.src#Makefile#' -acf $@ Makefile.src configure ../vendor ../LICENSE ../tailscale.go ../fd_linux.go ../fd_other.go ../go.mod ../go.sum \ No newline at end of file diff --git a/tailscale.go b/tailscale.go index ce7cb74..0f693bc 100644 --- a/tailscale.go +++ b/tailscale.go @@ -240,7 +240,9 @@ func TsnetListen(sd C.int, network, addr *C.char, listenerOut *C.int) C.int { // feed an fd for the connection through the listener. This lets C use // epoll on the tailscale_listener to know if it should call // tailscale_accept, which avoids a blocking call on the far side. - fds, err := syscall.Socketpair(syscall.AF_LOCAL, syscall.SOCK_STREAM, 0) + // Both ends are close-on-exec, so spawned processes do not inherit + // them. + fds, err := socketpair() if err != nil { return s.recErr(err) } @@ -336,7 +338,8 @@ func TsnetAccept(listenerFd C.int, connOut *C.int) C.int { // One record per connection: the fd via SCM_RIGHTS plus its remote // IP. The fd parsed below is the number the kernel installed, i.e. - // the number C will hold. + // the number C will hold, and it is close-on-exec (via recvFlags on + // Linux, or the closeOnExec call below elsewhere). // // The socketpair is a stream socket, so records are framed here: // each read asks for the remaining bytes of the head record, with @@ -352,7 +355,7 @@ func TsnetAccept(listenerFd C.int, connOut *C.int) C.int { // the cmsg rode with the record's first byte; only the tail remains cbuf = nil } - nn, on, _, _, err := syscall.Recvmsg(int(listenerFd), buf, cbuf, syscall.MSG_WAITALL) + nn, on, _, _, err := syscall.Recvmsg(int(listenerFd), buf, cbuf, recvFlags) n += nn if nn > 0 { oobn = on @@ -384,6 +387,7 @@ func TsnetAccept(listenerFd C.int, connOut *C.int) C.int { return ln.s.recErr(fmt.Errorf("libtailscale: got %d FDs, want 1", len(fds))) } fd := C.int(fds[0]) + closeOnExec(int(fd)) // the entry must exist before C can learn this fd number addrLen := bytes.IndexByte(data, 0) @@ -400,7 +404,7 @@ func TsnetAccept(listenerFd C.int, connOut *C.int) C.int { } func newConn(s *server, netConn net.Conn, connOut *C.int) error { - fds, err := syscall.Socketpair(syscall.AF_LOCAL, syscall.SOCK_STREAM, 0) + fds, err := socketpair() if err != nil { return err } diff --git a/tailscale_test.go b/tailscale_test.go index ee8e822..71d17a4 100644 --- a/tailscale_test.go +++ b/tailscale_test.go @@ -1,10 +1,12 @@ package main import ( + "syscall" "testing" "time" "github.com/tailscale/libtailscale/tsnetctest" + "golang.org/x/sys/unix" ) func TestConn(t *testing.T) { @@ -66,6 +68,89 @@ func TestGetRemoteAddr(t *testing.T) { tsnetctest.RunTestGetRemoteAddr(t) } +// TestSocketpairCloexec checks that socketpair returns both ends +// close-on-exec, so that spawned processes cannot inherit the +// listener and connection fds. +func TestSocketpairCloexec(t *testing.T) { + fds, err := socketpair() + if err != nil { + t.Fatal(err) + } + defer syscall.Close(fds[0]) + defer syscall.Close(fds[1]) + + for _, fd := range fds { + flags, err := unix.FcntlInt(uintptr(fd), syscall.F_GETFD, 0) + if err != nil { + t.Fatal(err) + } + if flags&syscall.FD_CLOEXEC == 0 { + t.Errorf("socketpair end %d is not close-on-exec", fd) + } + } +} + +// TestRecvFdCloexec exercises the SCM_RIGHTS receive path TsnetAccept +// uses: an fd passed over the listener socketpair must be +// close-on-exec by the time it reaches C (via MSG_CMSG_CLOEXEC in +// recvFlags on Linux, via closeOnExec on platforms without it). +func TestRecvFdCloexec(t *testing.T) { + sp, err := socketpair() + if err != nil { + t.Fatal(err) + } + defer syscall.Close(sp[0]) + defer syscall.Close(sp[1]) + + // The fd we ship across the socketpair. + pair, err := socketpair() + if err != nil { + t.Fatal(err) + } + defer syscall.Close(pair[0]) + defer syscall.Close(pair[1]) + + var rec [listenAddrLen]byte + copy(rec[:], "1.2.3.4") + if err := syscall.Sendmsg(sp[0], rec[:], syscall.UnixRights(pair[0]), nil, 0); err != nil { + t.Fatal(err) + } + + data := make([]byte, listenAddrLen) + cbuf := make([]byte, syscall.CmsgSpace(4)) + n, oobn, _, _, err := syscall.Recvmsg(sp[1], data, cbuf, recvFlags) + if err != nil { + t.Fatal(err) + } + if n != listenAddrLen { + t.Fatalf("got %d bytes, want %d", n, listenAddrLen) + } + scms, err := syscall.ParseSocketControlMessage(cbuf[:oobn]) + if err != nil { + t.Fatal(err) + } + if len(scms) != 1 { + t.Fatalf("got %d control messages, want 1", len(scms)) + } + fds, err := syscall.ParseUnixRights(&scms[0]) + if err != nil { + t.Fatal(err) + } + if len(fds) != 1 { + t.Fatalf("got %d fds, want 1", len(fds)) + } + closeOnExec(fds[0]) // mirrors TsnetAccept + defer syscall.Close(fds[0]) + + flags, err := unix.FcntlInt(uintptr(fds[0]), syscall.F_GETFD, 0) + if err != nil { + t.Fatal(err) + } + if flags&syscall.FD_CLOEXEC == 0 { + t.Errorf("fd received via SCM_RIGHTS is not close-on-exec") + } +} + func TestExtractIP(t *testing.T) { ipv4 := "1.23.33.4:12343" ipv6 := "[1::2234::34fc::44]:56576" diff --git a/tsnetctest/tsnetctest.go b/tsnetctest/tsnetctest.go index ed4e5cc..1d43529 100644 --- a/tsnetctest/tsnetctest.go +++ b/tsnetctest/tsnetctest.go @@ -10,6 +10,7 @@ package tsnetctest #include #include #include +#include #include "../tailscale.h" char* tmps1; @@ -27,6 +28,21 @@ char* err = NULL; tailscale s1, s2; +// check_cloexec fails the test if fd is not close-on-exec. Listener +// and connection fds must not leak into processes the host forks. +int check_cloexec(int fd) { + int flags = fcntl(fd, F_GETFD); + if (flags == -1) { + snprintf(err, errlen, "fcntl(%d, F_GETFD): errno %d (%s)", fd, errno, strerror(errno)); + return 1; + } + if (!(flags & FD_CLOEXEC)) { + snprintf(err, errlen, "fd %d is not close-on-exec", fd); + return 1; + } + return 0; +} + int set_err(tailscale sd, char tag) { err[0] = tag; err[1] = ':'; @@ -74,16 +90,25 @@ int test_conn() { if ((ret = tailscale_listen(s1, "tcp", ":8081", &ln)) != 0) { return set_err(s1, '8'); } + if (check_cloexec(ln)) { + return 1; + } tailscale_conn w; if ((ret = tailscale_dial(s2, "tcp", "100.64.0.1:8081", &w)) != 0) { return set_err(s2, '9'); } + if (check_cloexec(w)) { + return 1; + } tailscale_conn r; if ((ret = tailscale_accept(ln, &r)) != 0) { return set_err(s2, 'a'); } + if (check_cloexec(r)) { + return 1; + } const char want[] = "hello"; ssize_t wret;