Re: [PATCH 0/5] Add heurestic and customizable value for VPN auth errors

[Jussi Laakkonen <jussi.laakkonen@jolla.com>]

Hi Daniel,

On 9/13/21 9:43 AM, Daniel Wagner wrote:
> Hi Jussi,
> 
> On Thu, Sep 02, 2021 at 06:11:19PM +0300, Jussi Laakkonen wrote:
>> Implement heuristic for auth error counter to avoid losing credentials
>> because a VPN server can report back AUTH_FAILED control message
>> unnecessarily. This seemed to be common with OpenVPN providers that
>> allow only one authentication at a time from one account only and until
>> a short leeway-time (usually 30s) after an improper disconnect the
>> credentials are allowed again.
>>
>> This is achieved by adding a per provider property "AuthErrorLimit" that
>> can be set by the user via D-Bus or by a VPN plugin. By default one (1)
>> auth error is allowed until credentials are re-requested (or cleared by
>> the VPN agent) if there was a successful connection made in the past
>> hour. The feature can be disabled by setting value 0 for
>> "AuthErrorLimit".
>>
>> This was noticed to happen in a scenario where an OpenVPN is connected
>> over UDP or TCP (with some providers this seems to happen quite
>> frequently on both):
>> 1. connect VPN over cellular with autoconnect set
>> 2. connection is switched to WiFi and let VPN to autoconnect,
>> 3. set cellular off and back on again (WiFi should be preferred here)
>> 4. disconnect WiFi and let VPN to autoconnect over cellular
>> 5. re-enable WiFi after which VPN server reports AUTH_FAILED to client
>> 6. next connection attempt re-requests VPN creds with error set in msg.
>> Usually this is enough to time the network disconnects in the sense that
>> OpenVPN binary cannot send the disconnect message back to server having
>> following in the system log:
>>      openvpn: event_wait : Interrupted system call (code=4)
>>      openvpn: SIGTERM received, sending exit notification to peer
>>      openvpn: write UDP: Network is unreachable (code=101)
>>      openvpn: Closing TUN/TAP interface
>>
>> As a solution each provider sets a run-time only previous_connect_time
>> using the monotonic boot time clock whenever the connection has been
>> successful. This, in conjunction with a limit for allowed authentication
>> errors is used to determine whether the actual authentication error count
>> is reported back to the VPN agent or by returning 0 attempt again with
>> the current credentials. After a connection has been made all errors are
>> cleared. If the connection abruptly goes away, e.g., the server is lost
>> only the conn_error_counter is increased and previous_connect_time is
>> cleared but this does not trigger clearing of the authentication error
>> and, provided the credentials are still valid, the next attempt succeeds
>> when the server is again reachable. Same applies for a proper
>> disconnect. Thus, this solution does not interfere with normal cases but
>> mitigates unwanted credential loss in case if the VPN server determines
>> connection limit being full and reports back with an authentication error
>> message.
>>
>> OpenVPN sets this value by default to 10 unless already defined and can
>> be disabled by setting value zero (0). Some VPNs may not require this
>> option and that is why lenience of one (1) auth error is allowed by
>> default.
>>
>> Also change to report back EALREADY in case a VPN is attempted to be
>> disconneced when already disconnecting. It shouldn't be treated as an
>> error.
> 
> Thanks again for the extensive documentation. I've applied the series. I
> assume this has been in production for a while, so it supposed to work
> :) And it doesn't look too complex to figure out what's going on. So
> let's see if there are any regression reports (which I don't expect)
> 

You're welcome. These kind of odd issues that do not happen that often 
do really need a long explanation. If the fix to odd issue then has some 
issues itself these kind of stuff in commit messages help a lot I bet.

Yes we've been running this for a while now. Wider test comes when the 
new release comes out. It may be that the 1 hour limit could be adjusted 
downwards in the future but wider results show (or, when there are no 
more complaints - the origin of this was a report from an user) what are 
the most optimal limits. That is why the attempt count is an user 
configurable value. I'll setup new patches if needed.

Cheers,
  Jussi