Re: syslog over TLS
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
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
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
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
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
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
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
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
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
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
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); +