Message-ID: <c933e2bf-b19c-4f8b-b2c0-44de50eb4141@openvpn.net>
Date: Thu, 21 Nov 2024 22:17:09 +0100
From: Antonio Quartulli <antonio@...nvpn.net>
To: Sergey Ryazanov <ryazanov.s.a@...il.com>, Andrew Lunn <andrew@...n.ch>
Cc: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Donald Hunter <donald.hunter@...il.com>,
 Shuah Khan <shuah@...nel.org>, sd@...asysnail.net, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next v11 05/23] ovpn: keep carrier always on

On 20/11/2024 23:56, Sergey Ryazanov wrote:
> On 15.11.2024 16:13, Antonio Quartulli wrote:
>> On 09/11/2024 02:11, Sergey Ryazanov wrote:
>>> On 29.10.2024 12:47, Antonio Quartulli wrote:
>>>> An ovpn interface will keep carrier always on and let the user
>>>> decide when an interface should be considered disconnected.
>>>>
>>>> This way, even if an ovpn interface is not connected to any peer,
>>>> it can still retain all IPs and routes and thus prevent any data
>>>> leak.
>>>>
>>>> Signed-off-by: Antonio Quartulli <antonio@...nvpn.net>
>>>> Reviewed-by: Andrew Lunn <andrew@...n.ch>
>>>> ---
>>>>   drivers/net/ovpn/main.c | 7 +++++++
>>>>   1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c
>>>> index 
>>>> eead7677b8239eb3c48bb26ca95492d88512b8d4..eaa83a8662e4ac2c758201008268f9633643c0b6 100644
>>>> --- a/drivers/net/ovpn/main.c
>>>> +++ b/drivers/net/ovpn/main.c
>>>> @@ -31,6 +31,13 @@ static void ovpn_struct_free(struct net_device *net)
>>>>   static int ovpn_net_open(struct net_device *dev)
>>>>   {
>>>> +    /* ovpn keeps the carrier always on to avoid losing IP or route
>>>> +     * configuration upon disconnection. This way it can prevent leaks
>>>> +     * of traffic outside of the VPN tunnel.
>>>> +     * The user may override this behaviour by tearing down the 
>>>> interface
>>>> +     * manually.
>>>> +     */
>>>> +    netif_carrier_on(dev);
>>>
>>> If a user cares about the traffic leaking, then he can create a 
>>> blackhole route with huge metric:
>>>
>>> # ip route add blackhole default metric 10000
>>>
>>> Why the network interface should implicitly provide this 
>>> functionality? And on another hand, how a routing daemon can learn a 
>>> topology change without indication from the interface?
>>
>> This was discussed loooong ago with Andrew. Here my last response:
>>
>> https://lore.kernel.org/all/d896bbd8-2709-4834-a637- 
>> f982fc51fc57@...nvpn.net/
> 
> Thank you for sharing the link to the beginning of the conversation. 
> Till the moment we have 3 topics regarding the operational state 
> indication:
> 1. possible absence of a conception of running state,
> 2. influence on routing protocol implementations,
> 3. traffic leaking.
> 
> As for conception of the running state, it should exists for tunneling 
> protocols with a state tracking. In this specific case, we can assume 
> interface running when it has configured peer with keys. The protocol 
> even has nice feature for the connection monitoring - keepalive.

What about a device in MP mode? It doesn't make sense to turn the 
carrier off when the MP node has no peers connected.
At the same time I don't like having P2P and MP devices behaving 
differently in this regard.
Therefore keeping the carrier on seemed the most logical way forward (at 
least for now - we can still come back to this once we have something 
smarter to implement).

> 
> Routing protocols on one hand could benefit from the operational state 
> indication. On another hand, hello/hold timer values mentioned in the 
> documentation are comparable with default routing protocols timers. So, 
> actual improvement is debatable.
> 
> Regarding the traffic leading, as I mentioned before, the blackhole 
> route or a firewall rule works better then implicit blackholing with a 
> non-running interface.
> 
> Long story short, I agree that we might not need a real operational 
> state indication now. Still protecting from a traffic leaking is not 
> good enough justification.

Well, it's the so called "persistent interface" concept in VPNs: leave 
everything as is, even if the connection is lost.
I know it can be implemented in many other different ways..but I don't 
see a real problem with keeping this way.

A blackhole/firewall can still be added if the user prefers (and not use 
the persistent interface).

Regards,

> 
> Andrew, what do you think? Is the traffic leaking prevention any good 
> justification or it needs to be updated?
> 
> -- 
> Sergey

-- 
Antonio Quartulli
OpenVPN Inc.

Powered by blists - more mailing lists