This adds unveil to pflogd(8) pflogd(8) is a special case, residing in /sbin, it's a static PIE. As such, I thought it might be worth experimenting with execpromises here. This allows re-exec after privdrop, and removes chroot(2) in favour of only unveil(2) and pledge(2). I've left the code there for now just in case portability is a concern.
The privileged part of pflogd(8) is now disallowed from accessing most of the filesystem, veiled except for read-only opening of /dev/bpf, and rwc for the log file, typically /var/log/pflog. This process cannot yet pledge(2), so special care is needed to make sure no library functions called use files permitted by certain pledges. The unprivileged part already runs pledged "stdio recvfd" This includes the diff I sent previously, I'd like to commit that part separately, any oks on that one? https://marc.info/?l=openbsd-tech&m=153347271628532&w=2 Also, ok for this? ;-) -Bryan. Index: pflogd.8 =================================================================== RCS file: /cvs/src/sbin/pflogd/pflogd.8,v retrieving revision 1.49 diff -u -p -u -r1.49 pflogd.8 --- sbin/pflogd/pflogd.8 30 May 2017 17:15:06 -0000 1.49 +++ sbin/pflogd/pflogd.8 16 Aug 2018 20:05:12 -0000 @@ -86,9 +86,8 @@ temporarily uses the old snaplen to keep tries to preserve the integrity of the log file against I/O errors. Furthermore, integrity of an existing log file is verified before appending. -If there is an invalid log file or an I/O error, the log file is moved -out of the way and a new one is created. -If a new file cannot be created, logging is suspended until a +If there is an invalid log file or an I/O error, logging is suspended +until a .Dv SIGHUP or a .Dv SIGALRM Index: pflogd.c =================================================================== RCS file: /cvs/src/sbin/pflogd/pflogd.c,v retrieving revision 1.58 diff -u -p -u -r1.58 pflogd.c --- sbin/pflogd/pflogd.c 9 Sep 2017 13:02:52 -0000 1.58 +++ sbin/pflogd/pflogd.c 16 Aug 2018 20:05:12 -0000 @@ -75,7 +75,7 @@ int flush_buffer(FILE *); int if_exists(char *); void logmsg(int, const char *, ...); void purge_buffer(void); -int reset_dump(int); +int reset_dump(void); int scan_dump(FILE *, off_t); int set_snaplen(int); void set_suspended(int); @@ -84,8 +84,6 @@ void sig_close(int); void sig_hup(int); void usage(void); -static int try_reset_dump(int); - /* buffer must always be greater than snaplen */ static int bufpkt = 0; /* number of packets in buffer */ static int buflen = 0; /* allocated size of buffer */ @@ -199,6 +197,11 @@ if_exists(char *ifname) int init_pcap(void) { + if (unveil("/dev/bpf", "r") == -1) { + logmsg(LOG_ERR, "unveil: %s", strerror(errno)); + return (-1); + } + hpcap = pcap_open_live(interface, snaplen, 1, PCAP_TO_MS, errbuf); if (hpcap == NULL) { logmsg(LOG_ERR, "Failed to initialize: %s", errbuf); @@ -220,6 +223,11 @@ init_pcap(void) return (-1); } + if (unveil(NULL, NULL) == -1) { + logmsg(LOG_ERR, "unveil: %s", strerror(errno)); + return (-1); + } + return (0); } @@ -238,25 +246,7 @@ set_snaplen(int snap) } int -reset_dump(int nomove) -{ - int ret; - - for (;;) { - ret = try_reset_dump(nomove); - if (ret <= 0) - break; - } - - return (ret); -} - -/* - * tries to (re)open log file, nomove flag is used with -x switch - * returns 0: success, 1: retry (log moved), -1: error - */ -int -try_reset_dump(int nomove) +reset_dump(void) { struct pcap_file_header hdr; struct stat st; @@ -323,12 +313,9 @@ try_reset_dump(int nomove) } } else if (scan_dump(fp, st.st_size)) { fclose(fp); - if (nomove || priv_move_log()) { - logmsg(LOG_ERR, - "Invalid/incompatible log file, move it away"); - return (-1); - } - return (1); + logmsg(LOG_ERR, + "Invalid/incompatible log file, move it away"); + return (-1); } dpcap = fp; @@ -641,7 +628,7 @@ main(int argc, char **argv) bufpkt = 0; } - if (reset_dump(Xflag) < 0) { + if (reset_dump() < 0) { if (Xflag) return (1); @@ -666,10 +653,14 @@ main(int argc, char **argv) if (gotsig_close) break; if (gotsig_hup) { - if (reset_dump(0)) { + int was_suspended = suspended; + if (reset_dump()) { logmsg(LOG_ERR, "Logging suspended: open error"); set_suspended(1); + } else { + if (was_suspended) + logmsg(LOG_NOTICE, "Logging resumed"); } gotsig_hup = 0; } Index: pflogd.h =================================================================== RCS file: /cvs/src/sbin/pflogd/pflogd.h,v retrieving revision 1.7 diff -u -p -u -r1.7 pflogd.h --- sbin/pflogd/pflogd.h 9 Sep 2017 13:02:52 -0000 1.7 +++ sbin/pflogd/pflogd.h 16 Aug 2018 20:05:12 -0000 @@ -27,6 +27,7 @@ #define PFLOGD_LOG_FILE "/var/log/pflog" #define PFLOGD_DEFAULT_IF "pflog0" +#define PATH_PFLOGD "/sbin/pflogd" #define PFLOGD_MAXSNAPLEN INT_MAX #define PFLOGD_BUFSIZE 65536 /* buffer size for incoming packets */ Index: privsep.c =================================================================== RCS file: /cvs/src/sbin/pflogd/privsep.c,v retrieving revision 1.30 diff -u -p -u -r1.30 privsep.c --- sbin/pflogd/privsep.c 9 Sep 2017 13:02:52 -0000 1.30 +++ sbin/pflogd/privsep.c 16 Aug 2018 20:05:12 -0000 @@ -42,7 +42,6 @@ enum cmd_types { PRIV_INIT_PCAP, /* init pcap fdpass bpf */ PRIV_SET_SNAPLEN, /* set the snaplength */ - PRIV_MOVE_LOG, /* move logfile away */ PRIV_OPEN_LOG, /* open logfile for appending */ }; @@ -54,7 +53,6 @@ static int may_read(int, void *, size_t static void must_read(int, void *, size_t); static void must_write(int, void *, size_t); static int set_snaplen(int snap); -static int move_log(const char *name); extern char *filename; extern char *interface; @@ -77,6 +75,7 @@ priv_init(int Pflag, int argc, char *arg endpwent(); if (Pflag) { +#if 0 gid_t gidset[1]; /* Child - drop privileges and return */ @@ -92,6 +91,8 @@ priv_init(int Pflag, int argc, char *arg err(1, "setgroups() failed"); if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1) err(1, "setresuid() failed"); +#endif + /* re-exec'd child reduces pledge to "stdio recvfd" */ priv_fd = 3; return; } @@ -105,6 +106,10 @@ priv_init(int Pflag, int argc, char *arg err(1, "fork() failed"); if (!child_pid) { + if (unveil(PATH_PFLOGD, "x") == -1) + err(1, "unveil"); + if (pledge("stdio id exec", "stdio getpw recvfd") == -1) + err(1, "pledge"); close(socks[0]); if (dup2(socks[1], 3) == -1) err(1, "dup2 unpriv sock failed"); @@ -113,12 +118,22 @@ priv_init(int Pflag, int argc, char *arg sizeof(char *))) == NULL) err(1, "alloc unpriv argv failed"); nargc = 0; - nargv[nargc++] = argv[0]; + nargv[nargc++] = PATH_PFLOGD; nargv[nargc++] = "-P"; for (i = 1; i < argc; i++) nargv[nargc++] = argv[i]; nargv[nargc] = NULL; - execvp(nargv[0], nargv); + + gid_t gidset[1]; + gidset[0] = pw->pw_gid; + if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) == -1) + err(1, "setresgid() failed"); + if (setgroups(1, gidset) == -1) + err(1, "setgroups() failed"); + if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1) + err(1, "setresuid() failed"); + + execv(nargv[0], nargv); err(1, "exec unpriv '%s' failed", nargv[0]); } close(socks[1]); @@ -133,6 +148,9 @@ priv_init(int Pflag, int argc, char *arg setproctitle("[priv]"); + if (unveil(filename, "rwc") == -1) + err(1, "unveil"); + #if 0 /* This needs to do bpf ioctl */ BROKEN if (pledge("stdio rpath wpath cpath sendfd proc bpf", NULL) == -1) @@ -195,13 +213,6 @@ BROKEN if (pledge("stdio rpath wpath cpa filename, strerror(olderrno)); break; - case PRIV_MOVE_LOG: - logmsg(LOG_DEBUG, - "[priv]: msg PRIV_MOVE_LOG received"); - ret = move_log(filename); - must_write(socks[0], &ret, sizeof(int)); - break; - default: logmsg(LOG_ERR, "[priv]: unknown command %d", cmd); _exit(1); @@ -225,48 +236,6 @@ set_snaplen(int snap) return 0; } -static int -move_log(const char *name) -{ - char ren[PATH_MAX]; - int len; - - for (;;) { - int fd; - - len = snprintf(ren, sizeof(ren), "%s.bad.XXXXXXXX", name); - if (len >= sizeof(ren)) { - logmsg(LOG_ERR, "[priv] new name too long"); - return (1); - } - - /* lock destination */ - fd = mkstemp(ren); - if (fd >= 0) { - close(fd); - break; - } - if (errno != EINTR) { - logmsg(LOG_ERR, "[priv] failed to create new name: %s", - strerror(errno)); - return (1); - } - } - - if (rename(name, ren)) { - logmsg(LOG_ERR, "[priv] failed to rename %s to %s: %s", - name, ren, strerror(errno)); - unlink(ren); - return (1); - } - - logmsg(LOG_NOTICE, - "[priv]: log file %s moved to %s", name, ren); - - return (0); -} - - /* receive bpf fd from privileged process using fdpass and init pcap */ int priv_init_pcap(int snaplen) @@ -345,22 +314,6 @@ priv_open_log(void) fd = receive_fd(priv_fd); return (fd); -} - -/* Move-away and reopen log-file */ -int -priv_move_log(void) -{ - int cmd, ret; - - if (priv_fd < 0) - errx(1, "%s: called from privileged portion", __func__); - - cmd = PRIV_MOVE_LOG; - must_write(priv_fd, &cmd, sizeof(int)); - must_read(priv_fd, &ret, sizeof(int)); - - return (ret); } /* If priv parent gets a TERM or HUP, pass it through to child instead */
