Re: [Openvpn-devel] [PATCH] Rate-limit incoming P_CONTROL_HARD_RESET_* packets.
Hi, On Fri, Jun 24, 2022 at 01:15:05PM +0200, Arne Schwabe wrote: > > *This* patch won't apply anymore, but Arne said "we're now much faster > > in replying to packets than ever before" so we might indeed need a > > per-source-ip rate-limiter, to something like "10 per 10 seconds" or > > so (inventing arbitrary number that should be more than enough even > > for "5 users behind the same NAT reconnect at the same time", while > > at the same time too low to cause harm as a reflector) for the > > initial reply. > > Yeah. Keeping a per IP table is adding a lot of state to manage that. > Maybe instead to a (configurable) overall limit like 100/s? This would permit an attacker sending 1000 packets/s to an openvpn server to drown out 90% of all legitimate connection requests from everyone else... So while I see your argument about "please do not introduce state again, we just got rid of it", I'm not convinced we can get away so easily ;-) OTOH, 100/s inside OpenVPN, and good documentation how to offload the state to iptables... can we teach iptables to recognize CLIENT_RESET (or whatever they are) packets - only - and apply per-source rate-limiting there? iptables seems to be quite good at doing this for TCP SYNs already... 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
Re: [Openvpn-devel] [PATCH] Rate-limit incoming P_CONTROL_HARD_RESET_* packets.
Am 24.06.22 um 12:26 schrieb Gert Doering: Hi, On Fri, Jun 24, 2022 at 11:13:40AM +0200, Antonio Quartulli wrote: do we still need this patch after having merged Arne's HMAC feature? Yes and no. *This* patch won't apply anymore, but Arne said "we're now much faster in replying to packets than ever before" so we might indeed need a per-source-ip rate-limiter, to something like "10 per 10 seconds" or so (inventing arbitrary number that should be more than enough even for "5 users behind the same NAT reconnect at the same time", while at the same time too low to cause harm as a reflector) for the initial reply. Yeah. Keeping a per IP table is adding a lot of state to manage that. Maybe instead to a (configurable) overall limit like 100/s? Arne ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Rate-limit incoming P_CONTROL_HARD_RESET_* packets.
Hi, On Fri, Jun 24, 2022 at 11:13:40AM +0200, Antonio Quartulli wrote: > do we still need this patch after having merged Arne's HMAC feature? Yes and no. *This* patch won't apply anymore, but Arne said "we're now much faster in replying to packets than ever before" so we might indeed need a per-source-ip rate-limiter, to something like "10 per 10 seconds" or so (inventing arbitrary number that should be more than enough even for "5 users behind the same NAT reconnect at the same time", while at the same time too low to cause harm as a reflector) for the initial reply. 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
Re: [Openvpn-devel] [PATCH] Rate-limit incoming P_CONTROL_HARD_RESET_* packets.
Hi, do we still need this patch after having merged Arne's HMAC feature? Regards, -- Antonio Quartulli ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Rate-limit incoming P_CONTROL_HARD_RESET_* packets.
Hi, On 14/07/2019 11:45, Steffan Karger wrote: > Hi, > > On 06-12-18 18:10, Gert Doering wrote: >> Creation of new instances (= new incoming reset packets with different >> source IP addresses / source ports) can be rate-limited in the current >> code by use of the "--connect-freq" option. >> >> For packets sent with the same source port, OpenVPN would dilligently Did you mean "source IP"? or you really meant same port? Same IP/port should allow the server to match the request to an existing session, so I presume the packet would be ignored? (Maybe I am wrong) >> reply to every single packet, causing route reflection issues (plus >> using somewhat more server cycles). This patch introduces a timestamp >> in the tls_multi context which records when the last "reset" packet >> was seen, and ignores all further "reset" packets coming in in the >> next 10 second interval. > Is this patch something you are considering on top of "Stop state-exhaustion attacks from a single source address." ? Regards, > This makes sense, but why 10s? Our default connect-retry value is 5 > seconds. Wouldn't 5 (or 4?) be a better value in that regard? > >> Signed-off-by: Gert Doering >> --- >> src/openvpn/ssl.c| 14 ++ >> src/openvpn/ssl_common.h | 1 + >> 2 files changed, 15 insertions(+) >> >> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c >> index 74b88ce6..3078d76c 100644 >> --- a/src/openvpn/ssl.c >> +++ b/src/openvpn/ssl.c >> @@ -3468,6 +3468,20 @@ tls_pre_decrypt(struct tls_multi *multi, >> print_link_socket_actual(from, )); >> goto error; >> } >> + >> +/* only permit one is_hard_reset() packet per 10 seconds, >> + * ignore more frequent packets >> + */ >> +time_t now = time(NULL); >> +if ( now - multi->last_hard_reset_seen < 10 ) > > I think we use the "if (now - multi->last_hard_reset_seen < 10)" style > in the vast majority of the code base. > >> +{ >> +msg(D_MULTI_ERRORS, "TLS: tls_pre_decrypt: ignore >> incoming" >> +" '%s' (only %ds since last RESET)", >> +packet_opcode_name(op), >> +(int)(now - multi->last_hard_reset_seen) ); > > Here too, no space before ). > >> +goto error; >> +} >> +multi->last_hard_reset_seen = now; >> } >> >> /* >> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h >> index 7bf82b3a..e71696b5 100644 >> --- a/src/openvpn/ssl_common.h >> +++ b/src/openvpn/ssl_common.h >> @@ -515,6 +515,7 @@ struct tls_multi >> */ >> int n_hard_errors; /* errors due to TLS negotiation failure */ >> int n_soft_errors; /* errors due to unrecognized or >> failed-to-authenticate incoming packets */ >> +time_t last_hard_reset_seen;/* rate-limit incoming hard reset */ >> >> /* >> * Our locked common name, username, and cert hashes (cannot change >> during the life of this tls_multi object) >> > > Otherwise this looks good to me. I'll ACK once I understand your pick > for the timeout value. > > -Steffan > > > ___ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > -- Antonio Quartulli ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Rate-limit incoming P_CONTROL_HARD_RESET_* packets.
Hi, On 06-12-18 18:10, Gert Doering wrote: > Creation of new instances (= new incoming reset packets with different > source IP addresses / source ports) can be rate-limited in the current > code by use of the "--connect-freq" option. > > For packets sent with the same source port, OpenVPN would dilligently > reply to every single packet, causing route reflection issues (plus > using somewhat more server cycles). This patch introduces a timestamp > in the tls_multi context which records when the last "reset" packet > was seen, and ignores all further "reset" packets coming in in the > next 10 second interval. This makes sense, but why 10s? Our default connect-retry value is 5 seconds. Wouldn't 5 (or 4?) be a better value in that regard? > Signed-off-by: Gert Doering > --- > src/openvpn/ssl.c| 14 ++ > src/openvpn/ssl_common.h | 1 + > 2 files changed, 15 insertions(+) > > diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c > index 74b88ce6..3078d76c 100644 > --- a/src/openvpn/ssl.c > +++ b/src/openvpn/ssl.c > @@ -3468,6 +3468,20 @@ tls_pre_decrypt(struct tls_multi *multi, > print_link_socket_actual(from, )); > goto error; > } > + > +/* only permit one is_hard_reset() packet per 10 seconds, > + * ignore more frequent packets > + */ > +time_t now = time(NULL); > +if ( now - multi->last_hard_reset_seen < 10 ) I think we use the "if (now - multi->last_hard_reset_seen < 10)" style in the vast majority of the code base. > +{ > +msg(D_MULTI_ERRORS, "TLS: tls_pre_decrypt: ignore > incoming" > +" '%s' (only %ds since last RESET)", > +packet_opcode_name(op), > +(int)(now - multi->last_hard_reset_seen) ); Here too, no space before ). > +goto error; > +} > +multi->last_hard_reset_seen = now; > } > > /* > diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h > index 7bf82b3a..e71696b5 100644 > --- a/src/openvpn/ssl_common.h > +++ b/src/openvpn/ssl_common.h > @@ -515,6 +515,7 @@ struct tls_multi > */ > int n_hard_errors; /* errors due to TLS negotiation failure */ > int n_soft_errors; /* errors due to unrecognized or > failed-to-authenticate incoming packets */ > +time_t last_hard_reset_seen; /* rate-limit incoming hard reset */ > > /* > * Our locked common name, username, and cert hashes (cannot change > during the life of this tls_multi object) > Otherwise this looks good to me. I'll ACK once I understand your pick for the timeout value. -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel