Re: syslogd multiple listen addresses

2017-01-01 Thread Todd C. Miller
On Sat, 31 Dec 2016 01:18:16 +0100, Alexander Bluhm wrote:

> Currently syslogd accepts network input only for either IPv4 or
> IPv6.  To overcome this limitation, allow to specify more than one
> listen addresses.
> 
> As multiple TLS contexts need more thought, implement it only for
> TCP and UDP sockets now.

OK millert@

 - todd



Re: syslogd multiple listen addresses

2017-01-01 Thread Todd C. Miller
On Sun, 01 Jan 2017 21:05:58 +0100, Alexander Bluhm wrote:

> Regular programs should die as early as possible when an error
> occurs, then it can be fixed.  But syslogd is special.  If it dies,
> you become blind and don't see any errors at all.  An attacker could
> exploit this.  So I think syslogd should exit during startup e.g.
> if an invalid option was specified.  But then it should just log
> errors and run as many subsystems as possible.
> 
> So yes, a missing address should not be fatal.  But there is more
> to do.  For each error it should be considered wether err() or die()
> or logerror() is called.  This is quite on top on my TODO list.

I couldn't agree more.

 - todd



Re: syslogd multiple listen addresses

2017-01-01 Thread Alexander Bluhm
On Sun, Jan 01, 2017 at 08:43:49AM -0700, Todd C. Miller wrote:
> Now that syslogd supports multiple network sockets, does it still
> make sense to die if one of them cannot be bound?  It seems like
> syslogd should still run as long as there is at least one address
> it can bind to.
> 
> What do you think?

I have never changed syslogd's behavior in case of an error.  It
is more or less how it always was.  Especially the "if (!Debug)
die(0);" looks strange to me.

Regular programs should die as early as possible when an error
occurs, then it can be fixed.  But syslogd is special.  If it dies,
you become blind and don't see any errors at all.  An attacker could
exploit this.  So I think syslogd should exit during startup e.g.
if an invalid option was specified.  But then it should just log
errors and run as many subsystems as possible.

So yes, a missing address should not be fatal.  But there is more
to do.  For each error it should be considered wether err() or die()
or logerror() is called.  This is quite on top on my TODO list.

bluhm



Re: syslogd multiple listen addresses

2017-01-01 Thread Todd C. Miller
Now that syslogd supports multiple network sockets, does it still
make sense to die if one of them cannot be bound?  It seems like
syslogd should still run as long as there is at least one address
it can bind to.

What do you think?

 - todd



syslogd multiple listen addresses

2016-12-30 Thread Alexander Bluhm
Hi,

Currently syslogd accepts network input only for either IPv4 or
IPv6.  To overcome this limitation, allow to specify more than one
listen addresses.

As multiple TLS contexts need more thought, implement it only for
TCP and UDP sockets now.

ok?

bluhm

Index: usr.sbin/syslogd/syslogd.8
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.8,v
retrieving revision 1.52
diff -u -p -r1.52 syslogd.8
--- usr.sbin/syslogd/syslogd.8  17 Oct 2016 11:19:55 -  1.52
+++ usr.sbin/syslogd/syslogd.8  30 Dec 2016 23:48:23 -
@@ -174,6 +174,10 @@ This includes the year and the timezone,
 in UTC.
 .El
 .Pp
+The options
+.Fl a , Fl T , Fl U
+can be given more than once to specify multiple input sources.
+.Pp
 .Nm
 reads its configuration file,
 .Xr syslog.conf 5 ,
Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.226
diff -u -p -r1.226 syslogd.c
--- usr.sbin/syslogd/syslogd.c  30 Dec 2016 23:21:26 -  1.226
+++ usr.sbin/syslogd/syslogd.c  30 Dec 2016 23:33:40 -
@@ -214,13 +214,6 @@ intNoDNS = 0;  /* when true, refrain fr
 intZuluTime = 0;   /* display date and time in UTC ISO format */
 intIncludeHostname = 0;/* include RFC 3164 hostnames when forwarding */
 intFamily = PF_UNSPEC; /* protocol family, may disable IPv4 or IPv6 */
-char   *bind_host = NULL;  /* bind UDP receive socket */
-char   *bind_port = NULL;
-char   *listen_host = NULL;/* listen on TCP receive socket */
-char   *listen_port = NULL;
-char   *tls_hostport = NULL;   /* listen on TLS receive socket */
-char   *tls_host = NULL;
-char   *tls_port = NULL;
 char   *path_ctlsock = NULL;   /* Path to control socket */
 
 struct tls *server_ctx;
@@ -340,6 +333,7 @@ voidusage(void);
 void   wallmsg(struct filed *, struct iovec *);
 intloghost_parse(char *, char **, char **, char **);
 intgetmsgbufsize(void);
+void   address_alloc(const char *, const char *, char ***, char ***, int *);
 intsocket_bind(const char *, const char *, const char *, int,
 int *, int *);
 intunix_socket(char *, int, mode_t);
@@ -359,8 +353,10 @@ main(int argc, char *argv[])
char*p;
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_unix;
+   int  fd_ctlsock, fd_klog, fd_sendsys, *fd_bind, *fd_listen;
+   int *fd_unix, nbind, nlisten;
+   char**bind_host, **bind_port, **listen_host, **listen_port;
+   char*tls_hostport, *tls_host, *tls_port;
 
/* block signal until handler is set up */
sigemptyset();
@@ -373,6 +369,10 @@ main(int argc, char *argv[])
path_unix[0] = _PATH_LOG;
nunix = 1;
 
+   bind_host = bind_port = listen_host = listen_port = NULL;
+   tls_hostport = tls_host = NULL;
+   nbind = nlisten = 0;
+
while ((ch = getopt(argc, argv, "46a:C:c:dFf:hK:k:m:nP:p:S:s:T:U:uVZ"))
!= -1)
switch (ch) {
@@ -385,7 +385,7 @@ main(int argc, char *argv[])
case 'a':
if ((path_unix = reallocarray(path_unix, nunix + 1,
sizeof(*path_unix))) == NULL)
-   err(1, "malloc %s", optarg);
+   err(1, "unix path %s", optarg);
path_unix[nunix++] = optarg;
break;
case 'C':   /* file containing CA certificates */
@@ -440,18 +440,12 @@ main(int argc, char *argv[])
path_ctlsock = optarg;
break;
case 'T':   /* allow tcp and listen on address */
-   if ((p = strdup(optarg)) == NULL)
-   err(1, "strdup listen address");
-   if (loghost_parse(p, NULL, _host, _port)
-   == -1)
-   errx(1, "bad listen address: %s", optarg);
+   address_alloc("listen", optarg, _host,
+   _port, );
break;
case 'U':   /* allow udp only from address */
-   if ((p = strdup(optarg)) == NULL)
-   err(1, "strdup bind address");
-   if (loghost_parse(p, NULL, _host, _port)
-   == -1)
-   errx(1, "bad bind address: %s", optarg);
+   address_alloc("bind", optarg, _host, _port,
+   );
break;
case 'u':   /* allow