NewUnixSocketWithOpts(): reduce umask override time-window, document hack

This hack was originally added in moby/moby@24c73ce,
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 <[email protected]>

Author: thaJeztah

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
 	}