Re: [PATCH] selinux: map RTM_GETLINK to a privileged permission

[Paul Moore <paul@paul-moore.com>]

On Thu, Jan 16, 2020 at 9:27 AM Jeff Vander Stoep <jeffv@google.com> wrote:
> Persistent device identifiers like MAC addresses are sensitive
> because they are (usually) unique and can be used to
> identify/track a device or user [1]. The MAC address is
> accessible via the RTM_GETLINK request message type of a netlink
> route socket[2] which returns the RTM_NEWLINK message.
> Mapping RTM_GETLINK to a separate permission enables restricting
> access to the MAC address without changing the behavior for
> other RTM_GET* message types.
>
> [1] https://adamdrake.com/mac-addresses-udids-and-privacy.html
> [2] Other access vectors like ioctl(SIOCGIFHWADDR) are already covered
> by existing LSM hooks.
>
> Signed-off-by: Jeff Vander Stoep <jeffv@google.com>
> ---
>  security/selinux/include/classmap.h |  2 +-
>  security/selinux/include/security.h |  9 +++++++++
>  security/selinux/nlmsgtab.c         | 26 +++++++++++++++++++++++++-
>  security/selinux/ss/services.c      |  4 +++-
>  4 files changed, 38 insertions(+), 3 deletions(-)

...

> diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
> index c97fdae8f71b..aa7064a629a0 100644
> --- a/security/selinux/nlmsgtab.c
> +++ b/security/selinux/nlmsgtab.c
> @@ -25,7 +25,7 @@ struct nlmsg_perm {
>         u32     perm;
>  };
>
> -static const struct nlmsg_perm nlmsg_route_perms[] =
> +static struct nlmsg_perm nlmsg_route_perms[] =
>  {
>         { RTM_NEWLINK,          NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>         { RTM_DELLINK,          NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
> @@ -208,3 +208,27 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
>
>         return err;
>  }
> +
> +static void nlmsg_set_getlink_perm(u32 perm)
> +{
> +       int i;
> +
> +       for (i = 0; i < sizeof(nlmsg_route_perms)/sizeof(nlmsg_perm); i++) {
> +               if (nlmsg_route_perms[i].nlmsg_type == RTM_GETLINK) {
> +                       nlmsg_route_perms[i].perm = perm;
> +                       break;
> +               }
> +       }
> +}
> +
> +/**
> + * The value permission guarding RTM_GETLINK changes if nlroute_getlink
> + * policy capability is set.
> + */
> +void selinux_nlmsg_init(void)
> +{
> +       if (selinux_policycap_nlroute_getlink())
> +               nlmsg_set_getlink_perm(NETLINK_ROUTE_SOCKET__NLMSG_READPRIV);
> +       else
> +               nlmsg_set_getlink_perm(NETLINK_ROUTE_SOCKET__NLMSG_READ);
> +}

Two comments, with the first being rather trivial:

It might be nice to rename this to selinux_policycaps_init() or
something similar; that way we have some hope of collecting similar
policycaps related tweaks in one place.

Our current handling of netlink messages is rather crude, especially
when you consider the significance of the netlink messages and the
rather coarse granularity when compared to other SELinux object
classes.  I believe some (most? all?) of this is due to the number of
netlink messages compared to the maximum number of permissions in an
object class.  Back when xperms were added, one of the motivations for
making it a general solution was to potentially use them for netlink;
we obviously haven't made the change in the netlink controls, but I
think this might be the right time to do it.

--
paul moore
www.paul-moore.com