From: Roy Marples Date: Sat, 4 May 2024 11:33:24 +0000 (+0100) Subject: privsep: Reduce fd use X-Git-Tag: v10.0.7~5 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=40c99e59aa919d0b3a633b311e806ebc39a65e65;p=thirdparty%2Fdhcpcd.git privsep: Reduce fd use 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. --- diff --git a/src/dhcp6.c b/src/dhcp6.c index 58ec4eb0..037cc242 100644 --- a/src/dhcp6.c +++ b/src/dhcp6.c @@ -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; diff --git a/src/dhcpcd.c b/src/dhcpcd.c index d9e07157..ccc8fbcf 100644 --- a/src/dhcpcd.c +++ b/src/dhcpcd.c @@ -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; @@ -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 @@ -2461,9 +2462,14 @@ printpidfile: 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) { @@ -2681,10 +2687,6 @@ exit1: 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 diff --git a/src/if-bsd.c b/src/if-bsd.c index bab33c2c..9ebe36a8 100644 --- a/src/if-bsd.c +++ b/src/if-bsd.c @@ -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) @@ -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 @@ -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; diff --git a/src/if-linux-wext.c b/src/if-linux-wext.c index 40498571..88545dc9 100644 --- a/src/if-linux-wext.c +++ b/src/if-linux-wext.c @@ -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)); diff --git a/src/if-linux.c b/src/if-linux.c index e1825821..2a599516 100644 --- a/src/if-linux.c +++ b/src/if-linux.c @@ -500,15 +500,22 @@ setup_priv: 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 diff --git a/src/if-options.c b/src/if-options.c index c57f6a31..f4f55204 100644 --- a/src/if-options.c +++ b/src/if-options.c @@ -2465,7 +2465,7 @@ 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 | @@ -2473,7 +2473,7 @@ read_config(struct dhcpcd_ctx *ctx, } #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 | diff --git a/src/if-sun.c b/src/if-sun.c index 0a60368e..37d00961 100644 --- a/src/if-sun.c +++ b/src/if-sun.c @@ -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) { @@ -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 @@ -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) diff --git a/src/if.c b/src/if.c index 3446a9f7..3ed9f6b3 100644 --- a/src/if.c +++ b/src/if.c @@ -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 @@ -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 || @@ -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)) diff --git a/src/logerr.c b/src/logerr.c index 883b2b19..319d6f48 100644 --- a/src/logerr.c +++ b/src/logerr.c @@ -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); diff --git a/src/privsep-bpf.c b/src/privsep-bpf.c index 98940090..74286535 100644 --- a/src/privsep-bpf.c +++ b/src/privsep-bpf.c @@ -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) { @@ -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 diff --git a/src/privsep-control.c b/src/privsep-control.c index 40bfb164..5bce4174 100644 --- a/src/privsep-control.c +++ b/src/privsep-control.c @@ -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) { @@ -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 @@ -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, diff --git a/src/privsep-inet.c b/src/privsep-inet.c index 5895a573..919fe954 100644 --- a/src/privsep-inet.c +++ b/src/privsep-inet.c @@ -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) diff --git a/src/privsep-root.c b/src/privsep-root.c index eb2b3eef..ed4bb699 100644 --- a/src/privsep-root.c +++ b/src/privsep-root.c @@ -259,7 +259,7 @@ ps_root_doioctl(unsigned long req, void *data, size_t len) return -1; } - s = socket(PF_INET, SOCK_DGRAM, 0); + s = xsocket(PF_INET, SOCK_DGRAM, 0); if (s != -1) #ifdef IOCTL_REQUEST_TYPE { @@ -685,17 +685,6 @@ ps_root_startcb(struct ps_process *psp) if (if_opensockets(ctx) == -1) logerr("%s: if_opensockets", __func__); -#ifdef BSD - else { - /* 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 /* Open network sockets for sending. * This is a small bit wasteful for non sandboxed OS's @@ -899,7 +888,7 @@ ps_root_start(struct dhcpcd_ctx *ctx) return -1; #endif - if (socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CXNB, 0, datafd) == -1) + if (xsocketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CXNB, 0, datafd) == -1) return -1; if (ps_setbuf_fdpair(datafd) == -1) return -1; @@ -937,6 +926,32 @@ ps_root_start(struct dhcpcd_ctx *ctx) return pid; } +void +ps_root_close(struct dhcpcd_ctx *ctx) +{ + + if_closesockets(ctx); + +#ifdef INET + if (ctx->udp_wfd != -1) { + close(ctx->udp_wfd); + ctx->udp_wfd = -1; + } +#endif +#ifdef INET6 + if (ctx->nd_fd != -1) { + close(ctx->nd_fd); + ctx->nd_fd = -1; + } +#endif +#ifdef DHCP6 + if (ctx->dhcp6_wfd != -1) { + close(ctx->dhcp6_wfd); + ctx->dhcp6_wfd = -1; + } +#endif +} + int ps_root_stop(struct dhcpcd_ctx *ctx) { diff --git a/src/privsep-root.h b/src/privsep-root.h index ccb2f485..ce824db9 100644 --- a/src/privsep-root.h +++ b/src/privsep-root.h @@ -36,6 +36,7 @@ #endif pid_t ps_root_start(struct dhcpcd_ctx *ctx); +void ps_root_close(struct dhcpcd_ctx *ctx); int ps_root_stop(struct dhcpcd_ctx *ctx); void ps_root_signalcb(int, void *); diff --git a/src/privsep-sun.c b/src/privsep-sun.c index a645d15d..6e539c19 100644 --- a/src/privsep-sun.c +++ b/src/privsep-sun.c @@ -45,7 +45,7 @@ ps_root_doioctl6(unsigned long req, void *data, size_t len) { int s, err; - s = socket(PF_INET6, SOCK_DGRAM, 0); + s = xsocket(PF_INET6, SOCK_DGRAM, 0); if (s != -1) err = ioctl(s, req, data, len); else @@ -63,7 +63,7 @@ ps_root_doroute(void *data, size_t len) int s; ssize_t err; - s = socket(PF_ROUTE, SOCK_RAW, 0); + s = xsocket(PF_ROUTE, SOCK_RAW, 0); if (s != -1) err = write(s, data, len); else diff --git a/src/privsep.c b/src/privsep.c index 4cca12ee..4cccfbdb 100644 --- a/src/privsep.c +++ b/src/privsep.c @@ -408,15 +408,23 @@ ps_startprocess(struct ps_process *psp, return pid; } + /* If we are not the root process, close un-needed stuff. */ + if (ctx->ps_root != psp) { + ps_root_close(ctx); #ifdef PLUGIN_DEV - /* If we are not the root process, stop listening to devices. */ - if (ctx->ps_root != psp) dev_stop(ctx); #endif + } ctx->options |= DHCPCD_FORKED; if (ctx->ps_log_fd != -1) logsetfd(ctx->ps_log_fd); + +#ifdef DEBUG_FD + logerrx("pid %d log_fd=%d data_fd=%d psp_fd=%d", + getpid(), ctx->ps_log_fd, ctx->ps_data_fd, psp->psp_fd); +#endif + eloop_clear(ctx->eloop, -1); eloop_forked(ctx->eloop); eloop_signal_set_cb(ctx->eloop, @@ -459,18 +467,6 @@ ps_startprocess(struct ps_process *psp, #endif } - if (ctx->ps_inet != psp) - ctx->ps_inet = NULL; - if (ctx->ps_ctl != psp) - ctx->ps_ctl = NULL; - -#if 0 - char buf[1024]; - errno = 0; - ssize_t xx = recv(psp->psp_fd, buf, sizeof(buf), MSG_PEEK); - logerr("pid %d test fd %d recv peek %zd", getpid(), psp->psp_fd, xx); -#endif - if (eloop_event_add(ctx->eloop, psp->psp_fd, ELE_READ, recv_msg, psp) == -1) { @@ -1215,6 +1211,7 @@ ps_newprocess(struct dhcpcd_ctx *ctx, struct ps_id *psid) return NULL; psp->psp_ctx = ctx; memcpy(&psp->psp_id, psid, sizeof(psp->psp_id)); + psp->psp_fd = -1; psp->psp_work_fd = -1; #ifdef HAVE_CAPSICUM psp->psp_pfd = -1;