Re: pledge ifstated
On Sun, Jul 16, 2017 at 04:47:07PM -0400, Rob Pierce wrote:
> On Thu, Jul 13, 2017 at 09:16:14PM -0400, Rob Pierce wrote:
> > 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
>
> My original diff to initially pledge ifstated with "stdio rpath dns inet proc
> exec" was incorrectly polling from fetch_ifstate which resulted in the delayed
> completion of that function. This new diff resolves the problem.
>
> I also changed one instance of fatal to fatalx in sigchld_handler.
Running setproctitle in the parent breaks rcctl.
This diff removes the parent call to setproctitle so rcctl works again.
Rob
Index: Makefile
===
RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- Makefile20 Jul 2010 02:05:15 - 1.10
+++ Makefile17 Jul 2017 16:38:23 -
@@ -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
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 - 1.50
+++ ifstated.c 17 Jul 2017 16:38:23 -
@@ -25,13 +25,16 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -39,6 +42,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -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,21 @@ 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(, socks[0]);
+
event_init();
log_init(debug, LOG_DAEMON);
log_setverbose(opts & IFSD_OPT_VERBOSE);
@@ -160,6 +183,9 @@ main(int argc, char *argv[])
, 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(_ev, SIGCHLD, sigchld_handler, NULL);
signal_add(_ev, NULL);
@@ -252,6 +278,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, , WNOHANG);
+ if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status)))
+ fatalx("privchild %i exited due to receipt of signal
%i",
+ privchild_pid, WTERMSIG(status));
+ } while (pid == -1 && errno == EINTR);
+
check_external_status(>initstate);
if (conf->curstate != NULL)
check_external_status(conf->curstate);
@@ -599,29 +635,60 @@ do_action(struct ifsd_action *action)
void
fetch_ifstate(void)
{
+ struct imsg imsg;
struct ifaddrs *i
Re: pledge ifstated
On Thu, Jul 13, 2017 at 09:16:14PM -0400, Rob Pierce wrote:
> 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
My original diff to initially pledge ifstated with "stdio rpath dns inet proc
exec" was incorrectly polling from fetch_ifstate which resulted in the delayed
completion of that function. This new diff resolves the problem.
I also changed one instance of fatal to fatalx in sigchld_handler.
Regards,
Rob
Index: Makefile
===
RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- Makefile20 Jul 2010 02:05:15 - 1.10
+++ Makefile16 Jul 2017 20:38:48 -
@@ -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
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 - 1.50
+++ ifstated.c 16 Jul 2017 20:38:48 -
@@ -25,13 +25,16 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -39,6 +42,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -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(, 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[])
, 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(_ev, SIGCHLD, sigchld_handler, NULL);
signal_add(_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, , WNOHANG);
+ if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status)))
+ fatalx("privchild %i exited due to receipt of signal
%i",
+ privchild_pid, WTERMSIG(status));
+ } while (pid == -1 && errno == EINTR);
+
check_external_status(>initstate);
if (conf->curstate != NULL)
check_external_status(conf->curstate);
@@ -599,29 +636,60 @@ do_action(struct ifsd_action *action)
void
fetch_ifstate(void)
{
+ struct imsg imsg;
struct ifaddrs *ifap, *ifa;
char *oname = NULL;
int sock = socket(AF_INET, SOCK_DGRAM, 0);
+ int done = 0, state, rv;
+ ssize_t n;
if (getifaddrs() != 0)
err(1, "getifaddrs");
for (ifa = ifap; ifa; ifa = ifa->ifa_next)
Re: pledge ifstated
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
--- Makefile20 Jul 2010 02:05:15 - 1.10
+++ Makefile14 Jul 2017 01:12:58 -
@@ -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
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 - 1.50
+++ ifstated.c 14 Jul 2017 01:12:58 -
@@ -25,13 +25,16 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -39,6 +42,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -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(, 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[])
, 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(_ev, SIGCHLD, sigchld_handler, NULL);
signal_add(_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, , 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(>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() != 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)
-
- if (ioctl(sock, SIOCGIFDATA, (caddr_t)) == -1)
- continue;
- scan_ifstate(if_nametoindex(ifa->ifa_name),
- ifrdat.ifi_link_state, 0);
+ /*
+* synchronous imsg request/response to privchild
pledge ifstated
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
Index: Makefile
===
RCS file: /cvs/src/usr.sbin/ifstated/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- Makefile20 Jul 2010 02:05:15 - 1.10
+++ Makefile13 Jul 2017 16:17:20 -
@@ -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
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 - 1.50
+++ ifstated.c 13 Jul 2017 16:17:20 -
@@ -25,13 +25,16 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -39,6 +42,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -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(, 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[])
, 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(_ev, SIGCHLD, sigchld_handler, NULL);
signal_add(_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, , 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(>initstate);
if (conf->curstate != NULL)
check_external_status(conf->curstate);
@@ -599,29 +636,75 @@ 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() != 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)
-
- if (ioctl(sock, SIOCGIFDATA, (caddr_t)) == -1)
- continue;
- scan_ifstate(if_nametoindex(ifa->ifa_name),
- ifrdat.ifi_link_state, 0);
+ /*
+* synchronous imsg request/response to privchild
+*/
+ if (imsg_compose(, IMSG_IFSTATE_REQ, 0, 0, -1, ,
+ sizeof(struct ifreq)) == -1)
+
