From 196500f5ec31772eb850ec975731ff48c1b3274e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 15 Sep 2020 12:37:22 +0200 Subject: [PATCH] NewUnixSocketWithOpts(): reduce umask override time-window, document hack This hack was originally added in https://github.com/moby/moby/commit/24c73ce2d3d572313fe56bad08819e0ca8b74d26, but was scarce on information, and this code was cause for some confusion. net.Listen does not allow for permissions to be set. As a result, when specifying custom permissions ("WithChmod()"), there is a short time between creating the socket and applying the permissions, during which the socket permissions are Less restrictive than desired. To work around this limitation of net.Listen(), we temporarily set the umask to 0777, which forces the socket to be created with 000 permissions (i.e.: no access for anyone). After that, WithChmod() must be used to set the desired permissions. This patch also removes the use of `defer` here, so that we can reset the umask to its original value as soon as possible. Ideally we'd be able to detect if WithChmod() was passed as an option, and skip changing umask if default permissions are used. Signed-off-by: Sebastiaan van Stijn --- sockets/unix_socket.go | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/sockets/unix_socket.go b/sockets/unix_socket.go index bd3336de..e7591e6e 100644 --- a/sockets/unix_socket.go +++ b/sockets/unix_socket.go @@ -67,7 +67,7 @@ func WithChown(uid, gid int) SockOption { } } -// WithChmod modifies socket file's access mode +// WithChmod modifies socket file's access mode. func WithChmod(mask os.FileMode) SockOption { return func(path string) error { if err := os.Chmod(path, mask); err != nil { @@ -77,15 +77,35 @@ func WithChmod(mask os.FileMode) SockOption { } } -// NewUnixSocketWithOpts creates a unix socket with the specified options +// NewUnixSocketWithOpts creates a unix socket with the specified options. +// By default, socket permissions are 0000 (i.e.: no access for anyone); pass +// WithChmod() and WithChown() to set the desired ownership and permissions. +// +// This function temporarily changes the system's "umask" to 0777 to work around +// a race condition between creating the socket and setting its permissions. While +// this should only be for a short duration, it may affect other processes that +// create files/directories during that period. func NewUnixSocketWithOpts(path string, opts ...SockOption) (net.Listener, error) { if err := syscall.Unlink(path); err != nil && !os.IsNotExist(err) { return nil, err } - mask := syscall.Umask(0777) - defer syscall.Umask(mask) + // net.Listen does not allow for permissions to be set. As a result, when + // specifying custom permissions ("WithChmod()"), there is a short time + // between creating the socket and applying the permissions, during which + // the socket permissions are Less restrictive than desired. + // + // To work around this limitation of net.Listen(), we temporarily set the + // umask to 0777, which forces the socket to be created with 000 permissions + // (i.e.: no access for anyone). After that, WithChmod() must be used to set + // the desired permissions. + // + // We don't use "defer" here, to reset the umask to its original value as soon + // as possible. Ideally we'd be able to detect if WithChmod() was passed as + // an option, and skip changing umask if default permissions are used. + origUmask := syscall.Umask(0777) l, err := net.Listen("unix", path) + syscall.Umask(origUmask) if err != nil { return nil, err }