Skip to content

Commit

Permalink
privsep: Reduce fd use
Browse files Browse the repository at this point in the history
On start close all FD's above stderr.
Close some fd's we don't need in processes spawned from priv.
Ensure we init some FD's to -1 to ensure we don't close stdin.
If DEBUG_FD is defined, we log FD's opened by pid.
Audit process FD usage and document it so I don't forget it.

Fixes #316.
  • Loading branch information
rsmarples committed May 4, 2024
1 parent 6a6c13f commit 40c99e5
Show file tree
Hide file tree
Showing 16 changed files with 136 additions and 72 deletions.
2 changes: 1 addition & 1 deletion src/dhcp6.c
Original file line number Diff line number Diff line change
Expand Up @@ -3791,7 +3791,7 @@ dhcp6_openraw(void)
{
int fd, v;

fd = socket(PF_INET6, SOCK_RAW | SOCK_CXNB, IPPROTO_UDP);
fd = xsocket(PF_INET6, SOCK_RAW | SOCK_CXNB, IPPROTO_UDP);
if (fd == -1)
return -1;

Expand Down
12 changes: 7 additions & 5 deletions src/dhcpcd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1981,6 +1981,7 @@ main(int argc, char **argv, char **envp)
}

memset(&ctx, 0, sizeof(ctx));
closefrom(STDERR_FILENO + 1);

ifo = NULL;
ctx.cffile = CONFIG;
Expand Down Expand Up @@ -2010,7 +2011,7 @@ main(int argc, char **argv, char **envp)
ctx.dhcp6_wfd = -1;
#endif
#ifdef PRIVSEP
ctx.ps_log_fd = -1;
ctx.ps_log_fd = ctx.ps_log_root_fd = -1;
TAILQ_INIT(&ctx.ps_processes);
#endif

Expand Down Expand Up @@ -2461,9 +2462,14 @@ main(int argc, char **argv, char **envp)
goto run_loop;
}

#ifdef DEBUG_FD
loginfox("forkfd %d", ctx.fork_fd);
#endif

/* We have now forked, setsid, forked once more.
* From this point on, we are the controlling daemon. */
logdebugx("spawned manager process on PID %d", getpid());

start_manager:
ctx.options |= DHCPCD_STARTED;
if ((pid = pidfile_lock(ctx.pidfile)) != 0) {
Expand Down Expand Up @@ -2681,10 +2687,6 @@ main(int argc, char **argv, char **envp)
free(ctx.script_env);
rt_dispose(&ctx);
free(ctx.duid);
if (ctx.link_fd != -1) {
eloop_event_delete(ctx.eloop, ctx.link_fd);
close(ctx.link_fd);
}
if_closesockets(&ctx);
free_globals(&ctx);
#ifdef INET6
Expand Down
26 changes: 22 additions & 4 deletions src/if-bsd.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,18 @@ if_opensockets_os(struct dhcpcd_ctx *ctx)
&n, sizeof(n)) == -1)
logerr("%s: SO_USELOOPBACK", __func__);

#ifdef PRIVSEP
if (ctx->options & DHCPCD_PRIVSEPROOT) {
/* We only want to write to this socket, so set
* a small as possible buffer size. */
socklen_t smallbuf = 1;

if (setsockopt(ctx->link_fd, SOL_SOCKET, SO_RCVBUF,
&smallbuf, (socklen_t)sizeof(smallbuf)) == -1)
logerr("%s: setsockopt(SO_RCVBUF)", __func__);
}
#endif

#if defined(RO_MSGFILTER)
if (setsockopt(ctx->link_fd, PF_ROUTE, RO_MSGFILTER,
&msgfilter, sizeof(msgfilter)) == -1)
Expand All @@ -220,9 +232,8 @@ if_opensockets_os(struct dhcpcd_ctx *ctx)
ps_rights_limit_fd_sockopt(ctx->link_fd);
#endif


#if defined(SIOCALIFADDR) && defined(IFLR_ACTIVE) /*NetBSD */
priv->pf_link_fd = socket(PF_LINK, SOCK_DGRAM, 0);
priv->pf_link_fd = xsocket(PF_LINK, SOCK_DGRAM, 0);
if (priv->pf_link_fd == -1)
logerr("%s: socket(PF_LINK)", __func__);
#endif
Expand All @@ -235,13 +246,20 @@ if_closesockets_os(struct dhcpcd_ctx *ctx)
struct priv *priv;

priv = (struct priv *)ctx->priv;
if (priv == NULL)
return;

#ifdef INET6
if (priv->pf_inet6_fd != -1)
if (priv->pf_inet6_fd != -1) {
close(priv->pf_inet6_fd);
priv->pf_inet6_fd = -1;
}
#endif
#if defined(SIOCALIFADDR) && defined(IFLR_ACTIVE) /*NetBSD */
if (priv->pf_link_fd != -1)
if (priv->pf_link_fd != -1) {
close(priv->pf_link_fd);
priv->pf_link_fd = -1;
}
#endif
free(priv);
ctx->priv = NULL;
Expand Down
2 changes: 1 addition & 1 deletion src/if-linux-wext.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ if_getssid_wext(const char *ifname, uint8_t *ssid)
int s, retval;
struct iwreq iwr;

if ((s = socket(PF_INET, SOCK_DGRAM, 0)) == -1)
if ((s = xsocket(PF_INET, SOCK_DGRAM, 0)) == -1)
return -1;
memset(&iwr, 0, sizeof(iwr));
strlcpy(iwr.ifr_name, ifname, sizeof(iwr.ifr_name));
Expand Down
21 changes: 14 additions & 7 deletions src/if-linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -500,15 +500,22 @@ if_opensockets_os(struct dhcpcd_ctx *ctx)
void
if_closesockets_os(struct dhcpcd_ctx *ctx)
{
struct priv *priv;
struct priv *priv = ctx->priv;

if (priv == NULL)
return;

if (ctx->priv != NULL) {
priv = (struct priv *)ctx->priv;
if (priv->route_fd != -1)
close(priv->route_fd);
if (priv->generic_fd != -1)
close(priv->generic_fd);
if (priv->route_fd != -1) {
close(priv->route_fd);
priv->route_fd = -1;
}
if (priv->generic_fd != -1) {
close(priv->generic_fd);
priv->generic_fd = -1;
}

free(priv);
ctx->priv = NULL;
}

int
Expand Down
4 changes: 2 additions & 2 deletions src/if-options.c
Original file line number Diff line number Diff line change
Expand Up @@ -2465,15 +2465,15 @@ read_config(struct dhcpcd_ctx *ctx,
default_options |= DHCPCD_CONFIGURE | DHCPCD_DAEMONISE |
DHCPCD_GATEWAY;
#ifdef INET
skip = socket(PF_INET, SOCK_DGRAM, 0);
skip = xsocket(PF_INET, SOCK_DGRAM, 0);
if (skip != -1) {
close(skip);
default_options |= DHCPCD_IPV4 | DHCPCD_ARP |
DHCPCD_DHCP | DHCPCD_IPV4LL;
}
#endif
#ifdef INET6
skip = socket(PF_INET6, SOCK_DGRAM, 0);
skip = xsocket(PF_INET6, SOCK_DGRAM, 0);
if (skip != -1) {
close(skip);
default_options |= DHCPCD_IPV6 | DHCPCD_IPV6RS |
Expand Down
7 changes: 4 additions & 3 deletions src/if-sun.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ if_opensockets_os(struct dhcpcd_ctx *ctx)
* We will fail noisily elsewhere anyway. */
#endif

ctx->link_fd = socket(PF_ROUTE,
ctx->link_fd = xsocket(PF_ROUTE,
SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);

if (ctx->link_fd == -1) {
Expand All @@ -180,12 +180,13 @@ if_closesockets_os(struct dhcpcd_ctx *ctx)
struct priv *priv;

priv = (struct priv *)ctx->priv;
if (priv->pf_inet6_fd != -1)
if (priv && priv->pf_inet6_fd != -1)
close(priv->pf_inet6_fd);
#endif

/* each interface should have closed itself */
free(ctx->priv);
ctx->priv = NULL;
}

int
Expand Down Expand Up @@ -727,7 +728,7 @@ if_route_get(struct dhcpcd_ctx *ctx, struct rt *rt)

if_route0(ctx, &rtm, RTM_GET, rt);
rt = NULL;
s = socket(PF_ROUTE, SOCK_RAW | SOCK_CLOEXEC, 0);
s = xsocket(PF_ROUTE, SOCK_RAW | SOCK_CLOEXEC, 0);
if (s == -1)
return NULL;
if (write(s, &rtm, rtm.hdr.rtm_msglen) == -1)
Expand Down
27 changes: 18 additions & 9 deletions src/if.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,18 @@ void
if_closesockets(struct dhcpcd_ctx *ctx)
{

if (ctx->pf_inet_fd != -1)
close(ctx->pf_inet_fd);
#ifdef PF_LINK
if (ctx->pf_link_fd != -1)
close(ctx->pf_link_fd);
#endif
if (ctx->link_fd != -1) {
eloop_event_delete(ctx->eloop, ctx->link_fd);
close(ctx->link_fd);
ctx->link_fd = -1;
}

if (ctx->priv) {
if_closesockets_os(ctx);
free(ctx->priv);
if (ctx->pf_inet_fd != -1) {
close(ctx->pf_inet_fd);
ctx->pf_inet_fd = -1;
}

if_closesockets_os(ctx);
}

int
Expand Down Expand Up @@ -982,6 +983,10 @@ xsocket(int domain, int type, int protocol)

if ((s = socket(domain, type, protocol)) == -1)
return -1;
#ifdef DEBUG_FD
logerrx("pid %d fd=%d domain=%d type=%d protocol=%d",
getpid(), s, domain, type, protocol);
#endif

#ifndef HAVE_SOCK_CLOEXEC
if ((xtype & SOCK_CLOEXEC) && ((xflags = fcntl(s, F_GETFD)) == -1 ||
Expand Down Expand Up @@ -1023,6 +1028,10 @@ xsocketpair(int domain, int type, int protocol, int fd[2])
if ((s = socketpair(domain, type, protocol, fd)) == -1)
return -1;

#ifdef DEBUG_FD
logerrx("pid %d fd[0]=%d fd[1]=%d", getpid(), fd[0], fd[1]);
#endif

#ifndef HAVE_SOCK_CLOEXEC
if ((xtype & SOCK_CLOEXEC) && ((xflags = fcntl(fd[0], F_GETFD)) == -1 ||
fcntl(fd[0], F_SETFD, xflags | FD_CLOEXEC) == -1))
Expand Down
2 changes: 2 additions & 0 deletions src/logerr.c
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,8 @@ logsetfd(int fd)
struct logctx *ctx = &_logctx;

ctx->log_fd = fd;
if (fd != -1)
closelog();
#ifndef SMALL
if (fd != -1 && ctx->log_file != NULL) {
fclose(ctx->log_file);
Expand Down
5 changes: 5 additions & 0 deletions src/privsep-bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
#include "logerr.h"
#include "privsep.h"

/* We expect to have open 3 SEQPACKET and one RAW fd */

static void
ps_bpf_recvbpf(void *arg, unsigned short events)
{
Expand Down Expand Up @@ -160,6 +162,9 @@ ps_bpf_start_bpf(struct ps_process *psp)
ps_freeprocesses(ctx, psp);

psp->psp_bpf = bpf_open(&psp->psp_ifp, psp->psp_filter, ia);
#ifdef DEBUG_FD
logdebugx("pid %d bpf_fd=%d", getpid(), psp->psp_bpf->bpf_fd);
#endif
if (psp->psp_bpf == NULL)
logerr("%s: bpf_open",__func__);
#ifdef PRIVSEP_RIGHTS
Expand Down
27 changes: 16 additions & 11 deletions src/privsep-control.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
#include "logerr.h"
#include "privsep.h"

/* We expect to have open 2 SEQPACKET, 2 STREAM and 2 file STREAM fds */

static int
ps_ctl_startcb(struct ps_process *psp)
{
Expand Down Expand Up @@ -217,14 +219,16 @@ ps_ctl_start(struct dhcpcd_ctx *ctx)
.psi_cmd = PS_CTL,
};
struct ps_process *psp;
int data_fd[2], listen_fd[2];
int work_fd[2], listen_fd[2];
pid_t pid;

if (xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CXNB, 0, data_fd) == -1 ||
if_closesockets(ctx);

if (xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CXNB, 0, work_fd) == -1 ||
xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CXNB, 0, listen_fd) == -1)
return -1;
#ifdef PRIVSEP_RIGHTS
if (ps_rights_limit_fdpair(data_fd) == -1 ||
if (ps_rights_limit_fdpair(work_fd) == -1 ||
ps_rights_limit_fdpair(listen_fd) == -1)
return -1;
#endif
Expand All @@ -237,24 +241,25 @@ ps_ctl_start(struct dhcpcd_ctx *ctx)
if (pid == -1)
return -1;
else if (pid != 0) {
psp->psp_work_fd = data_fd[1];
close(data_fd[0]);
psp->psp_work_fd = work_fd[0];
close(work_fd[1]);
close(listen_fd[1]);
ctx->ps_control = control_new(ctx,
listen_fd[1], FD_SENDLEN | FD_LISTEN);
listen_fd[0], FD_SENDLEN | FD_LISTEN);
if (ctx->ps_control == NULL)
return -1;
close(listen_fd[0]);
return pid;
}

psp->psp_work_fd = data_fd[0];
close(data_fd[1]);
close(work_fd[0]);
close(listen_fd[0]);

psp->psp_work_fd = work_fd[1];
if (eloop_event_add(ctx->eloop, psp->psp_work_fd, ELE_READ,
ps_ctl_recv, ctx) == -1)
return -1;

ctx->ps_control = control_new(ctx, listen_fd[0], 0);
close(listen_fd[1]);
ctx->ps_control = control_new(ctx, listen_fd[1], 0);
if (ctx->ps_control == NULL)
return -1;
if (eloop_event_add(ctx->eloop, ctx->ps_control->fd, ELE_READ,
Expand Down
2 changes: 2 additions & 0 deletions src/privsep-inet.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
#include "logerr.h"
#include "privsep.h"

/* We expect to have open 2 SEQPACKET, 1 udp, 1 udp6 and 1 raw6 fds */

#ifdef INET
static void
ps_inet_recvbootp(void *arg, unsigned short events)
Expand Down

0 comments on commit 40c99e5

Please sign in to comment.