Re: syslog over TLS

2015-02-03 Thread sven falempin
On Fri, Jan 16, 2015 at 12:54 PM, Reyk Floeter r...@openbsd.org wrote:
 On Fri, Jan 16, 2015 at 12:56:45PM +0100, Alexander Bluhm wrote:
 On Fri, Jan 16, 2015 at 01:46:09AM +0100, Alexander Bluhm wrote:
  This diff enables sending syslog messages over TLS.

 Updated diff after sys/param.h commit.  Only some context changed.

 bluhm


 Despite my scepticism about putting it into libevent -

 The diff looks and works fine (lightly tested).

 The evbuffer_tls.c code is similar enough to what we have in
 relayd/httpd and libevent;  so I don't see any problems here.

 Minor comments (not inline):
 - The ebuf[100] looks a bit weird, but it is not a problem.
 - I would prefer checking return value as -1 instead of  0
   (tls_init() and other calls below).
 - The manpage bits are missing.

 I'm fine with putting it in and to improve/discuss the API issues later.

 OK

 Reyk

 Index: usr.sbin/syslogd/Makefile
 ===
 RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/Makefile,v
 retrieving revision 1.6
 diff -u -p -r1.6 Makefile
 --- usr.sbin/syslogd/Makefile 5 Oct 2014 18:14:01 -   1.6
 +++ usr.sbin/syslogd/Makefile 16 Jan 2015 11:45:40 -
 @@ -1,9 +1,9 @@
  #$OpenBSD: Makefile,v 1.6 2014/10/05 18:14:01 bluhm Exp $

  PROG=syslogd
 -SRCS=syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c
 +SRCS=syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c 
 evbuffer_tls.c
  MAN= syslogd.8 syslog.conf.5
 -LDADD=   -levent
 -DPADD=   ${LIBEVENT}
 +LDADD=   -levent -ltls -lssl -lcrypto
 +DPADD=   ${LIBEVENT} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}

  .include bsd.prog.mk
 Index: usr.sbin/syslogd/evbuffer_tls.c
 ===
 RCS file: usr.sbin/syslogd/evbuffer_tls.c
 diff -N usr.sbin/syslogd/evbuffer_tls.c
 --- /dev/null 1 Jan 1970 00:00:00 -
 +++ usr.sbin/syslogd/evbuffer_tls.c   16 Jan 2015 11:45:40 -
 @@ -0,0 +1,357 @@
 +/*   $OpenBSD$ */
 +
 +/*
 + * Copyright (c) 2002-2004 Niels Provos pro...@citi.umich.edu
 + * Copyright (c) 2014-2015 Alexander Bluhm bl...@openbsd.org
 + * All rights reserved.
 + *
 + * Redistribution and use in source and binary forms, with or without
 + * modification, are permitted provided that the following conditions
 + * are met:
 + * 1. Redistributions of source code must retain the above copyright
 + *notice, this list of conditions and the following disclaimer.
 + * 2. Redistributions in binary form must reproduce the above copyright
 + *notice, this list of conditions and the following disclaimer in the
 + *documentation and/or other materials provided with the distribution.
 + * 3. The name of the author may not be used to endorse or promote products
 + *derived from this software without specific prior written permission.
 + *
 + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
 + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
 + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
 + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
 + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
 + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
 + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 + */
 +
 +#include sys/types.h
 +#include sys/time.h
 +#include sys/ioctl.h
 +
 +#include errno.h
 +#include event.h
 +#include stdio.h
 +#include stdlib.h
 +#include string.h
 +#include stdarg.h
 +#include tls.h
 +
 +#include evbuffer_tls.h
 +
 +/* prototypes */
 +
 +void bufferevent_read_pressure_cb(struct evbuffer *, size_t, size_t, void 
 *);
 +int evtls_read(struct evbuffer *, int, int, struct tls *);
 +int evtls_write(struct evbuffer *, int, struct tls *);
 +
 +static int
 +bufferevent_add(struct event *ev, int timeout)
 +{
 + struct timeval tv, *ptv = NULL;
 +
 + if (timeout) {
 + timerclear(tv);
 + tv.tv_sec = timeout;
 + ptv = tv;
 + }
 +
 + return (event_add(ev, ptv));
 +}
 +
 +static void
 +buffertls_readcb(int fd, short event, void *arg)
 +{
 + struct buffertls *buftls = arg;
 + struct bufferevent *bufev = buftls-bt_bufev;
 + struct tls *ctx = buftls-bt_ctx;
 + int res = 0;
 + short what = EVBUFFER_READ;
 + size_t len;
 + int howmuch = -1;
 +
 + if (event == EV_TIMEOUT) {
 + what |= EVBUFFER_TIMEOUT;
 + goto error;
 + }
 +
 + /*
 +  * If we have a high watermark configured then we don't want to
 +  * read more data than would make us reach the watermark.
 +  */
 + if (bufev-wm_read.high 

Re: syslog over TLS

2015-01-16 Thread Reyk Floeter
On Fri, Jan 16, 2015 at 01:46:09AM +0100, Alexander Bluhm wrote:
 Hi,
 
 This diff enables sending syslog messages over TLS.
 
 To implement the buffer layer, I have copied evbuffer.c from libevent
 and changed TCP to TLS where necessary.  This way I made a buffertls
 wrapper around bufferevent.  This might be integrated into libevent
 later.
 
 It still has some limitations:
 - No certificate validation.  This will get a bit tricky because
   of privsep.

Not that tricky - it has already been done and I intended to port it
to libtls/libssl.  Let me see if I can create a diff quickly.

 - Wrong format.  The TLS RFC requires length-message encoding, I
   use message-newline inherited from TCP.
 - Not all lost messages are logged.
 - At SIGHUP messages may get lost.
 - Man page is missing.  You can active it with @tls://ip-address.
 
 comment, test, ok?
 
 bluhm
 

The implementation looks somewhat similar to the code in httpd/relayd
that didn't copy the evbuffer code.  But instead of calling
tls_read/tls_write directly, you implemented evtls_read/evtls_write.
I wonder if this solves/improves anything in the daemon but I haven't
seen any related issue.

I'm fine with adding it to syslogd locally and the diff looks sane.

But I'm not convinced that libevent is the right place to add a
dependency on libtls.  I never pushed the SSL evbuffer wrappers back
into libevent to keep portability and to avoid such a dependency; but
I also saw that the implementation outside of libevent is a bit
hackish because the evbuffers were never intended for anything except
TCP.

relayd could also benefit from it, but I want to continue to use bare
libssl instead of libtls there.

So it would be desirable to change the libevent code in a way that
that allows a more flexible approach - instead of duplicating
everything all over again, libevent should allow to provide custom
read/write callbacks in a smarter way.

For example, there is be no reason to have bufferevent_add() static
and to duplicate the function everywhere (server_bufferevent_add()).
evbuffer_read/write() could use an optional callback instead of
read/write directly to allow plugging in tls_read/SSL_read etc.
Problematic is the handling of things like TLS_READ_AGAIN instead of
EAGAIN, but there should be a way to abstract it further.

Reyk

 Index: usr.sbin/syslogd/Makefile
 ===
 RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/Makefile,v
 retrieving revision 1.6
 diff -u -p -r1.6 Makefile
 --- usr.sbin/syslogd/Makefile 5 Oct 2014 18:14:01 -   1.6
 +++ usr.sbin/syslogd/Makefile 15 Jan 2015 13:26:09 -
 @@ -1,9 +1,9 @@
  #$OpenBSD: Makefile,v 1.6 2014/10/05 18:14:01 bluhm Exp $
  
  PROG=syslogd
 -SRCS=syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c
 +SRCS=syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c 
 evbuffer_tls.c
  MAN= syslogd.8 syslog.conf.5
 -LDADD=   -levent
 -DPADD=   ${LIBEVENT}
 +LDADD=   -levent -ltls -lssl -lcrypto
 +DPADD=   ${LIBEVENT} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}
  
  .include bsd.prog.mk
 Index: usr.sbin/syslogd/evbuffer_tls.c
 ===
 RCS file: usr.sbin/syslogd/evbuffer_tls.c
 diff -N usr.sbin/syslogd/evbuffer_tls.c
 --- /dev/null 1 Jan 1970 00:00:00 -
 +++ usr.sbin/syslogd/evbuffer_tls.c   15 Jan 2015 15:18:01 -
 @@ -0,0 +1,357 @@
 +/*   $OpenBSD$ */
 +
 +/*
 + * Copyright (c) 2002-2004 Niels Provos pro...@citi.umich.edu
 + * Copyright (c) 2014-2015 Alexander Bluhm bl...@openbsd.org
 + * All rights reserved.
 + *
 + * Redistribution and use in source and binary forms, with or without
 + * modification, are permitted provided that the following conditions
 + * are met:
 + * 1. Redistributions of source code must retain the above copyright
 + *notice, this list of conditions and the following disclaimer.
 + * 2. Redistributions in binary form must reproduce the above copyright
 + *notice, this list of conditions and the following disclaimer in the
 + *documentation and/or other materials provided with the distribution.
 + * 3. The name of the author may not be used to endorse or promote products
 + *derived from this software without specific prior written permission.
 + *
 + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
 + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
 + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
 + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
 + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
 + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
 + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY 

Re: syslog over TLS

2015-01-16 Thread Nicholas Marriott
Hi

WRT libevent - we have already added some ASR functions to libevent for
smtpd, I'd say libevent 1.4 is pretty much closed for new development
upstream - there won't be much to sync - and we have a port of 2.x for
ports to use. So I don't think there are strong reasons not to change
our libevent as we like


On Fri, Jan 16, 2015 at 10:00:46AM +0100, Reyk Floeter wrote:
 On Fri, Jan 16, 2015 at 01:46:09AM +0100, Alexander Bluhm wrote:
  Hi,
  
  This diff enables sending syslog messages over TLS.
  
  To implement the buffer layer, I have copied evbuffer.c from libevent
  and changed TCP to TLS where necessary.  This way I made a buffertls
  wrapper around bufferevent.  This might be integrated into libevent
  later.
  
  It still has some limitations:
  - No certificate validation.  This will get a bit tricky because
of privsep.
 
 Not that tricky - it has already been done and I intended to port it
 to libtls/libssl.  Let me see if I can create a diff quickly.
 
  - Wrong format.  The TLS RFC requires length-message encoding, I
use message-newline inherited from TCP.
  - Not all lost messages are logged.
  - At SIGHUP messages may get lost.
  - Man page is missing.  You can active it with @tls://ip-address.
  
  comment, test, ok?
  
  bluhm
  
 
 The implementation looks somewhat similar to the code in httpd/relayd
 that didn't copy the evbuffer code.  But instead of calling
 tls_read/tls_write directly, you implemented evtls_read/evtls_write.
 I wonder if this solves/improves anything in the daemon but I haven't
 seen any related issue.
 
 I'm fine with adding it to syslogd locally and the diff looks sane.
 
 But I'm not convinced that libevent is the right place to add a
 dependency on libtls.  I never pushed the SSL evbuffer wrappers back
 into libevent to keep portability and to avoid such a dependency; but
 I also saw that the implementation outside of libevent is a bit
 hackish because the evbuffers were never intended for anything except
 TCP.
 
 relayd could also benefit from it, but I want to continue to use bare
 libssl instead of libtls there.
 
 So it would be desirable to change the libevent code in a way that
 that allows a more flexible approach - instead of duplicating
 everything all over again, libevent should allow to provide custom
 read/write callbacks in a smarter way.
 
 For example, there is be no reason to have bufferevent_add() static
 and to duplicate the function everywhere (server_bufferevent_add()).
 evbuffer_read/write() could use an optional callback instead of
 read/write directly to allow plugging in tls_read/SSL_read etc.
 Problematic is the handling of things like TLS_READ_AGAIN instead of
 EAGAIN, but there should be a way to abstract it further.
 
 Reyk
 
  Index: usr.sbin/syslogd/Makefile
  ===
  RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/Makefile,v
  retrieving revision 1.6
  diff -u -p -r1.6 Makefile
  --- usr.sbin/syslogd/Makefile   5 Oct 2014 18:14:01 -   1.6
  +++ usr.sbin/syslogd/Makefile   15 Jan 2015 13:26:09 -
  @@ -1,9 +1,9 @@
   #  $OpenBSD: Makefile,v 1.6 2014/10/05 18:14:01 bluhm Exp $
   
   PROG=  syslogd
  -SRCS=  syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c
  +SRCS=  syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c 
  evbuffer_tls.c
   MAN=   syslogd.8 syslog.conf.5
  -LDADD= -levent
  -DPADD= ${LIBEVENT}
  +LDADD= -levent -ltls -lssl -lcrypto
  +DPADD= ${LIBEVENT} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}
   
   .include bsd.prog.mk
  Index: usr.sbin/syslogd/evbuffer_tls.c
  ===
  RCS file: usr.sbin/syslogd/evbuffer_tls.c
  diff -N usr.sbin/syslogd/evbuffer_tls.c
  --- /dev/null   1 Jan 1970 00:00:00 -
  +++ usr.sbin/syslogd/evbuffer_tls.c 15 Jan 2015 15:18:01 -
  @@ -0,0 +1,357 @@
  +/* $OpenBSD$ */
  +
  +/*
  + * Copyright (c) 2002-2004 Niels Provos pro...@citi.umich.edu
  + * Copyright (c) 2014-2015 Alexander Bluhm bl...@openbsd.org
  + * All rights reserved.
  + *
  + * Redistribution and use in source and binary forms, with or without
  + * modification, are permitted provided that the following conditions
  + * are met:
  + * 1. Redistributions of source code must retain the above copyright
  + *notice, this list of conditions and the following disclaimer.
  + * 2. Redistributions in binary form must reproduce the above copyright
  + *notice, this list of conditions and the following disclaimer in the
  + *documentation and/or other materials provided with the distribution.
  + * 3. The name of the author may not be used to endorse or promote products
  + *derived from this software without specific prior written permission.
  + *
  + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
  + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED 
  WARRANTIES
  + * OF 

Re: syslog over TLS

2015-01-16 Thread Reyk Floeter
On Fri, Jan 16, 2015 at 09:11:47AM +, Nicholas Marriott wrote:
 Hi
 
 WRT libevent - we have already added some ASR functions to libevent for
 smtpd, I'd say libevent 1.4 is pretty much closed for new development
 upstream - there won't be much to sync - and we have a port of 2.x for
 ports to use. So I don't think there are strong reasons not to change
 our libevent as we like
 

I didn't say that we cannot change libevent - this was different when
I first started using the evbuffer API for SSL in relayd.

I just don't think that adding a specific dependency for
libtls/libssl/libcrypto to libevent is the right direction.  We could
change the evbuffer API _in libevent_ to make it easier using it with
TCP, libssl, libssl or anything else instead of just TCP.  But without
adding any TLS code to libevent.

Reyk

 
 On Fri, Jan 16, 2015 at 10:00:46AM +0100, Reyk Floeter wrote:
  On Fri, Jan 16, 2015 at 01:46:09AM +0100, Alexander Bluhm wrote:
   Hi,
   
   This diff enables sending syslog messages over TLS.
   
   To implement the buffer layer, I have copied evbuffer.c from libevent
   and changed TCP to TLS where necessary.  This way I made a buffertls
   wrapper around bufferevent.  This might be integrated into libevent
   later.
   
   It still has some limitations:
   - No certificate validation.  This will get a bit tricky because
 of privsep.
  
  Not that tricky - it has already been done and I intended to port it
  to libtls/libssl.  Let me see if I can create a diff quickly.
  
   - Wrong format.  The TLS RFC requires length-message encoding, I
 use message-newline inherited from TCP.
   - Not all lost messages are logged.
   - At SIGHUP messages may get lost.
   - Man page is missing.  You can active it with @tls://ip-address.
   
   comment, test, ok?
   
   bluhm
   
  
  The implementation looks somewhat similar to the code in httpd/relayd
  that didn't copy the evbuffer code.  But instead of calling
  tls_read/tls_write directly, you implemented evtls_read/evtls_write.
  I wonder if this solves/improves anything in the daemon but I haven't
  seen any related issue.
  
  I'm fine with adding it to syslogd locally and the diff looks sane.
  
  But I'm not convinced that libevent is the right place to add a
  dependency on libtls.  I never pushed the SSL evbuffer wrappers back
  into libevent to keep portability and to avoid such a dependency; but
  I also saw that the implementation outside of libevent is a bit
  hackish because the evbuffers were never intended for anything except
  TCP.
  
  relayd could also benefit from it, but I want to continue to use bare
  libssl instead of libtls there.
  
  So it would be desirable to change the libevent code in a way that
  that allows a more flexible approach - instead of duplicating
  everything all over again, libevent should allow to provide custom
  read/write callbacks in a smarter way.
  
  For example, there is be no reason to have bufferevent_add() static
  and to duplicate the function everywhere (server_bufferevent_add()).
  evbuffer_read/write() could use an optional callback instead of
  read/write directly to allow plugging in tls_read/SSL_read etc.
  Problematic is the handling of things like TLS_READ_AGAIN instead of
  EAGAIN, but there should be a way to abstract it further.
  
  Reyk
  
   Index: usr.sbin/syslogd/Makefile
   ===
   RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/Makefile,v
   retrieving revision 1.6
   diff -u -p -r1.6 Makefile
   --- usr.sbin/syslogd/Makefile 5 Oct 2014 18:14:01 -   1.6
   +++ usr.sbin/syslogd/Makefile 15 Jan 2015 13:26:09 -
   @@ -1,9 +1,9 @@
#$OpenBSD: Makefile,v 1.6 2014/10/05 18:14:01 bluhm Exp $

PROG=syslogd
   -SRCS=syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c
   +SRCS=syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c 
   evbuffer_tls.c
MAN= syslogd.8 syslog.conf.5
   -LDADD=   -levent
   -DPADD=   ${LIBEVENT}
   +LDADD=   -levent -ltls -lssl -lcrypto
   +DPADD=   ${LIBEVENT} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}

.include bsd.prog.mk
   Index: usr.sbin/syslogd/evbuffer_tls.c
   ===
   RCS file: usr.sbin/syslogd/evbuffer_tls.c
   diff -N usr.sbin/syslogd/evbuffer_tls.c
   --- /dev/null 1 Jan 1970 00:00:00 -
   +++ usr.sbin/syslogd/evbuffer_tls.c   15 Jan 2015 15:18:01 -
   @@ -0,0 +1,357 @@
   +/*   $OpenBSD$ */
   +
   +/*
   + * Copyright (c) 2002-2004 Niels Provos pro...@citi.umich.edu
   + * Copyright (c) 2014-2015 Alexander Bluhm bl...@openbsd.org
   + * All rights reserved.
   + *
   + * Redistribution and use in source and binary forms, with or without
   + * modification, are permitted provided that the following conditions
   + * are met:
   + * 1. Redistributions of source code must retain the above copyright
   + *notice, this list of 

Re: syslog over TLS

2015-01-16 Thread Dongsheng Song
On Fri, Jan 16, 2015 at 8:46 AM, Alexander Bluhm
alexander.bl...@gmx.net wrote:

 - Wrong format.  The TLS RFC requires length-message encoding, I
   use message-newline inherited from TCP.

Transmission of Syslog Messages over TCP (RFC 6587) prefer use
'octet-counting', not 'non-transparent-framing method'.

http://tools.ietf.org/html/rfc6587#section-3.4
The older method of non-transparent-framing has problems.  The newer
method of octet-counting is reliable and has not been seen to cause
problems noted with the non-transparent-framing method.


I'd like plain TCP transmission implement 'octet-counting' too.



Re: syslog over TLS

2015-01-16 Thread Alexander Bluhm
On Fri, Jan 16, 2015 at 06:17:01PM +0800, Dongsheng Song wrote:
 On Fri, Jan 16, 2015 at 8:46 AM, Alexander Bluhm
 alexander.bl...@gmx.net wrote:
 
  - Wrong format.  The TLS RFC requires length-message encoding, I
use message-newline inherited from TCP.
 
 Transmission of Syslog Messages over TCP (RFC 6587) prefer use
 'octet-counting', not 'non-transparent-framing method'.
 
 http://tools.ietf.org/html/rfc6587#section-3.4
 The older method of non-transparent-framing has problems.  The newer
 method of octet-counting is reliable and has not been seen to cause
 problems noted with the non-transparent-framing method.

I know.  Implementing and testing non-transparent-framing was easier.
My goal was to get TCP, TLS and events working first.  Changing the
format can be done later.

 I'd like plain TCP transmission implement 'octet-counting' too.

There will be only one way for TCP and TLS as this code is shared.
In the end it will be octet-counting.

bluhm



Re: syslog over TLS

2015-01-16 Thread Alexander Bluhm
On Fri, Jan 16, 2015 at 01:46:09AM +0100, Alexander Bluhm wrote:
 This diff enables sending syslog messages over TLS.

Updated diff after sys/param.h commit.  Only some context changed.

bluhm

Index: usr.sbin/syslogd/Makefile
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- usr.sbin/syslogd/Makefile   5 Oct 2014 18:14:01 -   1.6
+++ usr.sbin/syslogd/Makefile   16 Jan 2015 11:45:40 -
@@ -1,9 +1,9 @@
 #  $OpenBSD: Makefile,v 1.6 2014/10/05 18:14:01 bluhm Exp $
 
 PROG=  syslogd
-SRCS=  syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c
+SRCS=  syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c evbuffer_tls.c
 MAN=   syslogd.8 syslog.conf.5
-LDADD= -levent
-DPADD= ${LIBEVENT}
+LDADD= -levent -ltls -lssl -lcrypto
+DPADD= ${LIBEVENT} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}
 
 .include bsd.prog.mk
Index: usr.sbin/syslogd/evbuffer_tls.c
===
RCS file: usr.sbin/syslogd/evbuffer_tls.c
diff -N usr.sbin/syslogd/evbuffer_tls.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ usr.sbin/syslogd/evbuffer_tls.c 16 Jan 2015 11:45:40 -
@@ -0,0 +1,357 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2002-2004 Niels Provos pro...@citi.umich.edu
+ * Copyright (c) 2014-2015 Alexander Bluhm bl...@openbsd.org
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ *derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include sys/types.h
+#include sys/time.h
+#include sys/ioctl.h
+
+#include errno.h
+#include event.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdarg.h
+#include tls.h
+
+#include evbuffer_tls.h
+
+/* prototypes */
+
+void bufferevent_read_pressure_cb(struct evbuffer *, size_t, size_t, void *);
+int evtls_read(struct evbuffer *, int, int, struct tls *);
+int evtls_write(struct evbuffer *, int, struct tls *);
+
+static int
+bufferevent_add(struct event *ev, int timeout)
+{
+   struct timeval tv, *ptv = NULL;
+
+   if (timeout) {
+   timerclear(tv);
+   tv.tv_sec = timeout;
+   ptv = tv;
+   }
+
+   return (event_add(ev, ptv));
+}
+
+static void
+buffertls_readcb(int fd, short event, void *arg)
+{
+   struct buffertls *buftls = arg;
+   struct bufferevent *bufev = buftls-bt_bufev;
+   struct tls *ctx = buftls-bt_ctx;
+   int res = 0;
+   short what = EVBUFFER_READ;
+   size_t len;
+   int howmuch = -1;
+
+   if (event == EV_TIMEOUT) {
+   what |= EVBUFFER_TIMEOUT;
+   goto error;
+   }
+
+   /*
+* If we have a high watermark configured then we don't want to
+* read more data than would make us reach the watermark.
+*/
+   if (bufev-wm_read.high != 0) {
+   howmuch = bufev-wm_read.high - EVBUFFER_LENGTH(bufev-input);
+   /* we might have lowered the watermark, stop reading */
+   if (howmuch = 0) {
+   struct evbuffer *buf = bufev-input;
+   event_del(bufev-ev_read);
+   evbuffer_setcb(buf,
+   bufferevent_read_pressure_cb, bufev);
+   return;
+   }
+   }
+
+   res = evtls_read(bufev-input, fd, howmuch, ctx);
+   switch (res) {
+   case TLS_READ_AGAIN:
+   event_set(bufev-ev_read, fd, EV_READ, buffertls_readcb,
+   buftls);
+   goto reschedule;
+   case TLS_WRITE_AGAIN:
+   

Re: syslog over TLS

2015-01-16 Thread Theo de Raadt
 I just don't think that adding a specific dependency for
 libtls/libssl/libcrypto to libevent is the right direction.

Let's not get hung up on how this code will fit into the libtls
picture yet.

It is very valuable that Alexander can get it the async tls case
perfected for the syslogd case, then we learn and eventually figure
out how to refactor it later.

You aren't the only one with concerns about the final picture..



Re: syslog over TLS

2015-01-16 Thread Reyk Floeter
On Fri, Jan 16, 2015 at 12:56:45PM +0100, Alexander Bluhm wrote:
 On Fri, Jan 16, 2015 at 01:46:09AM +0100, Alexander Bluhm wrote:
  This diff enables sending syslog messages over TLS.
 
 Updated diff after sys/param.h commit.  Only some context changed.
 
 bluhm
 

Despite my scepticism about putting it into libevent -

The diff looks and works fine (lightly tested).

The evbuffer_tls.c code is similar enough to what we have in
relayd/httpd and libevent;  so I don't see any problems here.

Minor comments (not inline):
- The ebuf[100] looks a bit weird, but it is not a problem.
- I would prefer checking return value as -1 instead of  0
  (tls_init() and other calls below).
- The manpage bits are missing.

I'm fine with putting it in and to improve/discuss the API issues later.

OK

Reyk

 Index: usr.sbin/syslogd/Makefile
 ===
 RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/Makefile,v
 retrieving revision 1.6
 diff -u -p -r1.6 Makefile
 --- usr.sbin/syslogd/Makefile 5 Oct 2014 18:14:01 -   1.6
 +++ usr.sbin/syslogd/Makefile 16 Jan 2015 11:45:40 -
 @@ -1,9 +1,9 @@
  #$OpenBSD: Makefile,v 1.6 2014/10/05 18:14:01 bluhm Exp $
  
  PROG=syslogd
 -SRCS=syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c
 +SRCS=syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c 
 evbuffer_tls.c
  MAN= syslogd.8 syslog.conf.5
 -LDADD=   -levent
 -DPADD=   ${LIBEVENT}
 +LDADD=   -levent -ltls -lssl -lcrypto
 +DPADD=   ${LIBEVENT} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}
  
  .include bsd.prog.mk
 Index: usr.sbin/syslogd/evbuffer_tls.c
 ===
 RCS file: usr.sbin/syslogd/evbuffer_tls.c
 diff -N usr.sbin/syslogd/evbuffer_tls.c
 --- /dev/null 1 Jan 1970 00:00:00 -
 +++ usr.sbin/syslogd/evbuffer_tls.c   16 Jan 2015 11:45:40 -
 @@ -0,0 +1,357 @@
 +/*   $OpenBSD$ */
 +
 +/*
 + * Copyright (c) 2002-2004 Niels Provos pro...@citi.umich.edu
 + * Copyright (c) 2014-2015 Alexander Bluhm bl...@openbsd.org
 + * All rights reserved.
 + *
 + * Redistribution and use in source and binary forms, with or without
 + * modification, are permitted provided that the following conditions
 + * are met:
 + * 1. Redistributions of source code must retain the above copyright
 + *notice, this list of conditions and the following disclaimer.
 + * 2. Redistributions in binary form must reproduce the above copyright
 + *notice, this list of conditions and the following disclaimer in the
 + *documentation and/or other materials provided with the distribution.
 + * 3. The name of the author may not be used to endorse or promote products
 + *derived from this software without specific prior written permission.
 + *
 + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
 + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
 + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
 + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
 + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
 + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
 + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 + */
 +
 +#include sys/types.h
 +#include sys/time.h
 +#include sys/ioctl.h
 +
 +#include errno.h
 +#include event.h
 +#include stdio.h
 +#include stdlib.h
 +#include string.h
 +#include stdarg.h
 +#include tls.h
 +
 +#include evbuffer_tls.h
 +
 +/* prototypes */
 +
 +void bufferevent_read_pressure_cb(struct evbuffer *, size_t, size_t, void *);
 +int evtls_read(struct evbuffer *, int, int, struct tls *);
 +int evtls_write(struct evbuffer *, int, struct tls *);
 +
 +static int
 +bufferevent_add(struct event *ev, int timeout)
 +{
 + struct timeval tv, *ptv = NULL;
 +
 + if (timeout) {
 + timerclear(tv);
 + tv.tv_sec = timeout;
 + ptv = tv;
 + }
 +
 + return (event_add(ev, ptv));
 +}
 +
 +static void
 +buffertls_readcb(int fd, short event, void *arg)
 +{
 + struct buffertls *buftls = arg;
 + struct bufferevent *bufev = buftls-bt_bufev;
 + struct tls *ctx = buftls-bt_ctx;
 + int res = 0;
 + short what = EVBUFFER_READ;
 + size_t len;
 + int howmuch = -1;
 +
 + if (event == EV_TIMEOUT) {
 + what |= EVBUFFER_TIMEOUT;
 + goto error;
 + }
 +
 + /*
 +  * If we have a high watermark configured then we don't want to
 +  * read more data than would make us reach the watermark.
 +  */
 + if (bufev-wm_read.high != 0) {
 + howmuch = bufev-wm_read.high - 

Re: syslog over TLS

2015-01-15 Thread Ted Unangst
On Fri, Jan 16, 2015 at 01:46, Alexander Bluhm wrote:
 Hi,
 
 This diff enables sending syslog messages over TLS.
 
 To implement the buffer layer, I have copied evbuffer.c from libevent
 and changed TCP to TLS where necessary.  This way I made a buffertls
 wrapper around bufferevent.  This might be integrated into libevent
 later.

I like the direction this is going, and I think it's in the right
place for now. We can learn from this, then decide how to incorporate
libtls and libevent later.



syslog over TLS

2015-01-15 Thread Alexander Bluhm
Hi,

This diff enables sending syslog messages over TLS.

To implement the buffer layer, I have copied evbuffer.c from libevent
and changed TCP to TLS where necessary.  This way I made a buffertls
wrapper around bufferevent.  This might be integrated into libevent
later.

It still has some limitations:
- No certificate validation.  This will get a bit tricky because
  of privsep.
- Wrong format.  The TLS RFC requires length-message encoding, I
  use message-newline inherited from TCP.
- Not all lost messages are logged.
- At SIGHUP messages may get lost.
- Man page is missing.  You can active it with @tls://ip-address.

comment, test, ok?

bluhm

Index: usr.sbin/syslogd/Makefile
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- usr.sbin/syslogd/Makefile   5 Oct 2014 18:14:01 -   1.6
+++ usr.sbin/syslogd/Makefile   15 Jan 2015 13:26:09 -
@@ -1,9 +1,9 @@
 #  $OpenBSD: Makefile,v 1.6 2014/10/05 18:14:01 bluhm Exp $
 
 PROG=  syslogd
-SRCS=  syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c
+SRCS=  syslogd.c ttymsg.c privsep.c privsep_fdpass.c ringbuf.c evbuffer_tls.c
 MAN=   syslogd.8 syslog.conf.5
-LDADD= -levent
-DPADD= ${LIBEVENT}
+LDADD= -levent -ltls -lssl -lcrypto
+DPADD= ${LIBEVENT} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}
 
 .include bsd.prog.mk
Index: usr.sbin/syslogd/evbuffer_tls.c
===
RCS file: usr.sbin/syslogd/evbuffer_tls.c
diff -N usr.sbin/syslogd/evbuffer_tls.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ usr.sbin/syslogd/evbuffer_tls.c 15 Jan 2015 15:18:01 -
@@ -0,0 +1,357 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2002-2004 Niels Provos pro...@citi.umich.edu
+ * Copyright (c) 2014-2015 Alexander Bluhm bl...@openbsd.org
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ *derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include sys/types.h
+#include sys/time.h
+#include sys/ioctl.h
+
+#include errno.h
+#include event.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdarg.h
+#include tls.h
+
+#include evbuffer_tls.h
+
+/* prototypes */
+
+void bufferevent_read_pressure_cb(struct evbuffer *, size_t, size_t, void *);
+int evtls_read(struct evbuffer *, int, int, struct tls *);
+int evtls_write(struct evbuffer *, int, struct tls *);
+
+static int
+bufferevent_add(struct event *ev, int timeout)
+{
+   struct timeval tv, *ptv = NULL;
+
+   if (timeout) {
+   timerclear(tv);
+   tv.tv_sec = timeout;
+   ptv = tv;
+   }
+
+   return (event_add(ev, ptv));
+}
+
+static void
+buffertls_readcb(int fd, short event, void *arg)
+{
+   struct buffertls *buftls = arg;
+   struct bufferevent *bufev = buftls-bt_bufev;
+   struct tls *ctx = buftls-bt_ctx;
+   int res = 0;
+   short what = EVBUFFER_READ;
+   size_t len;
+   int howmuch = -1;
+
+   if (event == EV_TIMEOUT) {
+   what |= EVBUFFER_TIMEOUT;
+   goto error;
+   }
+
+   /*
+* If we have a high watermark configured then we don't want to
+* read more data than would make us reach the watermark.
+*/
+   if (bufev-wm_read.high != 0) {
+   howmuch = bufev-wm_read.high - EVBUFFER_LENGTH(bufev-input);
+   /* we might have lowered the watermark, stop reading */
+   if (howmuch = 0) {
+   struct evbuffer *buf = bufev-input;
+   event_del(bufev-ev_read);
+