On Thu, Nov 18, 2021 at 9:46 AM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
> On Tue, Nov 16, 2021 at 10:41 AM Stephen Smalley
> <stephen.smalley.work@xxxxxxxxx> wrote:
> >
> > On Wed, Nov 10, 2021 at 6:13 AM Bram Bonne <brambonne@xxxxxxxxxx> wrote:
> > >
> > > Reuse the existing extended permissions infrastructure to support
> > > sepolicy for different netlink message types.
> > >
> > > When individual netlink message types are omitted only the existing
> > > permissions are checked. As is the case for ioctl xperms, this feature
> > > is intended to provide finer granularity for nlmsg_read and nlmsg_write
> > > permissions, as they may be too imprecise. For example, a single
> > > NETLINK_ROUTE socket may provide access to both an interface's IP
> > > address and to its ARP table, which might have different privacy
> > > implications. In addition, the set of message types has grown over time,
> > > so even if the current list is acceptable, future additions might not be.
> > > It was suggested before on the mailing list [1] that extended permissions
> > > would be a good fit for this purpose.
> > >
> > > Existing policy using the nlmsg_read and nlmsg_write permissions will
> > > continue to work as-is. Similar to ioctl xperms, netlink xperms allow
> > > for a more fine-grained policy where needed.
> > >
> > > Example policy on Android, preventing regular apps from accessing the
> > > device's MAC address and ARP table, but allowing this access to
> > > privileged apps, looks as follows:
> > >
> > > allow netdomain self:netlink_route_socket {
> > > create read getattr write setattr lock append connect getopt
> > > setopt shutdown nlmsg_read
> > > };
> > > allowxperm netdomain self:netlink_route_socket nlmsg ~{
> > > RTM_GETLINK RTM_GETNEIGH RTM_GETNEIGHTBL
> > > };
> > > allowxperm priv_app self:netlink_route_socket nlmsg {
> > > RTM_GETLINK RTM_GETNEIGH RTM_GETNEIGHTBL
> > > };
> > >
> > > Android currently uses code similar to [1] as a temporary workaround to
> > > limit access to certain netlink message types; our hope is that this patch
> > > will allow us to move back to upstream code with an approach that works for
> > > everyone.
> > >
> > > [1] https://lore.kernel.org/selinux/CAHC9VhRSUhozBycHMZcMaJsibJDxNMsTsKVT2zOnW=5H4R4mdg@xxxxxxxxxxxxxx/
> > >
> > > Signed-off-by: Bram Bonne <brambonne@xxxxxxxxxx>
> > > ---
> > > security/selinux/hooks.c | 24 +++++++++++++++++++++++-
> > > security/selinux/ss/avtab.h | 1 +
> > > security/selinux/ss/services.c | 23 ++++++++++++++++++++++-
> > > 3 files changed, 46 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index e7ebd45ca345..a503865fabed 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -4662,6 +4662,28 @@ static int sock_has_perm(struct sock *sk, u32 perms)
> > > &ad);
> > > }
> > >
> > > +static int nlmsg_sock_has_extended_perms(struct sock *sk, u32 perms, u16 nlmsg_type)
> > > +{
> > > + struct sk_security_struct *sksec = sk->sk_security;
> > > + struct common_audit_data ad;
> > > + struct lsm_network_audit net = {0,};
> > > + u8 xperm;
> > > +
> > > + if (sksec->sid == SECINITSID_KERNEL)
> > > + return 0;
> > > +
> > > + ad.type = LSM_AUDIT_DATA_NET;
> > > + ad.u.net = &net;
> > > + ad.u.net->sk = sk;
> > > +
> > > + // nlmsg_types comfortably fit into a single driver, see RTM_MAX in uapi/linux/rtnetlink.h
> > > + xperm = nlmsg_type & 0xff;
> >
> > This seems like a dangerous assumption; obviously not all netlink
> > users are rtnetlink. Even if all existing netlink users follow this,
> > nothing prevents userspace from creating a netlink message that
> > violates it AFAIK, at which point you will just silently discard the
> > higher bits. If we think we can get away with this restriction, then
> > we need to enforce it here (i.e. return an error if they do not fit);
> > if not,
> > then we likely need to support multiple drivers with a simple mapping
> > of the upper bits to driver.
>
> Looks like generic netlink puts the family id into the message type
> field, with the actual command in the separate generic netlink header
> in the payload. generic netlink family ids appear to be dynamically
> allocated, with GENL_MAX_ID defined as 1023. genl-ctrl-list on a
> sample Linux system reports ids from 0x10 through 0x1f so those would
> fit but there isn't anything in the code to prevent higher ids from
> being allocated up to the max. And if someday you want to be able to
> filter generic netlink messages at the per-command level, you'd
> further need to deal with the separate cmd field.
There is also NETLINK_AUDIT which currently has message types defined
up to 2000. The netlink message header format allows for 16 bit
message types (look at the nlmsghdr struct) and I think it would be a
mistake if the SELinux netlink/xperms code didn't support a full 16
bits for the message type.
As far as NETLINK_GENERIC is concerned, yes, that's a bit of a
nuisance both with the dynamic family IDs and buried message types.
On the plus side, there are existing kernel functions that will
resolve the generic netlink family IDs to a genl_family struct but
those are currently private to the generic netlink code; I imagine if
there was a need those functions (or something similar) could be made
available outside of the genetlink.c. Once you've matched on
NETLINK_GENERIC and done the family resolution, it should just be a
matter of doing the generic netlink command lookup in the context of
the family, which really shouldn't be much different than looking up
the message type of a conventional netlink message.
--
paul moore
www.paul-moore.com
