netdev - Re: [PATCH 6/6] netlink: add infrastructure to expose policies to userspace

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: <8e42bd1255ea5f4e713b3eb6c8a10b282c9d0b5a.camel@sipsolutions.net>
Date:   Fri, 26 Apr 2019 21:22:20 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     netdev@...r.kernel.org, David Ahern <dsa@...ulusnetworks.com>
Subject: Re: [PATCH 6/6] netlink: add infrastructure to expose policies to
 userspace

On Fri, 2019-04-26 at 20:21 +0200, Pablo Neira Ayuso wrote:
> On Fri, Apr 26, 2019 at 02:13:06PM +0200, Johannes Berg wrote:
> > diff --git a/include/uapi/linux/genetlink.h b/include/uapi/linux/genetlink.h
> > index 877f7fa95466..9c0636ec2286 100644
> > --- a/include/uapi/linux/genetlink.h
> > +++ b/include/uapi/linux/genetlink.h
> > @@ -48,6 +48,7 @@ enum {
> >  	CTRL_CMD_NEWMCAST_GRP,
> >  	CTRL_CMD_DELMCAST_GRP,
> >  	CTRL_CMD_GETMCAST_GRP, /* unused */
> > +	CTRL_CMD_GETPOLICY,
> 
> It would be good to single entry point to request descriptions, ie.
> have a NETLINK_DESC family for this. Thus, we could use the same
> program to pull for policy/command descriptions without updating an
> array that includes the command to get the policy _for each
> subsystem_.
> 
> The program to inquire for policy/command descriptions would be very
> much the same along time, no need for updates to include new command
> type for each subsystem.
> 
> It would just spin over NETLINK_DESC discovering subsystems ID that we
> support.

No objection to that. The only problem I think is that there's no
natural point to "hang" the policy data, mostly it's just used in the
nla_parse() or nla_validate() calls in the code. Basically, it's code-
driven, not data-driven like generic netlink.

Take IFLA_AF_SPEC for example. To validate that, we end up calling into
validate_link_af() which is defined in IPv4 and IPv6, rather than having
the inet_af_policy/inet6_af_policy available and doing it in the caller
(also, validate_link_af() does some additional validation, though for
IFLA_INET_CONF that can actually now be expressed as a nested policy
inside inet_af_policy, I believe).

So to really generalize that you'd have change this - at least as far as
the netlink attribute validation is concerned, not the extra code - to
be data driven, rather than coded.

Then you could use and expose that data pretty easily.

> In genetlink, I understand this can be exception if you prefer so, ie.
> I'll be fine with this CTRL_CMD_GETPOLICY if that makes it look nicer
> in terms of integration with the existing infrastructure. But for
> other netlink subsystems, NETLINK_DESC allows you to pull the
> description for genetlink itself, not the internal subsystems.

I think genetlink it really makes more sense this way - the genetlink
family it basically the introspection point already: it lets you
discover which families there are, which commands they support,
multicast groups they have etc.

It's also easier to deal with in userspace because you already need to
deal with the genetlink family itself (to get your family ID), so
interacting with another NETLINK_DESC family would be annoying.

However, if we wanted to generalize that I guess we could make
NETLINK_DESC able to cover generic netlink as well, providing two entry
points to the same information? Very small amount of code, I guess.

johannes