Re: [Openvpn-devel] [PATCH v2] Add connect-freq-initial option to limit initial connection responses

2023-01-09 Thread Frank Lichtenheld
On Fri, Jan 06, 2023 at 03:38:41PM +0100, Arne Schwabe wrote:
> This limits the nubmer of packets OpenVPN will respond to. This avoid
> OpenVPN server being abused for refelection attacks in a large scale
> as we gotten a lot more efficient with the cookie approach in our
> initial connection approach.
> 
> The defaults of 100 attempts per 10s should work for most people,
> esepcially since completed three way handshakes are not counted. So
> the default will throttle connection attempts on server with high packet
> loss or that are actually under a DOS.
> 
> The 100 per 10s are similar in size to the old 2.5 and earlier behaviour
> where every initial connection attempt would take up a slot of the
> max-clients sessions and those would only expire after the TLS timeout.
> This roughly translates to 1024 connection attempts in 60s on an
> empty server.
> 
[...]
> diff --git a/doc/man-sections/server-options.rst 
> b/doc/man-sections/server-options.rst
> index 99263fff3..cbb2c3c92 100644
> --- a/doc/man-sections/server-options.rst
> +++ b/doc/man-sections/server-options.rst
> @@ -184,6 +184,25 @@ fast hardware. SSL/TLS authentication must be used in 
> this mode.
>For the best protection against DoS attacks in server mode, use
>``--proto udp`` and either ``--tls-auth`` or ``--tls-crypt``.

I think we should add a sentence about the relationship between connect-freq and
connect-freq-initial. Because users will wonder. Specifically are there attacks
that connect-freq protects against that connect-freq-initial does not?

>  
> +--connect-freq-initial args
> +  (UDP only) Allow a maximum of ``n`` initial connection packet responses
> +  per ``sec`` seconds from to clients.

Remove "from"?

> +
> +  Valid syntax:
> +  ::
> +
> + connect-freq-initial n sec
> +
> +  OpenVPN starting at 2.6 is very efficient in responding to initial
> +  connection packets. When not limiting the initial responses
> +  an OpenVPN daemon can be abused in reflection attacks.
> +  This option is designed to limit the rate OpenVPN will respond to initial
> +  attacks.
> +
> +  Connection attempts that complete the initial three-way handshake
> +  will not be counted against the limit. The default is to allow
> +  100 initial connection per 10s.
> +
>  --duplicate-cn
>Allow multiple clients with the same common name to concurrently
>connect. In the absence of this option, OpenVPN will disconnect a client
[...]
> diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
> index c27c6da5b..4ef2f355b 100644
> --- a/src/openvpn/mudp.c
> +++ b/src/openvpn/mudp.c
> @@ -82,6 +82,16 @@ do_pre_decrypt_check(struct multi_context *m,
>  struct openvpn_sockaddr *from = >top.c2.from.dest;
>  int handwindow = m->top.options.handshake_window;
>  
> +if (verdict == VERDICT_VALID_RESET_V3 || verdict == 
> VERDICT_VALID_RESET_V2)
> +{
> +/* Check if we are still below our limit for sending out
> + * responses */
> +if (!reflect_filter_rate_limit_check(m->initial_rate_limiter))
> +{
> +return false;
> +}
> +}
> +
>  if (verdict == VERDICT_VALID_RESET_V3)
>  {
>  /* Extract the packet id to check if it has the special format that
> @@ -244,6 +254,10 @@ multi_get_create_instance_udp(struct multi_context *m, 
> bool *floated)
>  
>  if (frequency_limit_event_allowed(m->new_connection_limiter))
>  {
> +/* a successful three-way handshake only counts against
> + * connect-freq but not against connect-req-initial */

"connect-freq-initial"

> +
> reflect_filter_rate_limit_decrease(m->initial_rate_limiter);
> +
>  mi = multi_create_instance(m, );
>  if (mi)
>  {
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index 0ba509fa0..1a7f278f7 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -52,6 +52,7 @@
>  #include "crypto_backend.h"
>  #include "ssl_util.h"
>  #include "dco.h"
> +#include "reflect_filter.h"
>  
>  /*#define MULTI_DEBUG_EVENT_LOOP*/
>  
> @@ -368,6 +369,8 @@ multi_init(struct multi_context *m, struct context *t, 
> bool tcp_mode)
>   */
>  m->new_connection_limiter = frequency_limit_init(t->options.cf_max,
>   t->options.cf_per);
> +m->initial_rate_limiter = 
> initial_rate_limit_init(t->options.cf_initial_max,
> +  
> t->options.cf_initial_per);
>  
>  /*
>   * Allocate broadcast/multicast buffer list
> @@ -729,6 +732,7 @@ multi_uninit(struct multi_context *m)
>  mbuf_free(m->mbuf);
>  ifconfig_pool_free(m->ifconfig_pool);
>  frequency_limit_free(m->new_connection_limiter);
> +initial_rate_limit_free(m->initial_rate_limiter);
>  multi_reap_free(m->reaper);
>  mroute_helper_free(m->route_helper);
> 

Re: [Openvpn-devel] [PATCH v2] Add connect-freq-initial option to limit initial connection responses

2023-01-09 Thread Gert Doering
Hi,

On Fri, Jan 06, 2023 at 03:38:41PM +0100, Arne Schwabe wrote:
> Patch v2: use strtol instead of atoi to be able to differentiate between
>   an error parsing and parsing 0. Use int64_t instead int to
>   avoid overflow errors.

I find this easier to read, so thanks.

This said...

> +/* Check if we are still below our limit for sending out
> + * responses */
> +if (!reflect_filter_rate_limit_check(m->initial_rate_limiter))
> +{
> +return false;
> +}

This does "we kill the packet if reflect_filter returns false(!)".

[..]
> +bool
> +reflect_filter_rate_limit_check(struct initial_packet_rate_limit *irl)
> +{
[..]
> +return irl->curr_period_counter > irl->max_per_period;
> +}

This does "return false *while under the limit*".

So it kills all UDP for me.

2023-01-09 09:03:01 us=694540 rrl: curr_period_counter=1, max_pp=100
2023-01-09 09:03:01 us=694598 connection killed by reflect_filter
2023-01-09 09:03:17 us=494917 rrl: curr_period_counter=1, max_pp=100
2023-01-09 09:03:17 us=495003 connection killed by reflect_filter
2023-01-09 09:03:22 us=773712 rrl: curr_period_counter=2, max_pp=100
2023-01-09 09:03:22 us=773773 connection killed by reflect_filter
2023-01-09 09:03:25 us=243757 rrl: curr_period_counter=3, max_pp=100
2023-01-09 09:03:25 us=243827 connection killed by reflect_filter


Turning around the condition in reflect_filter.c

 +return irl->curr_period_counter < irl->max_per_period;

makes things well-behaved - as in, normal connections succeed, excess
RESET_V3 fail - tested with your sendOvpn.py

$ python3.9 sendOvpn.py 1000
generating packets
creating sockets
Round 0
sending packets
sleeping
99 packets received

2023-01-09 09:22:47 us=429929 Connection Attempt Dropped 900 initial handshake 
packets due to --connect-freq-initial 100 10


While at it, I wonder if it would be a useful addition to do a single
message if the rate limiter kicks in, like this

if ( irl->curr_period_counter == irl->max_per_period )
{
msg( M_WARN, "connect-freq-initial rate limiter activated, dropping 
incoming UDP for the next %d seconds", 
(irl->last_period_reset + irl->period_length)-now );
}

we do have a log entry after the end of the period, but that might be an
arbitrary time ("when the next packet comes in") further down the log,
with no indication why OpenVPN decided to play dead at this point.

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v2] Add connect-freq-initial option to limit initial connection responses

2023-01-06 Thread Arne Schwabe
This limits the nubmer of packets OpenVPN will respond to. This avoid
OpenVPN server being abused for refelection attacks in a large scale
as we gotten a lot more efficient with the cookie approach in our
initial connection approach.

The defaults of 100 attempts per 10s should work for most people,
esepcially since completed three way handshakes are not counted. So
the default will throttle connection attempts on server with high packet
loss or that are actually under a DOS.

The 100 per 10s are similar in size to the old 2.5 and earlier behaviour
where every initial connection attempt would take up a slot of the
max-clients sessions and those would only expire after the TLS timeout.
This roughly translates to 1024 connection attempts in 60s on an
empty server.

OpenVPN will announce once per period how many packets it dropped:

Dropped 11 initial handshake packets due to --connect-freq-initial 100 10

to inform an amdin about the consequence of this feature.

Patch v2: use strtol instead of atoi to be able to differentiate between
  an error parsing and parsing 0. Use int64_t instead int to
  avoid overflow errors.

Signed-off-by: Arne Schwabe 
---
 Changes.rst |  4 ++
 doc/man-sections/server-options.rst | 19 ++
 src/openvpn/Makefile.am |  1 +
 src/openvpn/mudp.c  | 14 +
 src/openvpn/multi.c |  4 ++
 src/openvpn/multi.h |  2 +
 src/openvpn/options.c   | 21 +++
 src/openvpn/options.h   |  5 ++
 src/openvpn/reflect_filter.c| 93 +
 src/openvpn/reflect_filter.h| 71 ++
 10 files changed, 234 insertions(+)
 create mode 100644 src/openvpn/reflect_filter.c
 create mode 100644 src/openvpn/reflect_filter.h

diff --git a/Changes.rst b/Changes.rst
index 47933ae09..187d03fcf 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -92,6 +92,10 @@ Cookie based handshake for UDP server
 shake. The tls-crypt-v2 option allows controlling if older clients are
 accepted.
 
+By default the rate of initial packet responses is limited to 100 per 10s
+interval to avoid OpenVPN servers being abused in reflection attacks
+(see ``--connect-freq-initial``).
+
 Data channel offloading with ovpn-dco
 2.6.0+ implements support for data-channel offloading where the data 
packets
 are directly processed and forwarded in kernel space thanks to the ovpn-dco
diff --git a/doc/man-sections/server-options.rst 
b/doc/man-sections/server-options.rst
index 99263fff3..cbb2c3c92 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -184,6 +184,25 @@ fast hardware. SSL/TLS authentication must be used in this 
mode.
   For the best protection against DoS attacks in server mode, use
   ``--proto udp`` and either ``--tls-auth`` or ``--tls-crypt``.
 
+--connect-freq-initial args
+  (UDP only) Allow a maximum of ``n`` initial connection packet responses
+  per ``sec`` seconds from to clients.
+
+  Valid syntax:
+  ::
+
+ connect-freq-initial n sec
+
+  OpenVPN starting at 2.6 is very efficient in responding to initial
+  connection packets. When not limiting the initial responses
+  an OpenVPN daemon can be abused in reflection attacks.
+  This option is designed to limit the rate OpenVPN will respond to initial
+  attacks.
+
+  Connection attempts that complete the initial three-way handshake
+  will not be counted against the limit. The default is to allow
+  100 initial connection per 10s.
+
 --duplicate-cn
   Allow multiple clients with the same common name to concurrently
   connect. In the absence of this option, OpenVPN will disconnect a client
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index e80a35abd..35d60a65b 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -113,6 +113,7 @@ openvpn_SOURCES = \
ps.c ps.h \
push.c push.h \
pushlist.h \
+   reflect_filter.c reflect_filter.h \
reliable.c reliable.h \
route.c route.h \
run_command.c run_command.h \
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index c27c6da5b..4ef2f355b 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -82,6 +82,16 @@ do_pre_decrypt_check(struct multi_context *m,
 struct openvpn_sockaddr *from = >top.c2.from.dest;
 int handwindow = m->top.options.handshake_window;
 
+if (verdict == VERDICT_VALID_RESET_V3 || verdict == VERDICT_VALID_RESET_V2)
+{
+/* Check if we are still below our limit for sending out
+ * responses */
+if (!reflect_filter_rate_limit_check(m->initial_rate_limiter))
+{
+return false;
+}
+}
+
 if (verdict == VERDICT_VALID_RESET_V3)
 {
 /* Extract the packet id to check if it has the special format that
@@ -244,6 +254,10 @@ multi_get_create_instance_udp(struct multi_context *m, 
bool *floated)