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

Jakub Kicinski
2025-07-08T14:47:06Z

On Tue, 8 Jul 2025 12:20:09 +0200 Sabrina Dubroca wrote:
> Ok, I see. It's a bit verbose, especially with the nest, but adding a
> reject here and there as I was suggesting wouldn't work for per-op
> policies.

Right, it's a tricky problem to solve :(
Really, the best time to address it is when family is designed.
Even folks quite familiar with netlink make the mistake of treating
nesting as a cute way of grouping related attributes.
It is really, really counter productive to use it like that, nesting
has major drawbacks.
ethtool nesting may seem "inverted", but it's a good example of nesting
used _correctly_.

> In ovpn we should also reject attributes from GET and DEL that aren't
> currently used to match the peer we want to get/delete (ie everything
> except PEER_ID), while still being able to parse all possible peer
> attributes from the kernel's reply (only for GET). So I guess we'd
> want a different variant of the nested attribute "peer" for the
> request and reply here:

Yes, that's hard to the point of probably not being worth fixing
at the spec level? :( We could so something like:

--- a/Documentation/netlink/specs/ovpn.yaml
+++ b/Documentation/netlink/specs/ovpn.yaml
@@ -265,6 +265,11 @@ doc: Netlink protocol to control OpenVPN network devices
type: nest
doc: Peer specific cipher configuration
nested-attributes: keyconf
+ -
+ name: peer-input
+ type: nest
+ nested-attributes: peer-input
+ value: 2
-
name: ovpn-peer-input
subset-of: ovpn

but the codegen today will output this "fake" attribute into the uAPI
which we don't need.

In any case. I think what I suggested is slightly better than
opencoding, even if verbose :) So I set the patches to Changes
Requested..