From 0a12ab22a6abe691f11bb59efdb9b9a35dd6467b Mon Sep 17 00:00:00 2001 From: Ayush Sharma Date: Sat, 24 Aug 2024 00:22:08 +0530 Subject: [PATCH] Add support for setting the group on a unix domain socket (#901) Add new optional, immutable string config called `unixsocketgroup`. Change the group of the unix socket to `unixsocketgroup` after `bind()` if specified. Adds tests to validate the behavior. Fixes #873. Signed-off-by: Ayush Sharma --- src/anet.c | 25 +++++++++++++++++++++---- src/anet.h | 2 +- src/config.c | 1 + src/connection.h | 3 ++- src/rdma.c | 8 ++++---- src/server.c | 3 ++- src/server.h | 1 + src/unix.c | 5 +++-- tests/unit/introspection.tcl | 1 + tests/unit/other.tcl | 23 +++++++++++++++++++++++ valkey.conf | 1 + 11 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/anet.c b/src/anet.c index 36dd1b411..d4ac69898 100644 --- a/src/anet.c +++ b/src/anet.c @@ -45,6 +45,7 @@ #include #include #include +#include #include "anet.h" #include "config.h" @@ -505,7 +506,7 @@ int anetTcpNonBlockBestEffortBindConnect(char *err, const char *addr, int port, return anetTcpGenericConnect(err, addr, port, source_addr, ANET_CONNECT_NONBLOCK | ANET_CONNECT_BE_BINDING); } -static int anetListen(char *err, int s, struct sockaddr *sa, socklen_t len, int backlog, mode_t perm) { +static int anetListen(char *err, int s, struct sockaddr *sa, socklen_t len, int backlog, mode_t perm, char *group) { if (bind(s, sa, len) == -1) { anetSetError(err, "bind: %s", strerror(errno)); close(s); @@ -514,6 +515,22 @@ static int anetListen(char *err, int s, struct sockaddr *sa, socklen_t len, int if (sa->sa_family == AF_LOCAL && perm) chmod(((struct sockaddr_un *)sa)->sun_path, perm); + if (sa->sa_family == AF_LOCAL && group != NULL) { + struct group *grp; + if ((grp = getgrnam(group)) == NULL) { + anetSetError(err, "getgrnam error for group '%s': %s", group, strerror(errno)); + close(s); + return ANET_ERR; + } + + /* Owner of the socket remains same. */ + if (chown(((struct sockaddr_un *)sa)->sun_path, -1, grp->gr_gid) == -1) { + anetSetError(err, "chown error for group '%s': %s", group, strerror(errno)); + close(s); + return ANET_ERR; + } + } + if (listen(s, backlog) == -1) { anetSetError(err, "listen: %s", strerror(errno)); close(s); @@ -553,7 +570,7 @@ static int _anetTcpServer(char *err, int port, char *bindaddr, int af, int backl if (af == AF_INET6 && anetV6Only(err, s) == ANET_ERR) goto error; if (anetSetReuseAddr(err, s) == ANET_ERR) goto error; - if (anetListen(err, s, p->ai_addr, p->ai_addrlen, backlog, 0) == ANET_ERR) s = ANET_ERR; + if (anetListen(err, s, p->ai_addr, p->ai_addrlen, backlog, 0, NULL) == ANET_ERR) s = ANET_ERR; goto end; } if (p == NULL) { @@ -577,7 +594,7 @@ int anetTcp6Server(char *err, int port, char *bindaddr, int backlog) { return _anetTcpServer(err, port, bindaddr, AF_INET6, backlog); } -int anetUnixServer(char *err, char *path, mode_t perm, int backlog) { +int anetUnixServer(char *err, char *path, mode_t perm, int backlog, char *group) { int s; struct sockaddr_un sa; @@ -593,7 +610,7 @@ int anetUnixServer(char *err, char *path, mode_t perm, int backlog) { memset(&sa, 0, sizeof(sa)); sa.sun_family = AF_LOCAL; valkey_strlcpy(sa.sun_path, path, sizeof(sa.sun_path)); - if (anetListen(err, s, (struct sockaddr *)&sa, sizeof(sa), backlog, perm) == ANET_ERR) return ANET_ERR; + if (anetListen(err, s, (struct sockaddr *)&sa, sizeof(sa), backlog, perm, group) == ANET_ERR) return ANET_ERR; return s; } diff --git a/src/anet.h b/src/anet.h index a230eabbf..ab32f72e4 100644 --- a/src/anet.h +++ b/src/anet.h @@ -56,7 +56,7 @@ int anetTcpNonBlockBestEffortBindConnect(char *err, const char *addr, int port, int anetResolve(char *err, char *host, char *ipbuf, size_t ipbuf_len, int flags); int anetTcpServer(char *err, int port, char *bindaddr, int backlog); int anetTcp6Server(char *err, int port, char *bindaddr, int backlog); -int anetUnixServer(char *err, char *path, mode_t perm, int backlog); +int anetUnixServer(char *err, char *path, mode_t perm, int backlog, char *group); int anetTcpAccept(char *err, int serversock, char *ip, size_t ip_len, int *port); int anetUnixAccept(char *err, int serversock); int anetNonBlock(char *err, int fd); diff --git a/src/config.c b/src/config.c index b77e6065e..f8e0bffda 100644 --- a/src/config.c +++ b/src/config.c @@ -3110,6 +3110,7 @@ standardConfig static_configs[] = { /* String Configs */ createStringConfig("aclfile", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.acl_filename, "", NULL, NULL), createStringConfig("unixsocket", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.unixsocket, NULL, NULL, NULL), + createStringConfig("unixsocketgroup", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.unixsocketgroup, NULL, NULL, NULL), createStringConfig("pidfile", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.pidfile, NULL, NULL, NULL), createStringConfig("replica-announce-ip", "slave-announce-ip", MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.replica_announce_ip, NULL, NULL, NULL), createStringConfig("primaryuser", "masteruser", MODIFIABLE_CONFIG | SENSITIVE_CONFIG, EMPTY_STRING_IS_NULL, server.primary_user, NULL, NULL, NULL), diff --git a/src/connection.h b/src/connection.h index 076244173..97d79e565 100644 --- a/src/connection.h +++ b/src/connection.h @@ -144,7 +144,8 @@ struct connListener { int bindaddr_count; int port; ConnectionType *ct; - void *priv; /* used by connection type specified data */ + void *priv1; /* used by connection type specified data */ + void *priv2; /* used by connection type specified data */ }; /* The connection module does not deal with listening and accepting sockets, diff --git a/src/rdma.c b/src/rdma.c index b7b73fa18..bca01b983 100644 --- a/src/rdma.c +++ b/src/rdma.c @@ -748,7 +748,7 @@ static rdma_listener *rdmaFdToListener(connListener *listener, int fd) { for (int i = 0; i < listener->count; i++) { if (listener->fd[i] != fd) continue; - return (rdma_listener *)listener->priv + i; + return (rdma_listener *)listener->priv1 + i; } return NULL; @@ -1517,7 +1517,7 @@ int connRdmaListen(connListener *listener) { bindaddr = default_bindaddr; } - listener->priv = rdma_listener = zcalloc_num(bindaddr_count, sizeof(*rdma_listener)); + listener->priv1 = rdma_listener = zcalloc_num(bindaddr_count, sizeof(*rdma_listener)); for (j = 0; j < bindaddr_count; j++) { char *addr = bindaddr[j]; int optional = *addr == '-'; @@ -1736,13 +1736,13 @@ static int rdmaChangeListener(void) { aeDeleteFileEvent(server.el, listener->fd[i], AE_READABLE); listener->fd[i] = -1; - struct rdma_listener *rdma_listener = (struct rdma_listener *)listener->priv + i; + struct rdma_listener *rdma_listener = (struct rdma_listener *)listener->priv1 + i; rdma_destroy_id(rdma_listener->cm_id); rdma_destroy_event_channel(rdma_listener->cm_channel); } listener->count = 0; - zfree(listener->priv); + zfree(listener->priv1); closeListener(listener); diff --git a/src/server.c b/src/server.c index 91cd5980d..2cd40d291 100644 --- a/src/server.c +++ b/src/server.c @@ -2787,7 +2787,8 @@ void initListeners(void) { listener->bindaddr = &server.unixsocket; listener->bindaddr_count = 1; listener->ct = connectionByType(CONN_TYPE_UNIX); - listener->priv = &server.unixsocketperm; /* Unix socket specified */ + listener->priv1 = &server.unixsocketperm; /* Unix socket specified */ + listener->priv2 = server.unixsocketgroup; /* Unix socket group specified */ } /* create all the configured listener, and add handler to start to accept */ diff --git a/src/server.h b/src/server.h index c4c6e1fc4..c08dd5887 100644 --- a/src/server.h +++ b/src/server.h @@ -1712,6 +1712,7 @@ struct valkeyServer { int bindaddr_count; /* Number of addresses in server.bindaddr[] */ char *bind_source_addr; /* Source address to bind on for outgoing connections */ char *unixsocket; /* UNIX socket path */ + char *unixsocketgroup; /* UNIX socket group */ unsigned int unixsocketperm; /* UNIX socket permission (see mode_t) */ connListener listeners[CONN_TYPE_MAX]; /* TCP/Unix/TLS even more types */ uint32_t socket_mark_id; /* ID for listen socket marking */ diff --git a/src/unix.c b/src/unix.c index 795b2db9f..ddfd73465 100644 --- a/src/unix.c +++ b/src/unix.c @@ -51,7 +51,8 @@ static int connUnixIsLocal(connection *conn) { static int connUnixListen(connListener *listener) { int fd; - mode_t *perm = (mode_t *)listener->priv; + mode_t *perm = (mode_t *)listener->priv1; + char *group = (char *)listener->priv2; if (listener->bindaddr_count == 0) return C_OK; @@ -61,7 +62,7 @@ static int connUnixListen(connListener *listener) { char *addr = listener->bindaddr[j]; unlink(addr); /* don't care if this fails */ - fd = anetUnixServer(server.neterr, addr, *perm, server.tcp_backlog); + fd = anetUnixServer(server.neterr, addr, *perm, server.tcp_backlog, group); if (fd == ANET_ERR) { serverLog(LL_WARNING, "Failed opening Unix socket: %s", server.neterr); exit(1); diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl index 5396cd2e5..6d0e48e39 100644 --- a/tests/unit/introspection.tcl +++ b/tests/unit/introspection.tcl @@ -533,6 +533,7 @@ start_server {tags {"introspection"}} { io-threads logfile unixsocketperm + unixsocketgroup replicaof slaveof requirepass diff --git a/tests/unit/other.tcl b/tests/unit/other.tcl index 503ee391a..6e6230fc1 100644 --- a/tests/unit/other.tcl +++ b/tests/unit/other.tcl @@ -544,3 +544,26 @@ start_server {tags {"other external:skip"}} { } } } + +set tempFileName [file join [pwd] [pid]] +if {$::verbose} { + puts "Creating temp file $tempFileName" +} +set tempFileId [open $tempFileName w] +set group [dict get [file attributes $tempFileName] -group] +if {$group != ""} { + start_server [list tags {"repl external:skip"} overrides [list unixsocketgroup $group unixsocketperm 744]] { + test {test unixsocket options are set correctly} { + set socketpath [lindex [r config get unixsocket] 1] + set attributes [file attributes $socketpath] + set permissions [string range [dict get $attributes -permissions] end-2 end] + assert_equal [dict get $attributes -group] $group + assert_equal 744 $permissions + } + } +} +if {$::verbose} { + puts "Deleting temp file: $tempFileName" +} +close $tempFileId +file delete $tempFileName diff --git a/valkey.conf b/valkey.conf index 4b2da077f..8465facb5 100644 --- a/valkey.conf +++ b/valkey.conf @@ -154,6 +154,7 @@ tcp-backlog 511 # on a unix socket when not specified. # # unixsocket /run/valkey.sock +# unixsocketgroup wheel # unixsocketperm 700 # Close the connection after a client is idle for N seconds (0 to disable)