linux-kernel - Re: [PATCH net-next v24 18/23] ovpn: implement peer add/get/dump/delete via netlink

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z-E4i587M54Os5Yo@krikkit>
Date: Mon, 24 Mar 2025 11:48:43 +0100
From: Sabrina Dubroca <sd@...asysnail.net>
To: Antonio Quartulli <antonio@...nvpn.net>
Cc: netdev@...r.kernel.org, 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>, ryazanov.s.a@...il.com,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Simon Horman <horms@...nel.org>, linux-kernel@...r.kernel.org,
	linux-kselftest@...r.kernel.org, Xiao Liang <shaw.leon@...il.com>
Subject: Re: [PATCH net-next v24 18/23] ovpn: implement peer
 add/get/dump/delete via netlink

Hello Antonio,

A few questions wrt the API:

2025-03-18, 02:40:53 +0100, Antonio Quartulli wrote:
> +static bool ovpn_nl_attr_sockaddr_remote(struct nlattr **attrs,
> +					 struct sockaddr_storage *ss)
> +{
> +	struct sockaddr_in6 *sin6;
> +	struct sockaddr_in *sin;
> +	struct in6_addr *in6;
> +	__be16 port = 0;
> +	__be32 *in;
> +
> +	ss->ss_family = AF_UNSPEC;
> +
> +	if (attrs[OVPN_A_PEER_REMOTE_PORT])
> +		port = nla_get_be16(attrs[OVPN_A_PEER_REMOTE_PORT]);

What's the expected behavior if REMOTE_PORT isn't provided? We'll send
packets do port 0 (which I'm guessing will get dropped on the other
side) until we get a message from the peer and float sets the correct
port/address?


> +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
> +			       struct nlattr **attrs)
> +{
[...]
> +	/* when setting the keepalive, both parameters have to be configured */
> +	if (attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL] &&
> +	    attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT]) {
> +		interv = nla_get_u32(attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL]);
> +		timeout = nla_get_u32(attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT]);
> +		ovpn_peer_keepalive_set(peer, interv, timeout);

Should we interpret OVPN_A_PEER_KEEPALIVE_INTERVAL = 0 &&
OVPN_A_PEER_KEEPALIVE_TIMEOUT == 0 as "disable keepalive/timeout" on
an active peer?  And maybe "one set to 0, the other set to some
non-zero value" as invalid?  Setting either value to 0 doesn't seem
very useful (timeout = 0 will probably kill the peer immediately, and
I suspect interval = 0 would be quite spammy).

-- 
Sabrina