On Fri, Jan 17, 2020 at 1:32 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Thu, Jan 16, 2020 at 9:27 AM Jeff Vander Stoep <jeffv@xxxxxxxxxx> 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@xxxxxxxxxx> > > --- > > 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. That's a very large change with some unanswered questions - like how to handle generic netlink. I will have time later this year to make that change. In the meantime, this change is small (ideal for backporting) and consistent with how we differentiate between levels of sensitivity on netlink_audit messages. Would you consider taking v3 of this change with your suggested adjustment to selinux_policycaps_init()? (Apologies for the resend, gmail switched out of "plain text" mode so my initial response wasn't delivered to the mailing list). > > > -- > paul moore > www.paul-moore.com
