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 <g...@greenie.muc.de>
> ---
> 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, &gc));
> 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
