Message-ID: <e2caab8a-343e-4728-b5a7-b167f05c9bb9@gmail.com>
Date: Thu, 21 Nov 2024 00:56:27 +0200
From: Sergey Ryazanov <ryazanov.s.a@...il.com>
To: Antonio Quartulli <antonio@...nvpn.net>, 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 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.

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.

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

--
Sergey

Powered by blists - more mailing lists