Hi,
With this diff syslogd(8) does an exec on itself in the privileged
parent process to reshuffle its memory layout.
As syslogd only forks once, it does not really matter wether we
fork+exec in the child or in the parent. To do it in the parent
is easier as it has much less state.
ok?
bluhm
Index: usr.sbin/syslogd/privsep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v
retrieving revision 1.61
diff -u -p -r1.61 privsep.c
--- usr.sbin/syslogd/privsep.c 28 Jun 2016 18:22:50 -0000 1.61
+++ usr.sbin/syslogd/privsep.c 29 Sep 2016 17:55:03 -0000
@@ -2,6 +2,7 @@
/*
* Copyright (c) 2003 Anil Madhavapeddy <a...@recoil.org>
+ * Copyright (c) 2016 Alexander Bluhm <bl...@openbsd.org>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -71,9 +72,6 @@ enum cmd_types {
static int priv_fd = -1;
static volatile pid_t child_pid = -1;
-static char config_file[PATH_MAX];
-static struct stat cf_info;
-static int allow_getnameinfo = 0;
static volatile sig_atomic_t cur_state = STATE_INIT;
/* Queue for the allowed logfiles */
@@ -94,25 +92,12 @@ static void must_read(int, void *, size_
static void must_write(int, void *, size_t);
static int may_read(int, void *, size_t);
-int
-priv_init(char *conf, int numeric, int lockfd, int nullfd, char *argv[])
+void
+priv_init(int lockfd, int nullfd, int argc, char *argv[])
{
- int i, fd, socks[2], cmd, addr_len, result, restart;
- size_t path_len, protoname_len, hostname_len, servname_len;
- char path[PATH_MAX], protoname[5];
- char hostname[NI_MAXHOST], servname[NI_MAXSERV];
- struct sockaddr_storage addr;
- struct stat cf_stat;
+ int i, socks[2];
struct passwd *pw;
- struct addrinfo hints, *res0;
- struct sigaction sa;
-
- memset(&sa, 0, sizeof(sa));
- sigemptyset(&sa.sa_mask);
- sa.sa_flags = SA_RESTART;
- sa.sa_handler = SIG_DFL;
- for (i = 1; i < _NSIG; i++)
- sigaction(i, &sa, NULL);
+ char childnum[11], **privargv;
/* Create sockets */
if (socketpair(AF_LOCAL, SOCK_STREAM, PF_UNSPEC, socks) == -1)
@@ -141,12 +126,9 @@ priv_init(char *conf, int numeric, int l
err(1, "setresuid() failed");
close(socks[0]);
priv_fd = socks[1];
- return 0;
+ return;
}
-
- if (pledge("stdio rpath wpath cpath dns getpw sendfd id proc exec",
- NULL) == -1)
- err(1, "pledge priv");
+ close(socks[1]);
if (!Debug) {
close(lockfd);
@@ -157,20 +139,6 @@ priv_init(char *conf, int numeric, int l
if (nullfd > 2)
close(nullfd);
- /* Father */
- /* Pass TERM/HUP/INT/QUIT through to child, and accept CHLD */
- sa.sa_handler = sig_pass_to_chld;
- sigaction(SIGTERM, &sa, NULL);
- sigaction(SIGHUP, &sa, NULL);
- sigaction(SIGINT, &sa, NULL);
- sigaction(SIGQUIT, &sa, NULL);
- sa.sa_handler = sig_got_chld;
- sa.sa_flags |= SA_NOCLDSTOP;
- sigaction(SIGCHLD, &sa, NULL);
-
- setproctitle("[priv]");
- close(socks[1]);
-
/* Close descriptors that only the unpriv child needs */
if (fd_ctlconn != -1)
close(fd_ctlconn);
@@ -194,38 +162,87 @@ priv_init(char *conf, int numeric, int l
if (fd_unix[i] != -1)
close(fd_unix[i]);
- /* Save the config file specified by the child process */
- if (strlcpy(config_file, conf, sizeof config_file) >=
sizeof(config_file))
- errx(1, "config_file truncation");
+ if (dup3(socks[0], 3, 0) == -1)
+ err(1, "dup3 priv sock failed");
+ snprintf(childnum, sizeof(childnum), "%d", child_pid);
+ if ((privargv = reallocarray(NULL, argc + 3, sizeof(char *))) == NULL)
+ err(1, "alloc priv argv failed");
+ for (i = 0; i < argc; i++)
+ privargv[i] = argv[i];
+ privargv[i++] = "-P";
+ privargv[i++] = childnum;
+ privargv[i++] = NULL;
+ execv(privargv[0], privargv);
+ err(1, "exec priv '%s' failed", privargv[0]);
+}
- if (stat(config_file, &cf_info) < 0)
- err(1, "stat config file failed");
+__dead void
+priv_exec(char *conf, int numeric, int child, int argc, char *argv[])
+{
+ int i, fd, sock, cmd, addr_len, result, restart;
+ size_t path_len, protoname_len, hostname_len, servname_len;
+ char path[PATH_MAX], protoname[5];
+ char hostname[NI_MAXHOST], servname[NI_MAXSERV];
+ struct sockaddr_storage addr;
+ struct stat cf_info, cf_stat;
+ struct addrinfo hints, *res0;
+ struct sigaction sa;
- /* Save whether or not the child can have access to getnameinfo(3) */
- if (numeric > 0)
- allow_getnameinfo = 0;
- else
- allow_getnameinfo = 1;
+ if (pledge("stdio rpath wpath cpath dns getpw sendfd id proc exec",
+ NULL) == -1)
+ err(1, "pledge priv");
+
+ if (argc <= 2 || strcmp("-P", argv[argc - 2]) != 0)
+ errx(1, "exec without priv");
+ argv[argc -= 2] = NULL;
+
+ sock = 3;
+ for (fd = 4; fd < 1024; fd++)
+ close(fd);
+
+ child_pid = child;
+
+ memset(&sa, 0, sizeof(sa));
+ sigemptyset(&sa.sa_mask);
+ sa.sa_flags = SA_RESTART;
+ sa.sa_handler = SIG_DFL;
+ for (i = 1; i < _NSIG; i++)
+ sigaction(i, &sa, NULL);
+
+ /* Pass TERM/HUP/INT/QUIT through to child, and accept CHLD */
+ sa.sa_handler = sig_pass_to_chld;
+ sigaction(SIGTERM, &sa, NULL);
+ sigaction(SIGHUP, &sa, NULL);
+ sigaction(SIGINT, &sa, NULL);
+ sigaction(SIGQUIT, &sa, NULL);
+ sa.sa_handler = sig_got_chld;
+ sa.sa_flags |= SA_NOCLDSTOP;
+ sigaction(SIGCHLD, &sa, NULL);
+
+ setproctitle("[priv]");
+
+ if (stat(conf, &cf_info) < 0)
+ err(1, "stat config file failed");
TAILQ_INIT(&lognames);
increase_state(STATE_CONFIG);
restart = 0;
while (cur_state < STATE_QUIT) {
- if (may_read(socks[0], &cmd, sizeof(int)))
+ if (may_read(sock, &cmd, sizeof(int)))
break;
switch (cmd) {
case PRIV_OPEN_TTY:
logdebug("[priv]: msg PRIV_OPEN_TTY received\n");
/* Expecting: length, path */
- must_read(socks[0], &path_len, sizeof(size_t));
+ must_read(sock, &path_len, sizeof(size_t));
if (path_len == 0 || path_len > sizeof(path))
_exit(1);
- must_read(socks[0], &path, path_len);
+ must_read(sock, &path, path_len);
path[path_len - 1] = '\0';
check_tty_name(path, sizeof(path));
fd = open(path, O_WRONLY|O_NONBLOCK, 0);
- send_fd(socks[0], fd);
+ send_fd(sock, fd);
if (fd < 0)
warnx("priv_open_tty failed");
else
@@ -237,10 +254,10 @@ priv_init(char *conf, int numeric, int l
logdebug("[priv]: msg PRIV_OPEN_%s received\n",
cmd == PRIV_OPEN_PIPE ? "PIPE" : "LOG");
/* Expecting: length, path */
- must_read(socks[0], &path_len, sizeof(size_t));
+ must_read(sock, &path_len, sizeof(size_t));
if (path_len == 0 || path_len > sizeof(path))
_exit(1);
- must_read(socks[0], &path, path_len);
+ must_read(sock, &path, path_len);
path[path_len - 1] = '\0';
check_log_name(path, sizeof(path));
@@ -251,7 +268,7 @@ priv_init(char *conf, int numeric, int l
else
errx(1, "invalid cmd");
- send_fd(socks[0], fd);
+ send_fd(sock, fd);
if (fd < 0)
warnx("priv_open_log failed");
else
@@ -261,7 +278,7 @@ priv_init(char *conf, int numeric, int l
case PRIV_OPEN_UTMP:
logdebug("[priv]: msg PRIV_OPEN_UTMP received\n");
fd = open(_PATH_UTMP, O_RDONLY|O_NONBLOCK, 0);
- send_fd(socks[0], fd);
+ send_fd(sock, fd);
if (fd < 0)
warnx("priv_open_utmp failed");
else
@@ -270,9 +287,9 @@ priv_init(char *conf, int numeric, int l
case PRIV_OPEN_CONFIG:
logdebug("[priv]: msg PRIV_OPEN_CONFIG received\n");
- stat(config_file, &cf_info);
- fd = open(config_file, O_RDONLY|O_NONBLOCK, 0);
- send_fd(socks[0], fd);
+ stat(conf, &cf_info);
+ fd = open(conf, O_RDONLY|O_NONBLOCK, 0);
+ send_fd(sock, fd);
if (fd < 0)
warnx("priv_open_config failed");
else
@@ -281,16 +298,16 @@ priv_init(char *conf, int numeric, int l
case PRIV_CONFIG_MODIFIED:
logdebug("[priv]: msg PRIV_CONFIG_MODIFIED received\n");
- if (stat(config_file, &cf_stat) < 0 ||
+ if (stat(conf, &cf_stat) < 0 ||
timespeccmp(&cf_info.st_mtimespec,
&cf_stat.st_mtimespec, <) ||
cf_info.st_size != cf_stat.st_size) {
logdebug("config file modified: restarting\n");
restart = result = 1;
- must_write(socks[0], &result, sizeof(int));
+ must_write(sock, &result, sizeof(int));
} else {
result = 0;
- must_write(socks[0], &result, sizeof(int));
+ must_write(sock, &result, sizeof(int));
}
break;
@@ -303,25 +320,25 @@ priv_init(char *conf, int numeric, int l
case PRIV_GETADDRINFO:
logdebug("[priv]: msg PRIV_GETADDRINFO received\n");
/* Expecting: len, proto, len, host, len, serv */
- must_read(socks[0], &protoname_len, sizeof(size_t));
+ must_read(sock, &protoname_len, sizeof(size_t));
if (protoname_len == 0 ||
protoname_len > sizeof(protoname))
_exit(1);
- must_read(socks[0], &protoname, protoname_len);
+ must_read(sock, &protoname, protoname_len);
protoname[protoname_len - 1] = '\0';
- must_read(socks[0], &hostname_len, sizeof(size_t));
+ must_read(sock, &hostname_len, sizeof(size_t));
if (hostname_len == 0 ||
hostname_len > sizeof(hostname))
_exit(1);
- must_read(socks[0], &hostname, hostname_len);
+ must_read(sock, &hostname, hostname_len);
hostname[hostname_len - 1] = '\0';
- must_read(socks[0], &servname_len, sizeof(size_t));
+ must_read(sock, &servname_len, sizeof(size_t));
if (servname_len == 0 ||
servname_len > sizeof(servname))
_exit(1);
- must_read(socks[0], &servname, servname_len);
+ must_read(sock, &servname, servname_len);
servname[servname_len - 1] = '\0';
memset(&hints, 0, sizeof(hints));
@@ -356,34 +373,34 @@ priv_init(char *conf, int numeric, int l
i = getaddrinfo(hostname, servname, &hints, &res0);
if (i != 0 || res0 == NULL) {
addr_len = 0;
- must_write(socks[0], &addr_len, sizeof(int));
+ must_write(sock, &addr_len, sizeof(int));
} else {
/* Just send the first address */
i = res0->ai_addrlen;
- must_write(socks[0], &i, sizeof(int));
- must_write(socks[0], res0->ai_addr, i);
+ must_write(sock, &i, sizeof(int));
+ must_write(sock, res0->ai_addr, i);
freeaddrinfo(res0);
}
break;
case PRIV_GETNAMEINFO:
logdebug("[priv]: msg PRIV_GETNAMEINFO received\n");
- if (!allow_getnameinfo)
+ if (numeric)
errx(1, "rejected attempt to getnameinfo");
/* Expecting: length, sockaddr */
- must_read(socks[0], &addr_len, sizeof(int));
+ must_read(sock, &addr_len, sizeof(int));
if (addr_len <= 0 || (size_t)addr_len > sizeof(addr))
_exit(1);
- must_read(socks[0], &addr, addr_len);
+ must_read(sock, &addr, addr_len);
if (getnameinfo((struct sockaddr *)&addr, addr_len,
hostname, sizeof(hostname), NULL, 0,
NI_NOFQDN|NI_NAMEREQD|NI_DGRAM) != 0) {
addr_len = 0;
- must_write(socks[0], &addr_len, sizeof(int));
+ must_write(sock, &addr_len, sizeof(int));
} else {
addr_len = strlen(hostname) + 1;
- must_write(socks[0], &addr_len, sizeof(int));
- must_write(socks[0], hostname, addr_len);
+ must_write(sock, &addr_len, sizeof(int));
+ must_write(sock, hostname, addr_len);
}
break;
default:
@@ -392,23 +409,23 @@ priv_init(char *conf, int numeric, int l
}
}
- close(socks[0]);
+ close(sock);
/* Unlink any domain sockets that have been opened */
for (i = 0; i < nunix; i++)
- if (fd_unix[i] != -1)
- (void)unlink(path_unix[i]);
- if (path_ctlsock != NULL && fd_ctlsock != -1)
+ (void)unlink(path_unix[i]);
+ if (path_ctlsock != NULL)
(void)unlink(path_ctlsock);
if (restart) {
- int r;
+ int status;
- wait(&r);
+ waitpid(child_pid, &status, 0);
execvp(argv[0], argv);
+ err(1, "exec restart '%s' failed", argv[0]);
}
unlink(_PATH_LOGPID);
- _exit(0);
+ exit(0);
}
static int
Index: usr.sbin/syslogd/syslogd.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.215
diff -u -p -r1.215 syslogd.c
--- usr.sbin/syslogd/syslogd.c 23 Sep 2016 15:46:39 -0000 1.215
+++ usr.sbin/syslogd/syslogd.c 29 Sep 2016 16:25:28 -0000
@@ -208,6 +208,7 @@ int Initialized = 0; /* set when we have
int MarkInterval = 20 * 60; /* interval between marks in seconds */
int MarkSeq = 0; /* mark sequence number */
+int PrivChild = 0; /* Exec the privileged parent process */
int SecureMode = 1; /* when true, speak only unix domain socks */
int NoDNS = 0; /* when true, will refrain from doing DNS
lookups */
int IncludeHostname = 0; /* include RFC 3164 style hostnames when
forwarding */
@@ -358,7 +359,7 @@ main(int argc, char *argv[])
int ch, i;
int lockpipe[2] = { -1, -1}, pair[2], nullfd, fd;
- while ((ch = getopt(argc, argv, "46a:C:c:dFf:hK:k:m:np:S:s:T:U:uV"))
+ while ((ch = getopt(argc, argv, "46a:C:c:dFf:hK:k:m:nP:p:S:s:T:U:uV"))
!= -1)
switch (ch) {
case '4': /* disable IPv6 */
@@ -405,6 +406,11 @@ main(int argc, char *argv[])
case 'n': /* don't do DNS lookups */
NoDNS = 1;
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);
+ break;
case 'p': /* path */
path_unix[0] = optarg;
break;
@@ -441,7 +447,7 @@ main(int argc, char *argv[])
default:
usage();
}
- if ((argc -= optind) != 0)
+ if (argc != optind)
usage();
if (Debug)
@@ -459,6 +465,9 @@ main(int argc, char *argv[])
}
}
+ if (PrivChild > 1)
+ priv_exec(ConfFile, NoDNS, PrivChild, argc, argv);
+
consfile.f_type = F_CONSOLE;
(void)strlcpy(consfile.f_un.f_fname, ctty,
sizeof(consfile.f_un.f_fname));
@@ -691,8 +700,7 @@ main(int argc, char *argv[])
}
/* Privilege separation begins here */
- if (priv_init(ConfFile, NoDNS, lockpipe[1], nullfd, argv) < 0)
- errx(1, "unable to privsep");
+ priv_init(lockpipe[1], nullfd, argc, argv);
if (pledge("stdio unix inet recvfd", NULL) == -1)
err(1, "pledge");
Index: usr.sbin/syslogd/syslogd.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.h,v
retrieving revision 1.23
diff -u -p -r1.23 syslogd.h
--- usr.sbin/syslogd/syslogd.h 23 Oct 2015 16:28:52 -0000 1.23
+++ usr.sbin/syslogd/syslogd.h 29 Sep 2016 16:25:58 -0000
@@ -21,7 +21,8 @@
#include <sys/uio.h>
/* Privilege separation */
-int priv_init(char *, int, int, int, char **);
+void priv_init(int, int, int, char **);
+__dead void priv_exec(char *, int, int, int, char **);
int priv_open_tty(const char *);
int priv_open_log(const char *);
FILE *priv_open_utmp(void);
