Unveil fits nicely into the syslogd privsep model. Unveiled files

include config file "r", utmp "r", /dev "rw", /bin/sh "x" for running
piped commands, and the syslogd binary "x" itself for HUP re-exec upon
config loads with changes.  Also unveiled in the privsep process are
the specific log files being written to.

If a config file reload changes no files, the existing privsep process
keeps running with unveil's to the relevant files (therefore it can
cope with newsyslogd taking files away).  If a new config file is loaded
which changes the output files, the privsep process is restarted with
fork+exec, and installs new unveils as needed.  The safety we gain from
unveil is that we've pigeonholed the privsep file-writer to exactly the
files required.

Help from bluhm for some edge cases.

diff --git a/usr.sbin/syslogd/privsep.c b/usr.sbin/syslogd/privsep.c
index 755b1f1..45463db 100644 (file)
--- a/usr.sbin/syslogd/privsep.c
+++ b/usr.sbin/syslogd/privsep.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: privsep.c,v 1.68 2018/04/26 13:40:09 bluhm Exp $      */
+/*     $OpenBSD: privsep.c,v 1.69 2018/08/07 18:36:49 deraadt Exp $    */
 
 /*
  * Copyright (c) 2003 Anil Madhavapeddy <anil@recoil.org>
@@ -184,7 +184,25 @@ priv_exec(char *conf, int numeric, int child, int argc, char *argv[])
        if (pw == NULL)
                errx(1, "unknown user _syslogd");
 
-       if (pledge("stdio rpath wpath cpath dns sendfd id proc exec",
+       if (unveil(conf, "r") == -1)
+               err(1, "unveil");
+       if (unveil(_PATH_UTMP, "r") == -1)
+               err(1, "unveil");
+       if (unveil(_PATH_DEV, "rw") == -1)
+               err(1, "unveil");
+
+       /* for pipes */
+       if (unveil(_PATH_BSHELL, "x") == -1)
+               err(1, "unveil");
+
+       /* For HUP / re-exec */
+       if (unveil("/usr/sbin/syslogd", "x") == -1)
+               err(1, "unveil");
+       if (argv[0][0] == '/')
+               if (unveil(argv[0], "x") == -1)
+                       err(1, "unveil");
+
+       if (pledge("stdio unveil rpath wpath cpath dns sendfd id proc exec",
            NULL) == -1)
                err(1, "pledge priv");
 
@@ -312,6 +330,9 @@ priv_exec(char *conf, int numeric, int child, int argc, char *argv[])
                        break;
 
                case PRIV_DONE_CONFIG_PARSE:
+                       if (pledge("stdio rpath wpath cpath dns sendfd id proc exec",
+                           NULL) == -1)
+                               err(1, "pledge done config");
                        log_debug("[priv]: msg PRIV_DONE_CONFIG_PARSE "
                            "received");
                        increase_state(STATE_RUNNING);
@@ -554,6 +575,10 @@ check_log_name(char *lognam, size_t logsize)
                        err(1, "check_log_name() malloc");
                strlcpy(lg->path, lognam, PATH_MAX);
                TAILQ_INSERT_TAIL(&lognames, lg, next);
+               if (lognam[0] != '|') {
+                       if (unveil(lognam, "w") == -1)
+                               goto bad_path;
+               }
                break;
        case STATE_RUNNING:
                TAILQ_FOREACH(lg, &lognames, next)