diff --git a/client/cmd/service_allow_group_unix.go b/client/cmd/service_allow_group_unix.go index 63d724ae9..c06e18dd8 100644 --- a/client/cmd/service_allow_group_unix.go +++ b/client/cmd/service_allow_group_unix.go @@ -4,6 +4,7 @@ package cmd import ( "fmt" + "net" "os" "path/filepath" "strconv" @@ -21,6 +22,27 @@ const ( restrictedSocketMode os.FileMode = 0660 ) +// umaskOwnerOnly masks every permission bit except the owner's, so a file +// created under it is 0600 whatever the process umask happens to be. +const umaskOwnerOnly = 0o177 + +// listenUnixPrivate binds a Unix socket that only its owner can connect to, +// whatever umask the service manager started the daemon with. applySocketAccess +// widens it afterwards to exactly what the configuration asks for. +// +// The umask is process-wide, so it is restored before returning and the window +// is kept to the bind itself. Nothing else creates files during daemon startup: +// this runs before the server and its goroutines exist. +func listenUnixPrivate(address string) (net.Listener, error) { + previous := syscall.Umask(umaskOwnerOnly) + listener, err := net.Listen("unix", address) + syscall.Umask(previous) + if err != nil { + return nil, err + } + return listener, nil +} + // resolveAllowGroup resolves one --allow-group value to a "gid:" // principal. A numeric value, with or without the prefix, is the GID itself; // anything else is a group name resolved through NSS, so groups that only diff --git a/client/cmd/service_allow_group_unix_test.go b/client/cmd/service_allow_group_unix_test.go index 0a7ea1bde..6e9310289 100644 --- a/client/cmd/service_allow_group_unix_test.go +++ b/client/cmd/service_allow_group_unix_test.go @@ -157,6 +157,32 @@ func TestApplySocketAccess(t *testing.T) { }) } +// The kernel checks a Unix socket's mode at connect(), not at accept(), so a +// socket that is briefly world-writable can be connected to before the daemon +// narrows it, and the caller stays connected afterwards. Binding under a +// restrictive umask closes that window whatever umask the service manager used. +func TestListenUnixPrivate_IgnoresAPermissiveUmask(t *testing.T) { + previous := syscall.Umask(0) + t.Cleanup(func() { syscall.Umask(previous) }) + + dir, err := os.MkdirTemp("", "nb-sock") + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, os.RemoveAll(dir)) }) + + path := filepath.Join(dir, "d.sock") + listener, err := listenUnixPrivate(path) + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, listener.Close()) }) + + assert.Equal(t, os.FileMode(0600), socketMode(t, path), + "the socket must be owner-only as bound, before any restriction is applied") + + // And the process umask is left as it was found. + restored := syscall.Umask(0) + syscall.Umask(restored) + assert.Equal(t, 0, restored, "listenUnixPrivate must restore the umask it changed") +} + func listenTestSocket(t *testing.T) string { t.Helper() diff --git a/client/cmd/service_allow_group_windows.go b/client/cmd/service_allow_group_windows.go index f108f3897..0eb12bbcb 100644 --- a/client/cmd/service_allow_group_windows.go +++ b/client/cmd/service_allow_group_windows.go @@ -4,6 +4,7 @@ package cmd import ( "fmt" + "net" "golang.org/x/sys/windows" @@ -47,6 +48,14 @@ func checkAllowGroupSet([]string) error { return nil } // afterwards. See allowedPipeSDDL. func applySocketAccess(string, []string) error { return nil } +// listenUnixPrivate binds a Unix socket. Windows has no umask, and a Unix +// socket there carries no mode the daemon could narrow, so there is nothing to +// do beyond binding it. A restriction on this transport is refused before it +// gets here: see listenOnAddress. +func listenUnixPrivate(address string) (net.Listener, error) { + return net.Listen("unix", address) +} + // allowedPipeSDDL renders the security descriptor for the daemon control pipe. // An empty principal list yields the descriptor that lets any local caller // connect. diff --git a/client/cmd/service_socket.go b/client/cmd/service_socket.go index bf1393afe..080b8893c 100644 --- a/client/cmd/service_socket.go +++ b/client/cmd/service_socket.go @@ -50,6 +50,18 @@ func listenOnAddress(addr string, allowed []string) (*socketListener, error) { if network == "unix" { removeStaleUnixSocket(address) + + // A Unix socket accepts connections the moment it is bound, and the + // kernel checks its mode at connect() rather than at accept(). Creating + // it owner-only closes the window between the bind and applySocketAccess: + // without this, a permissive umask leaves the socket open to everybody + // for that interval, and a caller that got in stays connected after the + // mode is narrowed. + listener, err := listenUnixPrivate(address) + if err != nil { + return nil, err + } + return &socketListener{Listener: listener, network: network, address: address}, nil } listener, err := net.Listen(network, address)