Skip to content

Commit

Permalink
Add support for setting the group on a unix domain socket (#901)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
ayush933 authored Aug 23, 2024
1 parent 829aa7f commit b48596a
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 13 deletions.
25 changes: 21 additions & 4 deletions src/anet.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include <errno.h>
#include <stdarg.h>
#include <stdio.h>
#include <grp.h>

#include "anet.h"
#include "config.h"
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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;

Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/anet.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
3 changes: 2 additions & 1 deletion src/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions src/rdma.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 == '-';
Expand Down Expand Up @@ -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);

Expand Down
3 changes: 2 additions & 1 deletion src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
1 change: 1 addition & 0 deletions src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
5 changes: 3 additions & 2 deletions src/unix.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions tests/unit/introspection.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ start_server {tags {"introspection"}} {
io-threads
logfile
unixsocketperm
unixsocketgroup
replicaof
slaveof
requirepass
Expand Down
23 changes: 23 additions & 0 deletions tests/unit/other.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions valkey.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit b48596a

Please sign in to comment.