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 for Android: free password hash cracker in your pocket

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

Message-ID: <cddf44e2d6c86c2eba7f5e0e7145668295060341.camel@sipsolutions.net>
Date:   Sun, 28 Apr 2019 22:03:34 +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 Sat, 2019-04-27 at 13:25 +0200, Pablo Neira Ayuso wrote:
> 
> > 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.
> 
> I see, agreed, we would need to rework this to make data driven, so
> this nest:
> 
> IFLA_AF_SPEC (nest)
>   family = AF_INET6 (nest)
>     IFLA_INET6_...
> 
> IFLA_AF_SPEC (nest)
>   family = AF_INET4 (nest)
>     IFLA_INET_...
> 
> needs a nla_policy definition for each family.

Right.

> We can do this rework progressively, as we start exposing description
> though the list of nla_policy structure for each subsystem.

Sure, of course.

I'm just saying it's not easy right now to provide that, as all of this
is mostly code-driven today. You have
	rtnl_register(PF_UNSPEC, RTM_NEWLINK, rtnl_newlink, NULL, 0);

but the actual parsing happens WAY down in the code, multiple levels of
functions deep that are sometimes shared etc.

I'm not even sure how we could express this in the policy - right now we
only have
	[ILFA_AF_SPEC] = NLA_POLICY_NESTED(xyz_policy)
but that cannot capture the fact that the inner attributes depend on
another attribute or some other circumstance.

Oh, actually, it's not hard in this case because the AF is prescribed by
the type, so we'd need something like

struct nla_policy af_spec_policy[...] = {
	[AF_INET] = NLA_POLICY_NESTED(af_inet_policy),
	[AF_INET6] = NLA_POLICY_NESTED(af_inet6_policy),
	// .. also for MPLS
};
struct nla_policy ifla_policy[...] = {
	[IFLA_AF_SPEC] = NLA_POLICY_NESTED(af_spec_policy),
};

So it's actually easy to do and we'd just need to change the
rtnl_register() to something like rtnl_register_with_policy() or
something like that, and then we'd have all the data we want directly
available and would actually save quite a bit of code, including
possibly the whole validate_link_af() method pointer.

johannes