netdev - Re: [PATCH net 2/3] ovpn: explicitly reject netlink attr PEER_LOCAL_PORT in CMD_PEER

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: <aGaApy-muPmgfGtR@krikkit>
Date: Thu, 3 Jul 2025 15:07:51 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Donald Hunter <donald.hunter@...il.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Antonio Quartulli <antonio@...nvpn.net>
Cc: netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Ralf Lici <ralf@...delbit.com>
Subject: Re: [PATCH net 2/3] ovpn: explicitly reject netlink attr
 PEER_LOCAL_PORT in CMD_PEER_NEW/SET

2025-07-03, 13:45:11 +0200, Antonio Quartulli wrote:
> The OVPN_A_PEER_LOCAL_PORT is designed to be a read-only attribute
> that ovpn sends back to userspace to show the local port being used
> to talk to that specific peer.

Seems like we'd want NLA_REJECT in the nla_policy instead of
NLA_POLICY_MIN, but my quick grepping in ynl and specs doesn't show
anything like that. Donald/Jakub, does it already exist? If not, does
it seem possible to extend the specs and ynl with something like:

name: local-port
type: reject(u16)

or maybe:

name: local-port
type: u16
checks:
  reject: true

(or maybe "read-only", and I don't know if yaml tolerates
"checks:\n reject" without the ": true". we can bikeshed the syntax
later :))


> However, we forgot to reject it when parsing CMD_PEER_NEW/SET messages.

Right :( Also a bunch of others: OVPN_A_PEER_SOCKET_NETNSID, all the
packets/bytes stats.

> This is not a critical issue because the incoming value is just
> ignored, but it may fool userspace which expects some change in
> behaviour.
> 
> Explicitly error out and send back a message if OVPN_A_PEER_LOCAL_PORT
> is specified in a CMD_PEER_NEW/SET message.
> 
> Reported-by: Ralf Lici <ralf@...delbit.com>
> Closes: https://github.com/OpenVPN/ovpn-net-next/issues/19
> Signed-off-by: Antonio Quartulli <antonio@...nvpn.net>

Either way I'm ok with this patch.

Reviewed-by: Sabrina Dubroca <sd@...asysnail.net>

-- 
Sabrina