Merge pull request #75 from thaJeztah/move_umask

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

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
 	}