On Mon, Jul 10, 2017 at 01:21:58PM -0400, Rob Pierce wrote:
> The following diff is loosely based on the approach that was taken for
> pledging mountd. Other code/approaches leveraged from various networking
> daemons.
>
> This first step moves the ioctl with SIOCGIFDATA call to a privileged
> child so we can at least pledge "stdio rpath dns inet proc exec" without
> (intentionally) changing existing behaviour.
>
> Comments and suggestions welcome.
>
> Thanks!
>
> Rob
An unnecessary call to log_info snuck in. Here is a clean diff.
Rob
Index: Makefile
===================================================================
RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- Makefile 20 Jul 2010 02:05:15 -0000 1.10
+++ Makefile 14 Jul 2017 01:12:58 -0000
@@ -8,7 +8,7 @@ CFLAGS+= -Wmissing-declarations
CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual
YFLAGS=
MAN= ifstated.8 ifstated.conf.5
-LDADD+=-levent
+LDADD+=-levent -lutil
DPADD+= ${LIBEVENT}
.include <bsd.prog.mk>
Index: ifstated.c
===================================================================
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.50
diff -u -p -r1.50 ifstated.c
--- ifstated.c 4 Jul 2017 21:09:52 -0000 1.50
+++ ifstated.c 14 Jul 2017 01:12:58 -0000
@@ -25,13 +25,16 @@
#include <sys/types.h>
#include <sys/time.h>
#include <sys/ioctl.h>
+#include <sys/queue.h>
#include <sys/socket.h>
+#include <sys/uio.h>
#include <sys/wait.h>
#include <net/if.h>
#include <net/route.h>
#include <netinet/in.h>
+#include <poll.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -39,6 +42,7 @@
#include <stdint.h>
#include <syslog.h>
#include <err.h>
+#include <errno.h>
#include <event.h>
#include <unistd.h>
#include <ifaddrs.h>
@@ -47,7 +51,9 @@
#include "log.h"
struct ifsd_config *conf = NULL, *newconf = NULL;
+struct imsgbuf ibuf;
+pid_t privchild_pid;
int opts = 0;
int opt_inhibit = 0;
char *configfile = "/etc/ifstated.conf";
@@ -74,6 +80,7 @@ void do_action(struct ifsd_action *);
void remove_action(struct ifsd_action *, struct ifsd_state *);
void remove_expression(struct ifsd_expression *,
struct ifsd_state *);
+void privchild(int);
__dead void
usage(void)
@@ -89,6 +96,7 @@ int
main(int argc, char *argv[])
{
struct timeval tv;
+ int socks[2];
int ch, rt_fd;
int debug = 0;
unsigned int rtfilter;
@@ -143,6 +151,22 @@ main(int argc, char *argv[])
if (!debug)
daemon(1, 0);
+ if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, socks) == -1) {
+ fatal("%s: socketpair", __func__);
+ }
+
+ switch (privchild_pid = fork()) {
+ case -1:
+ fatal("%s: fork", __func__);
+ case 0:
+ close(socks[0]);
+ privchild(socks[1]);
+ }
+ close(socks[1]);
+
+ imsg_init(&ibuf, socks[0]);
+ setproctitle("parent");
+
event_init();
log_init(debug, LOG_DAEMON);
log_setverbose(opts & IFSD_OPT_VERBOSE);
@@ -160,6 +184,9 @@ main(int argc, char *argv[])
&rtfilter, sizeof(rtfilter)) == -1) /* not fatal */
log_warn("%s: setsockopt tablefilter", __func__);
+ if (pledge("stdio rpath dns inet proc exec", NULL) == -1)
+ fatal("pledge");
+
signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL);
signal_add(&sigchld_ev, NULL);
@@ -252,6 +279,16 @@ rt_msg_handler(int fd, short event, void
void
sigchld_handler(int fd, short event, void *arg)
{
+ pid_t pid;
+ int status;
+
+ do {
+ pid = waitpid(privchild_pid, &status, WNOHANG);
+ if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status)))
+ fatal("privchild %i exited due to receipt of signal %i",
+ privchild_pid, WTERMSIG(status));
+ } while (pid == -1 && errno == EINTR);
+
check_external_status(&conf->initstate);
if (conf->curstate != NULL)
check_external_status(conf->curstate);
@@ -599,29 +636,74 @@ do_action(struct ifsd_action *action)
void
fetch_ifstate(void)
{
+ struct imsg imsg;
struct ifaddrs *ifap, *ifa;
+ struct pollfd pfd[1];
char *oname = NULL;
int sock = socket(AF_INET, SOCK_DGRAM, 0);
+ int done = 0, state, rv;
+ ssize_t n;
if (getifaddrs(&ifap) != 0)
err(1, "getifaddrs");
for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
struct ifreq ifr;
- struct if_data ifrdat;
if (oname && !strcmp(oname, ifa->ifa_name))
continue;
oname = ifa->ifa_name;
strlcpy(ifr.ifr_name, ifa->ifa_name, sizeof(ifr.ifr_name));
- ifr.ifr_data = (caddr_t)&ifrdat;
-
- if (ioctl(sock, SIOCGIFDATA, (caddr_t)&ifr) == -1)
- continue;
- scan_ifstate(if_nametoindex(ifa->ifa_name),
- ifrdat.ifi_link_state, 0);
+ /*
+ * synchronous imsg request/response to privchild
+ */
+ if (imsg_compose(&ibuf, IMSG_IFSTATE_REQ, 0, 0, -1, &ifr,
+ sizeof(struct ifreq)) == -1)
+ fatal("%s: imsg_compose", __func__);
+ do {
+ rv = imsg_flush(&ibuf);
+ } while (rv == -1 && errno == EAGAIN);
+ if (rv == -1)
+ fatal("%s: imsg_flush", __func__);
+
+ pfd[0].fd = ibuf.fd;
+ pfd[0].events = POLLIN;
+ while (!done) {
+ switch (poll(pfd, 1, IFSTATE_REQ_TIMEOUT)) {
+ case -1:
+ fatal("%s: poll", __func__);
+ case 0:
+ log_warnx("%s: poll timeout", __func__);
+ break;
+ default:
+ break;
+ }
+ if ((n = imsg_read(&ibuf)) == -1 && errno != EAGAIN)
+ fatal("%s: imsg_read", __func__);
+ if (n == 0)
+ fatalx("%s: socket disconnected", __func__);
+ while (!done) {
+ if ((n = imsg_get(&ibuf, &imsg)) == -1)
+ fatal("%s: imsg_get", __func__);
+ if (n == 0) {
+ log_warnx("%s: no message ready",
+ __func__);
+ break;
+ }
+ if (imsg.hdr.type != IMSG_IFSTATE_RES)
+ fatalx("%s: invalid response",
+ __func__);
+ IMSG_SIZE_CHECK(&imsg, &state);
+ memcpy(&state, imsg.data,
+ sizeof(int));
+ scan_ifstate(if_nametoindex(
+ ifa->ifa_name), state, 0);
+ done = 1;
+ imsg_free(&imsg);
+ }
+ }
}
freeifaddrs(ifap);
close(sock);
@@ -710,4 +792,84 @@ remove_expression(struct ifsd_expression
break;
}
free(expression);
+}
+
+void
+privchild(int sock)
+{
+ struct pollfd pfd[1];
+ struct imsg imsg;
+ struct ifreq ifr;
+ struct if_data ifrdat;
+ int sockfd = socket(AF_INET, SOCK_DGRAM, 0);
+ int n, rv, state;
+
+ setproctitle("[priv]");
+ imsg_init(&ibuf, sock);
+
+ signal(SIGHUP, SIG_IGN);
+
+ for (;;) {
+ pfd[0].fd = ibuf.fd;
+ pfd[0].events = POLLIN;
+ switch (poll(pfd, 1, INFTIM)) {
+ case -1:
+ if (errno == EINTR)
+ continue;
+ fatal("%s: poll", __func__);
+ case 0:
+ log_warnx("%s: poll timeout", __func__);
+ break;
+ default:
+ break;
+ }
+ if (pfd[0].revents & POLLHUP) {
+ fatalx("%s: socket disconnected", __func__);
+ }
+ if (!(pfd[0].revents & POLLIN))
+ continue;
+
+ if ((n = imsg_read(&ibuf)) == -1 && errno != EAGAIN)
+ fatal("%s: imsg_read", __func__);
+ if (n == 0)
+ fatalx("%s: socket disconnected", __func__);
+
+ while ((n = imsg_get(&ibuf, &imsg)) != 0) {
+ if (n == -1) {
+ fatal("%s: imsg_get", __func__);
+ }
+ if (n == 0) {
+ log_warnx("%s: no message ready", __func__);
+ break;
+ }
+ switch (imsg.hdr.type) {
+ case IMSG_IFSTATE_REQ:
+ IMSG_SIZE_CHECK(&imsg, &ifr);
+ memcpy(&ifr, imsg.data, sizeof(struct ifreq
+ *));
+ ifr.ifr_data = (caddr_t)&ifrdat;
+
+ if (ioctl(sockfd, SIOCGIFDATA,
+ (caddr_t)&ifr) == -1) {
+ fatal("%s: ioctl", __func__);
+ }
+ state = (int)ifrdat.ifi_link_state;
+
+ if (imsg_compose(&ibuf, IMSG_IFSTATE_RES, 0,
+ 0, -1, &state, sizeof(int)) == -1)
+ fatal("%s: imsg_compose", __func__);
+ do {
+ rv = imsg_flush(&ibuf);
+ } while (rv == -1 && errno == EAGAIN);
+ if (rv == -1)
+ fatal("imsg_flush");
+ break;
+ default:
+ fatalx("%s: invalid message type", __func__);
+ }
+ imsg_free(&imsg);
+ }
+ }
+ close(sockfd);
+ close(sock);
}
Index: ifstated.h
===================================================================
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v
retrieving revision 1.16
diff -u -p -r1.16 ifstated.h
--- ifstated.h 4 Jul 2017 21:04:14 -0000 1.16
+++ ifstated.h 14 Jul 2017 01:12:58 -0000
@@ -28,6 +28,7 @@
#include <sys/types.h>
#include <sys/queue.h>
+#include <imsg.h>
struct ifsd_expression;
TAILQ_HEAD(ifsd_expression_list, ifsd_expression);
@@ -126,6 +127,19 @@ struct ifsd_config {
#define IFSD_OPT_VERBOSE2 0x00000002
#define IFSD_OPT_NOACTION 0x00000004
int maxdepth;
+};
+
+#define IMSG_SIZE_CHECK(imsg, p) do { \
+ if (IMSG_DATA_SIZE(imsg) < sizeof(*p)) \
+ fatalx("bad length imsg received"); \
+} while (0)
+#define IMSG_DATA_SIZE(imsg) ((imsg)->hdr.len - IMSG_HEADER_SIZE)
+
+#define IFSTATE_REQ_TIMEOUT 200
+
+enum imsg_type {
+ IMSG_IFSTATE_REQ,
+ IMSG_IFSTATE_RES
};
enum { IFSD_EVTIMER_ADD, IFSD_EVTIMER_DEL };
