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.
