On Thu, Aug 16, 2018 at 04:20:54PM -0400, Bryan Steele wrote: > 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.
Please ignore this diff for now. > 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 */
