On Fri, Feb 09, 2018 at 04:10:14AM -0500, Jiri B wrote:
> I was speculating about another instance of syslogd, just as a log
> host services while having base syslogd running on same box.

I doubt that this makes much sense.  You have one syslogd(8) process
and control everything in syslog.conf(5).  There you can match on
the incoming host name for special treatment.

> 1. -p /dev/null deletes /dev/null and replaces it with socket file
>    with same name

Don't do that.

> srw-rw-rw-  1 root  wheel  0 Feb  8 13:26 /dev/null

Replacing existing files with unix domain sockets may be considerd
a bug.

> Could we make syslogd not to open /dev/klog and disable any unix socket
> listening?

No.  That is syslogd's job.


The file removal is part of a regression introduced by moving to
fork+exec.  Before only files that were actually open were removed.
Now the reexec parent does not know that and removes all files
passed with -p, -a, or -s.

This can be fixed by passing the files that have been successfully
opened with -R to the reexec parent.  While doing that I found a
missing realpath(3) before chdir(2).  It is not clever to remove
relative files in / that were created somewhere else.

Instead of using the global variables nunix, path_unix, and
path_ctlsock I pass the files to be removed to the parent explicitly.

When I started syslogd -d I noticed that it was doing almost what
you want.  It checks whether a unix domain socket is used by another
server.  I guess that this is only done in debug mode to debug
syslogd while another instance is running.

I consider it a bad idea to change behavior in debug mode.  That
makes debugging real problems harder.  So let's rearrange the
existing connect(2) code in a way that only unconnected unix domain
sockets are removed before creating a new server socket.

Althoug I think your use case does not make much sense, I made it
nearly possible by improving the code.  You can utilize that syslogd
does not exit if some subsystems do not work, but continues to use
the others.  Note that syslog.pid does not work as you want it.
I will not fix that.

ok?

bluhm

Index: usr.sbin/syslogd/privsep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v
retrieving revision 1.67
diff -u -p -r1.67 privsep.c
--- usr.sbin/syslogd/privsep.c  5 Apr 2017 11:31:45 -0000       1.67
+++ usr.sbin/syslogd/privsep.c  9 Feb 2018 21:48:21 -0000
@@ -94,7 +94,8 @@ static void must_write(int, void *, size
 static int  may_read(int, void *, size_t);
 
 void
-priv_init(int lockfd, int nullfd, int argc, char *argv[])
+priv_init(int lockfd, int nullfd, int nremove, char *path_remove[],
+    int argc, char *argv[])
 {
        int i, socks[2];
        struct passwd *pw;
@@ -135,6 +136,13 @@ priv_init(int lockfd, int nullfd, int ar
                execpath = argv[0];
        else if ((execpath = realpath(argv[0], NULL)) == NULL)
                err(1, "realpath %s", argv[0]);
+       for (i = 0; i < nremove; i++) {
+               char *tmp;
+
+               if ((tmp = realpath(path_remove[i], NULL)) == NULL)
+                       err(1, "remove path %s", path_remove[i]);
+               path_remove[i] = tmp;
+       }
        if (chdir("/") != 0)
                err(1, "chdir /");
 
@@ -153,11 +161,16 @@ priv_init(int lockfd, int nullfd, int ar
                err(1, "closefrom 4 failed");
 
        snprintf(childnum, sizeof(childnum), "%d", child_pid);
-       if ((privargv = reallocarray(NULL, argc + 3, sizeof(char *))) == NULL)
+       if ((privargv = reallocarray(NULL, argc + 2 * nremove + 3,
+           sizeof(char *))) == NULL)
                err(1, "alloc priv argv failed");
        privargv[0] = execpath;
        for (i = 1; i < argc; i++)
                privargv[i] = argv[i];
+       while (nremove > 0) {
+               privargv[i++] = "-R";
+               privargv[i++] = path_remove[--nremove];
+       }
        privargv[i++] = "-P";
        privargv[i++] = childnum;
        privargv[i++] = NULL;
@@ -166,7 +179,8 @@ priv_init(int lockfd, int nullfd, int ar
 }
 
 __dead void
-priv_exec(char *conf, int numeric, int child, int argc, char *argv[])
+priv_exec(char *conf, int numeric, int child, int nremove, char *path_remove[],
+    int argc, char *argv[])
 {
        int i, fd, sock, cmd, addr_len, result, restart;
        size_t path_len, protoname_len, hostname_len, servname_len;
@@ -406,10 +420,8 @@ priv_exec(char *conf, int numeric, int c
        close(sock);
 
        /* Unlink any domain sockets that have been opened */
-       for (i = 0; i < nunix; i++)
-               (void)unlink(path_unix[i]);
-       if (path_ctlsock != NULL)
-               (void)unlink(path_ctlsock);
+       for (i = 0; i < nremove; i++)
+               (void)unlink(path_remove[i]);
 
        if (restart) {
                int status;
Index: usr.sbin/syslogd/syslogd.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.254
diff -u -p -r1.254 syslogd.c
--- usr.sbin/syslogd/syslogd.c  24 Nov 2017 23:11:42 -0000      1.254
+++ usr.sbin/syslogd/syslogd.c  9 Feb 2018 22:49:37 -0000
@@ -214,8 +214,6 @@ char        *TypeNames[] = {
 SIMPLEQ_HEAD(filed_list, filed) Files;
 struct filed consfile;
 
-int    nunix;                  /* Number of Unix domain sockets requested */
-char   **path_unix;            /* Paths to Unix domain sockets */
 int    Debug;                  /* debug flag */
 int    Foreground;             /* run in foreground, instead of daemonizing */
 char   LocalHostName[HOST_NAME_MAX+1]; /* our hostname */
@@ -232,7 +230,6 @@ int NoDNS = 0;              /* when true, refrain fr
 int    ZuluTime = 0;           /* display date and time in UTC ISO format */
 int    IncludeHostname = 0;    /* include RFC 3164 hostnames when forwarding */
 int    Family = PF_UNSPEC;     /* protocol family, may disable IPv4 or IPv6 */
-char   *path_ctlsock = NULL;   /* Path to control socket */
 
 struct tls *server_ctx;
 struct tls_config *client_config, *server_config;
@@ -371,7 +368,9 @@ main(int argc, char *argv[])
        int              ch, i;
        int              lockpipe[2] = { -1, -1}, pair[2], nullfd, fd;
        int              fd_ctlsock, fd_klog, fd_sendsys, *fd_bind, *fd_listen;
-       int             *fd_tls, *fd_unix, nbind, nlisten, ntls;
+       int             *fd_tls, *fd_unix;
+       int              nunix, nremove, nbind, nlisten, ntls;
+       char            *path_ctlsock, **path_unix, **path_remove;
        char            **bind_host, **bind_port, **listen_host, **listen_port;
        char            *tls_hostport, **tls_host, **tls_port;
 
@@ -381,10 +380,13 @@ main(int argc, char *argv[])
        if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
                err(1, "sigprocmask block");
 
+       path_ctlsock = NULL;
        if ((path_unix = malloc(sizeof(*path_unix))) == NULL)
                err(1, "malloc %s", _PATH_LOG);
        path_unix[0] = _PATH_LOG;
        nunix = 1;
+       path_remove = NULL;
+       nremove = 0;
 
        bind_host = listen_host = tls_host = NULL;
        bind_port = listen_port = tls_port = NULL;
@@ -392,7 +394,7 @@ main(int argc, char *argv[])
        nbind = nlisten = ntls = 0;
 
        while ((ch = getopt(argc, argv,
-           "46a:C:c:dFf:hK:k:m:nP:p:rS:s:T:U:uVZ")) != -1) {
+           "46a:C:c:dFf:hK:k:m:nP:p:R:rS:s:T:U:uVZ")) != -1) {
                switch (ch) {
                case '4':               /* disable IPv6 */
                        Family = PF_INET;
@@ -447,6 +449,12 @@ main(int argc, char *argv[])
                case 'p':               /* path */
                        path_unix[0] = optarg;
                        break;
+               case 'R':               /* used internally, remove at exit */
+                       if ((path_remove = reallocarray(path_remove,
+                           nremove + 1, sizeof(*path_remove))) == NULL)
+                               err(1, "remove path %s", optarg);
+                       path_remove[nremove++] = optarg;
+                       break;
                case 'r':
                        Repeat++;
                        break;
@@ -497,7 +505,8 @@ main(int argc, char *argv[])
        }
 
        if (PrivChild > 1)
-               priv_exec(ConfFile, NoDNS, PrivChild, argc, argv);
+               priv_exec(ConfFile, NoDNS, PrivChild, nremove, path_remove,
+                   argc, argv);
 
        consfile.f_type = F_CONSOLE;
        (void)strlcpy(consfile.f_un.f_fname, ctty,
@@ -547,6 +556,10 @@ main(int argc, char *argv[])
                        log_warnx("socket listen tls failed");
        }
 
+       nremove = 0;
+       if ((path_remove = reallocarray(path_remove, nunix + 1,
+           sizeof(*path_remove))) == NULL)
+               fatal("allocate remove path");
        if ((fd_unix = reallocarray(NULL, nunix, sizeof(*fd_unix))) == NULL)
                fatal("allocate unix fd");
        for (i = 0; i < nunix; i++) {
@@ -556,6 +569,7 @@ main(int argc, char *argv[])
                                log_warnx("log socket %s failed", path_unix[i]);
                        continue;
                }
+               path_remove[nremove++] = path_unix[i];
                double_sockbuf(fd_unix[i], SO_RCVBUF);
        }
 
@@ -574,6 +588,7 @@ main(int argc, char *argv[])
                if (fd_ctlsock == -1) {
                        log_warnx("control socket %s failed", path_ctlsock);
                } else {
+                       path_remove[nremove++] = path_ctlsock;
                        if (listen(fd_ctlsock, 5) == -1) {
                                log_warn("listen control socket");
                                close(fd_ctlsock);
@@ -750,7 +765,7 @@ main(int argc, char *argv[])
        }
 
        /* Privilege separation begins here */
-       priv_init(lockpipe[1], nullfd, argc, argv);
+       priv_init(lockpipe[1], nullfd, nremove, path_remove, argc, argv);
 
        if (pledge("stdio unix inet recvfd", NULL) == -1)
                err(1, "pledge");
@@ -2999,14 +3014,12 @@ unix_socket(char *path, int type, mode_t
                return (-1);
        }
 
-       if (Debug) {
-               if (connect(fd, (struct sockaddr *)&s_un, sizeof(s_un)) == 0 ||
-                   errno == EPROTOTYPE) {
-                       close(fd);
-                       errno = EISCONN;
-                       log_warn("connect unix \"%s\"", path);
-                       return (-1);
-               }
+       errno = EISCONN;
+       if (connect(fd, (struct sockaddr *)&s_un, sizeof(s_un)) == 0 ||
+           errno == EPROTOTYPE || errno == ENOTSOCK) {
+               close(fd);
+               log_warn("connect unix \"%s\"", path);
+               return (-1);
        }
 
        old_umask = umask(0177);
Index: usr.sbin/syslogd/syslogd.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.h,v
retrieving revision 1.32
diff -u -p -r1.32 syslogd.h
--- usr.sbin/syslogd/syslogd.h  5 Oct 2017 16:15:24 -0000       1.32
+++ usr.sbin/syslogd/syslogd.h  9 Feb 2018 21:17:49 -0000
@@ -24,8 +24,8 @@
 #include <stdarg.h>
 
 /* Privilege separation */
-void  priv_init(int, int, int, char **);
-__dead void priv_exec(char *, int, int, int, char **);
+void  priv_init(int, int, int, char **, int, char **);
+__dead void priv_exec(char *, int, int, int, char**, int, char **);
 int   priv_open_tty(const char *);
 int   priv_open_log(const char *);
 FILE *priv_open_utmp(void);
@@ -43,11 +43,6 @@ void ttymsg(struct iovec *, int, char *)
 /* File descriptor send/recv */
 void send_fd(int, int);
 int  receive_fd(int);
-
-/* The list of domain sockets */
-extern int nunix;
-extern char **path_unix;
-extern char *path_ctlsock;
 
 #define ERRBUFSIZE     256
 void vlogmsg(int pri, const char *, const char *, va_list);

Reply via email to