>From lessons learned with tcpdump and talking to deraadt@ and bluhm@,
this reworks pflogd to re-exec its unpriv child instead of the priv
parent, my initial understanding of the syslogd privsep design was off
and pflogd doesn't need any such clever approach.
I can also send a diff with my previous attempt reverted if that helps
ease review.
-Bryan.
Index: pflogd.c
===================================================================
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.57
diff -u -p -u -r1.57 pflogd.c
--- sbin/pflogd/pflogd.c 8 Sep 2017 13:34:29 -0000 1.57
+++ sbin/pflogd/pflogd.c 8 Sep 2017 15:03:21 -0000
@@ -54,7 +54,6 @@ pcap_t *hpcap;
static FILE *dpcap;
int Debug = 0;
-static int privchild = 0;
static int snaplen = DEF_SNAPLEN;
static int cur_snaplen = DEF_SNAPLEN;
@@ -534,13 +533,13 @@ dump_packet(u_char *user, const struct p
int
main(int argc, char **argv)
{
- int ch, np, ret, Xflag = 0;
+ int ch, np, ret, Pflag = 0, Xflag = 0;
pcap_handler phandler = dump_packet;
const char *errstr = NULL;
ret = 0;
- while ((ch = getopt(argc, argv, "Dxd:f:i:P:s:")) != -1) {
+ while ((ch = getopt(argc, argv, "Dxd:f:i:Ps:")) != -1) {
switch (ch) {
case 'D':
Debug = 1;
@@ -556,10 +555,9 @@ main(int argc, char **argv)
case 'i':
interface = optarg;
break;
- case 'P': /* used internally, exec the parent */
- privchild = strtonum(optarg, 2, INT_MAX, &errstr);
- if (errstr)
- errx(1, "priv child %s: %s", errstr, optarg);
+ case 'P': /* used internally, exec the child */
+ if (strcmp("-P", argv[1]) == 0)
+ Pflag = 1;
break;
case 's':
snaplen = strtonum(optarg, 0, PFLOGD_MAXSNAPLEN,
@@ -593,9 +591,12 @@ main(int argc, char **argv)
if (!Debug) {
openlog("pflogd", LOG_PID, LOG_DAEMON);
- if (daemon(0, 0)) {
- logmsg(LOG_WARNING, "Failed to become daemon: %s",
- strerror(errno));
+ if (!Pflag) {
+ if (daemon(0, 0)) {
+ logmsg(LOG_WARNING,
+ "Failed to become daemon: %s",
+ strerror(errno));
+ }
}
}
@@ -611,11 +612,8 @@ main(int argc, char **argv)
argc += optind;
argv -= optind;
- if (privchild > 1)
- priv_exec(privchild, argc, argv);
-
/* Privilege separation begins here */
- priv_init(argc, argv);
+ priv_init(Pflag, argc, argv);
if (pledge("stdio recvfd", NULL) == -1)
err(1, "pledge");
Index: pflogd.h
===================================================================
RCS file: /cvs/src/sbin/pflogd/pflogd.h,v
retrieving revision 1.6
diff -u -p -u -r1.6 pflogd.h
--- sbin/pflogd/pflogd.h 5 Sep 2017 15:41:25 -0000 1.6
+++ sbin/pflogd/pflogd.h 8 Sep 2017 15:03:21 -0000
@@ -34,8 +34,7 @@
void logmsg(int priority, const char *message, ...);
/* Privilege separation */
-void priv_init(int, char **);
-__dead void priv_exec(int, int, char **);
+void priv_init(int, int, char **);
int priv_init_pcap(int);
int priv_set_snaplen(int snaplen);
int priv_open_log(void);
Index: privsep.c
===================================================================
RCS file: /cvs/src/sbin/pflogd/privsep.c,v
retrieving revision 1.29
diff -u -p -u -r1.29 privsep.c
--- sbin/pflogd/privsep.c 6 Sep 2017 12:43:16 -0000 1.29
+++ sbin/pflogd/privsep.c 8 Sep 2017 15:03:21 -0000
@@ -63,26 +63,20 @@ extern pcap_t *hpcap;
/* based on syslogd privsep */
void
-priv_init(int argc, char *argv[])
+priv_init(int Pflag, int argc, char *argv[])
{
- int i, nargc, socks[2];
+ int i, fd = -1, bpfd = -1, nargc, socks[2], cmd;
+ int snaplen, ret, olderrno;
struct passwd *pw;
- char childnum[11], **privargv;
-
- /* Create sockets */
- if (socketpair(AF_LOCAL, SOCK_STREAM, PF_UNSPEC, socks) == -1)
- err(1, "socketpair() failed");
+ char **nargv;
+ unsigned int buflen;
pw = getpwnam("_pflogd");
if (pw == NULL)
errx(1, "unknown user _pflogd");
endpwent();
- child_pid = fork();
- if (child_pid < 0)
- err(1, "fork() failed");
-
- if (!child_pid) {
+ if (Pflag) {
gid_t gidset[1];
/* Child - drop privileges and return */
@@ -98,47 +92,36 @@ priv_init(int argc, char *argv[])
err(1, "setgroups() failed");
if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1)
err(1, "setresuid() failed");
- close(socks[0]);
- priv_fd = socks[1];
+ priv_fd = 3;
return;
}
- close(socks[1]);
-
- if (dup2(socks[0], 3) == -1)
- err(1, "dup2 priv sock failed");
- closefrom(4);
-
- snprintf(childnum, sizeof(childnum), "%d", child_pid);
- if ((privargv = reallocarray(NULL, argc + 3, sizeof(char *))) == NULL)
- err(1, "alloc priv argv failed");
- nargc = 0;
- privargv[nargc++] = argv[0];
- privargv[nargc++] = "-P";
- privargv[nargc++] = childnum;
- for (i = 1; i < argc; i++)
- privargv[nargc++] = argv[i];
- privargv[nargc] = NULL;
- execvp(privargv[0], privargv);
- err(1, "exec priv '%s' failed", privargv[0]);
-}
-__dead void
-priv_exec(int child, int argc, char *argv[])
-{
- int i, fd = -1, bpfd = -1, sock, cmd;
- int snaplen, ret, olderrno;
- unsigned int buflen;
-
- if (argc <= 2 || strcmp("-P", argv[1]) != 0)
- errx(1, "exec without priv");
-
- sock = 3;
- closefrom(4);
+ /* Create sockets */
+ if (socketpair(AF_LOCAL, SOCK_STREAM, PF_UNSPEC, socks) == -1)
+ err(1, "socketpair() failed");
- child_pid = child;
+ child_pid = fork();
+ if (child_pid < 0)
+ err(1, "fork() failed");
- for (i = 1; i < _NSIG; i++)
- signal(i, SIG_DFL);
+ if (!child_pid) {
+ close(socks[0]);
+ if (dup2(socks[1], 3) == -1)
+ err(1, "dup2 unpriv sock failed");
+ close(socks[1]);
+ if ((nargv = reallocarray(NULL, argc + 2,
+ sizeof(char *))) == NULL)
+ err(1, "alloc unpriv argv failed");
+ nargc = 0;
+ nargv[nargc++] = argv[0];
+ nargv[nargc++] = "-P";
+ for (i = 1; i < argc; i++)
+ nargv[nargc++] = argv[i];
+ nargv[nargc] = NULL;
+ execvp(nargv[0], nargv);
+ err(1, "exec unpriv '%s' failed", nargv[0]);
+ }
+ close(socks[1]);
/* Father */
/* Pass ALRM/TERM/HUP/INT/QUIT through to child, and accept CHLD */
@@ -157,7 +140,7 @@ BROKEN if (pledge("stdio rpath wpath cpa
#endif
while (1) {
- if (may_read(sock, &cmd, sizeof(int)))
+ if (may_read(socks[0], &cmd, sizeof(int)))
break;
switch (cmd) {
case PRIV_INIT_PCAP:
@@ -169,9 +152,9 @@ BROKEN if (pledge("stdio rpath wpath cpa
_exit(1);
}
buflen = hpcap->bufsize; /* BIOCGBLEN for unpriv proc */
- must_write(sock, &buflen, sizeof(unsigned int));
+ must_write(socks[0], &buflen, sizeof(unsigned int));
fd = pcap_fileno(hpcap);
- send_fd(sock, fd);
+ send_fd(socks[0], fd);
if (fd < 0) {
logmsg(LOG_ERR, "[priv]: Exiting, init failed");
_exit(1);
@@ -181,7 +164,7 @@ BROKEN if (pledge("stdio rpath wpath cpa
case PRIV_SET_SNAPLEN:
logmsg(LOG_DEBUG,
"[priv]: msg PRIV_SET_SNAPLENGTH received");
- must_read(sock, &snaplen, sizeof(int));
+ must_read(socks[0], &snaplen, sizeof(int));
ret = set_snaplen(snaplen);
if (ret) {
@@ -190,7 +173,7 @@ BROKEN if (pledge("stdio rpath wpath cpa
snaplen);
}
- must_write(sock, &ret, sizeof(int));
+ must_write(socks[0], &ret, sizeof(int));
break;
case PRIV_OPEN_LOG:
@@ -205,7 +188,7 @@ BROKEN if (pledge("stdio rpath wpath cpa
O_RDWR|O_CREAT|O_APPEND|O_NONBLOCK|O_NOFOLLOW,
0600);
olderrno = errno;
- send_fd(sock, bpfd);
+ send_fd(socks[0], bpfd);
if (bpfd < 0)
logmsg(LOG_NOTICE,
"[priv]: failed to open %s: %s",
@@ -216,7 +199,7 @@ BROKEN if (pledge("stdio rpath wpath cpa
logmsg(LOG_DEBUG,
"[priv]: msg PRIV_MOVE_LOG received");
ret = move_log(filename);
- must_write(sock, &ret, sizeof(int));
+ must_write(socks[0], &ret, sizeof(int));
break;
default:
