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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aGzw2RqUP-yMaVFh@krikkit>
Date: Tue, 8 Jul 2025 12:20:09 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Donald Hunter <donald.hunter@...il.com>,
	Antonio Quartulli <antonio@...nvpn.net>, 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-07, 14:48:28 -0700, Jakub Kicinski wrote:
> On Thu, 3 Jul 2025 15:07:51 +0200 Sabrina Dubroca 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
> 
> There's no way to explicitly reject, because we expect that only what's
> needed will be listed (IOW we depend on NLA_UNSPEC rather than
> NLA_REJECT). It gets complicated at times but I think it should work
> here. Key mechanism is to define subsets of the nests:

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.


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:

    -
      name: peer-get
      attribute-set: ovpn
      flags: [admin-perm]
      doc: Retrieve data about existing remote peers (or a specific one)
      do:
        pre: ovpn-nl-pre-doit
        post: ovpn-nl-post-doit
        request:
          attributes:
            - ifindex
            - peer
        reply:
          attributes:
            - peer
      dump:
        request:
          attributes:
            - ifindex
        reply:
          attributes:
            - peer


-- 
Sabrina