From 22927e278dc29c9d1804c20f483510ec569f23ed Mon Sep 17 00:00:00 2001 From: Emily Date: Fri, 23 Jun 2023 22:49:41 +0200 Subject: [PATCH] core: Add optional unix socket file permissions (#4741) * core: Add optional unix socket file permissions This commit also changes the default unix socket file permissions to `u=w,g=,o=` (octal: `0200`). It used to default to the shell's umask (usually `u=rwx,g=rx,o=rx`, octal: `0755`). `/run/caddy.sock` -> `/run/caddy.sock` with `0200` default perms `/run/caddy.sock|0222` -> `/run/caddy.sock` with `0222` perms `|` instead of `:` is used as a separator, to account for the `:` in Windows drive letters (e.g. `C:\absolute\path.sock`) Fun fact: The old unix(7) man page (pre Jun 2016) stated a socket needs both read and write perms. Turns out, only write perms are needed. Corrected in https://github.com/mkerrisk/man-pages/commit/7578ea2f85b272363d22680d69e7d32f0b59c83b Despite this, most implementations still default to read+write to this date. * Add cases with Windows paths to test * Require write perms for the owning user --- listeners.go | 64 +++++++++++++++++++++++++++++-- listeners_test.go | 95 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 3 deletions(-) diff --git a/listeners.go b/listeners.go index 1387d385b..cbce68d61 100644 --- a/listeners.go +++ b/listeners.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "io" + "io/fs" "net" "net/netip" "os" @@ -148,11 +149,27 @@ func (na NetworkAddress) Listen(ctx context.Context, portOffset uint, config net func (na NetworkAddress) listen(ctx context.Context, portOffset uint, config net.ListenConfig) (any, error) { var ln any var err error + var address string + var unixFileMode fs.FileMode - address := na.JoinHostPort(portOffset) + // split unix socket addr early so lnKey + // is independent of permissions bits + if na.IsUnixNetwork() { + var err error + address, unixFileMode, err = splitUnixSocketPermissionsBits(na.Host) + if err != nil { + return nil, err + } + } else { + address = na.JoinHostPort(portOffset) + } - // if this is a unix socket, see if we already have it open + // if this is a unix socket, see if we already have it open, + // force socket permissions on it and return early if socket, err := reuseUnixSocket(na.Network, address); socket != nil || err != nil { + if err := os.Chmod(address, unixFileMode); err != nil { + return nil, fmt.Errorf("unable to set permissions (%s) on %s: %v", unixFileMode, address, err) + } return socket, err } @@ -193,6 +210,12 @@ func (na NetworkAddress) listen(ctx context.Context, portOffset uint, config net unixSockets[lnKey] = unix } + if IsUnixNetwork(na.Network) { + if err := os.Chmod(address, unixFileMode); err != nil { + return nil, fmt.Errorf("unable to set permissions (%s) on %s: %v", unixFileMode, address, err) + } + } + return ln, nil } @@ -288,6 +311,40 @@ func IsUnixNetwork(netw string) bool { return strings.HasPrefix(netw, "unix") } +// Takes a unix socket address in the unusual "path|bits" format +// (e.g. /run/caddy.sock|0222) and tries to split it into +// socket path (host) and permissions bits (port). Colons (":") +// can't be used as separator, as socket paths on Windows may +// include a drive letter (e.g. `unix/c:\absolute\path.sock`). +// Permission bits will default to 0200 if none are specified. +// Throws an error, if the first carrying bit does not +// include write perms (e.g. `0422` or `022`). +// Symbolic permission representation (e.g. `u=w,g=w,o=w`) +// is not supported and will throw an error for now! +func splitUnixSocketPermissionsBits(addr string) (path string, fileMode fs.FileMode, err error) { + addrSplit := strings.SplitN(addr, "|", 2) + + if len(addrSplit) == 2 { + // parse octal permission bit string as uint32 + fileModeUInt64, err := strconv.ParseUint(addrSplit[1], 8, 32) + if err != nil { + return "", 0, fmt.Errorf("could not parse octal permission bits in %s: %v", addr, err) + } + fileMode = fs.FileMode(fileModeUInt64) + + // FileMode.String() returns a string like `-rwxr-xr--` for `u=rwx,g=rx,o=r` (`0754`) + if string(fileMode.String()[2]) != "w" { + return "", 0, fmt.Errorf("owner of the socket requires '-w-' (write, octal: '2') permissions at least; got '%s' in %s", fileMode.String()[1:4], addr) + } + + return addrSplit[0], fileMode, nil + } + + // default to 0200 (symbolic: `u=w,g=,o=`) + // if no permission bits are specified + return addr, 0200, nil +} + // ParseNetworkAddress parses addr into its individual // components. The input string is expected to be of // the form "network/host:port-range" where any part is @@ -312,10 +369,11 @@ func ParseNetworkAddressWithDefaults(addr, defaultNetwork string, defaultPort ui network = defaultNetwork } if IsUnixNetwork(network) { + _, _, err := splitUnixSocketPermissionsBits(host) return NetworkAddress{ Network: network, Host: host, - }, nil + }, err } var start, end uint64 if port == "" { diff --git a/listeners_test.go b/listeners_test.go index 5508a9f08..1b534563d 100644 --- a/listeners_test.go +++ b/listeners_test.go @@ -555,3 +555,98 @@ func TestExpand(t *testing.T) { } } } + +func TestSplitUnixSocketPermissionsBits(t *testing.T) { + for i, tc := range []struct { + input string + expectNetwork string + expectPath string + expectFileMode string + expectErr bool + }{ + { + input: "./foo.socket", + expectPath: "./foo.socket", + expectFileMode: "--w-------", + }, + { + input: `.\relative\path.socket`, + expectPath: `.\relative\path.socket`, + expectFileMode: "--w-------", + }, + { + // literal colon in resulting address + // and defaulting to 0200 bits + input: "./foo.socket:0666", + expectPath: "./foo.socket:0666", + expectFileMode: "--w-------", + }, + { + input: "./foo.socket|0220", + expectPath: "./foo.socket", + expectFileMode: "--w--w----", + }, + { + input: "/var/run/foo|222", + expectPath: "/var/run/foo", + expectFileMode: "--w--w--w-", + }, + { + input: "./foo.socket|0660", + expectPath: "./foo.socket", + expectFileMode: "-rw-rw----", + }, + { + input: "./foo.socket|0666", + expectPath: "./foo.socket", + expectFileMode: "-rw-rw-rw-", + }, + { + input: "/var/run/foo|666", + expectPath: "/var/run/foo", + expectFileMode: "-rw-rw-rw-", + }, + { + input: `c:\absolute\path.socket|220`, + expectPath: `c:\absolute\path.socket`, + expectFileMode: "--w--w----", + }, + { + // symbolic permission representation is not supported for now + input: "./foo.socket|u=rw,g=rw,o=rw", + expectErr: true, + }, + { + // octal (base-8) permission representation has to be between + // `0` for no read, no write, no exec (`---`) and + // `7` for read (4), write (2), exec (1) (`rwx` => `4+2+1 = 7`) + input: "./foo.socket|888", + expectErr: true, + }, + { + // too many colons in address + input: "./foo.socket|123456|0660", + expectErr: true, + }, + { + // owner is missing write perms + input: "./foo.socket|0522", + expectErr: true, + }, + } { + actualPath, actualFileMode, err := splitUnixSocketPermissionsBits(tc.input) + if tc.expectErr && err == nil { + t.Errorf("Test %d: Expected error but got: %v", i, err) + } + if !tc.expectErr && err != nil { + t.Errorf("Test %d: Expected no error but got: %v", i, err) + } + if actualPath != tc.expectPath { + t.Errorf("Test %d: Expected path '%s' but got '%s'", i, tc.expectPath, actualPath) + } + // fileMode.Perm().String() parses 0 to "----------" + if !tc.expectErr && actualFileMode.Perm().String() != tc.expectFileMode { + t.Errorf("Test %d: Expected perms '%s' but got '%s'", i, tc.expectFileMode, actualFileMode.Perm().String()) + } + } +}