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
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.

"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

Attachment: signature.asc
Description: PGP signature

Openvpn-devel mailing list

Reply via email to