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  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]
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

Powered by blists - more mailing lists