Re: [Openvpn-devel] proper configuring of "tls-verify"

2017-09-17 Thread Steffan Karger 
Hi,

On 11-09-17 15:23, Илья Шипицин wrote:
> 2017-09-11 17:30 GMT+05:00 David Sommerseth
>  >:
> 
> On 11/09/17 14:02, Илья Шипицин wrote:
> >
> >
> > 2017-09-11 16:54 GMT+05:00 Илья Шипицин  
> > >>:
> >
> >
> >
> > 2017-09-11 16:45 GMT+05:00 Jan Just Keijser  
> > >>:
> >
> > Hi,
> >
> > On 11/09/17 13:22, Илья Шипицин wrote:
> >
> > Hello,
> >
> > is someone actually using "tls-verify" in production ?
> > we tried to implement additional certificate check using
> > tls-verify
> >
> >
> > while it works in general, in case when it hits "exit
> 1", it
> > look like a timeout from client point of view. it is
> not any
> > good
> >
> >
> > do you mean that when a client is denied access (i.e. the
> > tls-verify script exits 1 on the server) that the client sees
> > this as a timeout?  that is "normal" behaviour, as the server
> > does not tell the client *WHY* access is refused - it simply
> > stop responding to a client that does not pass
> > authentication/authorization. The client will not hear
> from the
> > server, and will time out after a specified interval.  This is
> > actually the most secure way to do things, as a rogue client
> > cannot DoS a server this way.
> >
> >
> > I'd say it depends.
> >
> > we run a lot of openvpn-gui with real people sitting in front of
> > them, from their point of view it "oh, it does not work! fix it!"
> > in out case better UX is to deliver proper reason to the client
> >
> > for someone maybe the better UX is to keep silence
> >
> >
> >
> > what is wrong with timeout is endless retry.
> > there's no way to pass authentication once it failed, so why does
> client
> > have to retry ?
> 
> User-friendliness and security seldom walks hand-in-hand.  As this
> friendliness provides enough information fragments for an attacker to
> figure out "I need to try something else".  A non-responding server
> gives no clues.  It can be a crappy server or it can be access denied;
> the attacker doesn't know - thus making it harder to figure out what to
> do next.
> 
> 
> 
> an attacker can brute force a password, for example.
> but what do you mean "to try next" in case of ssl certificate ?
>  
> 
> 
> The client will by default try to reconnect, because that is what it in
> most cases is told to do when the server is unresponsive.  And since
> this happens with many seconds in between, a single client will not
> attempt to DoS a server by mistake by retrying in a too tight loop.
> 
> A failed authentication is a failed authentication.  Thus UX client
> front-ends could treat this silence like that - but also account for
> other types of connectivity issues.  If it should try to reconnect or
> not, well, that's entirely up to the configuration file.   There is
> --single-session which can be used to control this.
> 
> But for servers running OpenVPN clients, retrying indefinitely at
> 
> 
> if retry might be successful, yes.
> if authentication failed - no

As JJK already posted, this is indeed by design.  I haven't been around
long enough to know the exact reasons behind it.  My guess is that it's
partially a KISS thing - if we immediately abort the connection, we do
not have to worry about corner cases.  If we allow the connection to go
on, to e.g. send TLS alerts, we need to be more careful that we do not
allow anything else accident by too.

But judging from what I do know, I think it might be worth it to rely on
tls-auth/tls-crypt (and hopefully soon tls-crypt-v2) for DoS protection,
and improve the user experience by letting TLS send it's 'connection
rejected' and 'alert' messages.  But that would require some careful
thinking and code review and will certainly not happen before 2.5.

Finally, to preven confusion: this is not only about 'tls-verify', but
about TLS connection failures in general.  A 'tls-verify' failure should
behave just as any other TLS failure, and right now that is aborting the
connection immediately.

-Steffan

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list

Re: [Openvpn-devel] [PATCH 1/2] merge *-inline.h files with their main header

2017-09-17 Thread Steffan Karger 
Hi,

On 28-08-17 10:56, Antonio Quartulli wrote:
> *-inline.h files are not very useful anymore.
> In the attempt of cleaning up the code some more,
> merge them into their main header files.
> 
> No functional change is part of this patch.
> 
> Signed-off-by: Antonio Quartulli 
> ---
> 
> patch has been verified with travis-ci
> 
>  src/openvpn/Makefile.am |   8 +-
>  src/openvpn/forward-inline.h| 341 
> 
>  src/openvpn/forward.c   |   5 +-
>  src/openvpn/forward.h   | 319 -
>  src/openvpn/init.c  |   4 +-
>  src/openvpn/mtcp.c  |   2 +-
>  src/openvpn/mudp.c  |   2 +-
>  src/openvpn/multi.c |   4 +-
>  src/openvpn/occ-inline.h|  95 --
>  src/openvpn/occ.c   |   4 +-
>  src/openvpn/occ.h   |  61 +++
>  src/openvpn/openvpn.c   |   2 -
>  src/openvpn/openvpn.vcxproj |   4 -
>  src/openvpn/openvpn.vcxproj.filters |  12 --
>  src/openvpn/pf-inline.h |  63 ---
>  src/openvpn/pf.c|  24 ++-
>  src/openvpn/pf.h|  15 ++
>  src/openvpn/ping-inline.h   |  64 ---
>  src/openvpn/ping.c  |   1 -
>  src/openvpn/ping.h  |  37 
>  20 files changed, 465 insertions(+), 602 deletions(-)
>  delete mode 100644 src/openvpn/forward-inline.h
>  delete mode 100644 src/openvpn/occ-inline.h
>  delete mode 100644 src/openvpn/pf-inline.h
>  delete mode 100644 src/openvpn/ping-inline.h
> 
> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
> index fcc22d68..babc0adb 100644
> --- a/src/openvpn/Makefile.am
> +++ b/src/openvpn/Makefile.am
> @@ -55,7 +55,7 @@ openvpn_SOURCES = \
>   error.c error.h \
>   event.c event.h \
>   fdmisc.c fdmisc.h \
> - forward.c forward.h forward-inline.h \
> + forward.c forward.h \
>   fragment.c fragment.h \
>   gremlin.c gremlin.h \
>   helper.c helper.h \
> @@ -80,7 +80,7 @@ openvpn_SOURCES = \
>   mudp.c mudp.h \
>   multi.c multi.h \
>   ntlm.c ntlm.h \
> - occ.c occ.h occ-inline.h \
> + occ.c occ.h \
>   openssl_compat.h \
>   pkcs11.c pkcs11.h pkcs11_backend.h \
>   pkcs11_openssl.c \
> @@ -90,8 +90,8 @@ openvpn_SOURCES = \
>   otime.c otime.h \
>   packet_id.c packet_id.h \
>   perf.c perf.h \
> - pf.c pf.h pf-inline.h \
> - ping.c ping.h ping-inline.h \
> + pf.c pf.h \
> + ping.c ping.h \
>   plugin.c plugin.h \
>   pool.c pool.h \
>   proto.c proto.h \
> diff --git a/src/openvpn/forward-inline.h b/src/openvpn/forward-inline.h
> deleted file mode 100644
> index ab83ea40..
> --- a/src/openvpn/forward-inline.h
> +++ /dev/null
> @@ -1,341 +0,0 @@
> -/*
> - *  OpenVPN -- An application to securely tunnel IP networks
> - * over a single TCP/UDP port, with support for SSL/TLS-based
> - * session authentication and key exchange,
> - * packet encryption, packet authentication, and
> - * packet compression.
> - *
> - *  Copyright (C) 2002-2017 OpenVPN Technologies, Inc. 
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License version 2
> - *  as published by the Free Software Foundation.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License along
> - *  with this program; if not, write to the Free Software Foundation, Inc.,
> - *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> - */
> -
> -#ifndef FORWARD_INLINE_H
> -#define FORWARD_INLINE_H
> -
> -/*
> - * Inline functions
> - */
> -
> -/*
> - * Does TLS session need service?
> - */
> -static inline void
> -check_tls(struct context *c)
> -{
> -#if defined(ENABLE_CRYPTO)
> -void check_tls_dowork(struct context *c);
> -
> -if (c->c2.tls_multi)
> -{
> -check_tls_dowork(c);
> -}
> -#endif
> -}
> -
> -/*
> - * TLS errors are fatal in TCP mode.
> - * Also check for --tls-exit trigger.
> - */
> -static inline void
> -check_tls_errors(struct context *c)
> -{
> -#if defined(ENABLE_CRYPTO)
> -void check_tls_errors_co(struct context *c);
> -
> -void check_tls_errors_nco(struct context *c);
> -
> -if (c->c2.tls_multi && c->c2.tls_exit_signal)
> -{
> -if (link_socket_connection_oriented(c->c2.link_socket))
> -{
> -if (c->c2.tls_multi->n_soft_errors)
> -{
> -check_tls_errors_co(c);
> -}
> -}
> -else
> -