Re: [PATCH net 2/3] ovpn: explicitly reject netlink attr PEER_LOCAL_PORT in CMD_PEER_NEW/SET

[Sabrina Dubroca <sd@queasysnail.net>]

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@mandelbit.com>
> Closes: https://github.com/OpenVPN/ovpn-net-next/issues/19
> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

Either way I'm ok with this patch.

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

-- 
Sabrina