On Wed, Aug 21, 2019 at 3:45 PM Jeff Vander Stoep <jeffv@xxxxxxxxxx> wrote: > > MAC addresses are often considered 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_NEWLINK message type of a > netlink route socket[2]. Ideally we could grant/deny access to the > MAC address on a case-by-case basis without blocking the entire > RTM_NEWLINK message type which contains a lot of other useful > information. This can be achieved using a new LSM hook on the netlink > message receive path. Using this new hook, individual LSMs can select > which processes are allowed access to the real MAC, otherwise a > default value of zeros is returned. Offloading access control > decisions like this to an LSM is convenient because it preserves the > status quo for most Linux users while giving the various LSMs > flexibility to make finer grained decisions on access to sensitive > data based on policy. > > [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> > --- > include/linux/lsm_hooks.h | 8 ++++++++ > include/linux/security.h | 6 ++++++ > net/core/rtnetlink.c | 12 ++++++++++-- > security/security.c | 5 +++++ > 4 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index df1318d85f7d..dfcb2e11ff43 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -728,6 +728,12 @@ > * > * Security hooks for Netlink messaging. > * > + * @netlink_receive > + * Check permissions on a netlink message field before populating it. > + * @sk associated sock of task receiving the message. > + * @skb contains the sk_buff structure for the netlink message. > + * Return 0 if the data should be included in the message. > + * > * @netlink_send: > * Save security information for a netlink message so that permission > * checking can be performed when the message is processed. The security > @@ -1673,6 +1679,7 @@ union security_list_options { > int (*sem_semop)(struct kern_ipc_perm *perm, struct sembuf *sops, > unsigned nsops, int alter); > > + int (*netlink_receive)(struct sock *sk, struct sk_buff *skb); > int (*netlink_send)(struct sock *sk, struct sk_buff *skb); > > void (*d_instantiate)(struct dentry *dentry, struct inode *inode); > @@ -1952,6 +1959,7 @@ struct security_hook_heads { > struct hlist_head sem_associate; > struct hlist_head sem_semctl; > struct hlist_head sem_semop; > + struct hlist_head netlink_receive; > struct hlist_head netlink_send; > struct hlist_head d_instantiate; > struct hlist_head getprocattr; > diff --git a/include/linux/security.h b/include/linux/security.h > index 5f7441abbf42..46b5af6de59e 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -382,6 +382,7 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name, > char **value); > int security_setprocattr(const char *lsm, const char *name, void *value, > size_t size); > +int security_netlink_receive(struct sock *sk, struct sk_buff *skb); > int security_netlink_send(struct sock *sk, struct sk_buff *skb); > int security_ismaclabel(const char *name); > int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen); > @@ -1162,6 +1163,11 @@ static inline int security_setprocattr(const char *lsm, char *name, > return -EINVAL; > } > > +static inline int security_netlink_receive(struct sock *sk, struct sk_buff *skb) > +{ > + return 0; > +} > + > static inline int security_netlink_send(struct sock *sk, struct sk_buff *skb) > { > return 0; > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index 1ee6460f8275..7d69fcb8d22e 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -1650,8 +1650,16 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, > goto nla_put_failure; > > if (dev->addr_len) { > - if (nla_put(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr) || > - nla_put(skb, IFLA_BROADCAST, dev->addr_len, dev->broadcast)) > + if (skb->sk && security_netlink_receive(skb->sk, skb)) { > + if (!nla_reserve(skb, IFLA_ADDRESS, dev->addr_len)) > + goto nla_put_failure; Is populating the field with zeros the right approach or should I just omit it entirely? Even though this change will only impact LSM users I would still like to minimize the potential for breakage of userspace processes. Returning the same packet size and format seems like the least fragile thing to do. > > + > + } else { > + if (nla_put(skb, IFLA_ADDRESS, dev->addr_len, > + dev->dev_addr)) > + goto nla_put_failure; > + } > + if (nla_put(skb, IFLA_BROADCAST, dev->addr_len, dev->broadcast)) > goto nla_put_failure; > } > > diff --git a/security/security.c b/security/security.c > index 250ee2d76406..35c5929921b2 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1861,6 +1861,11 @@ int security_setprocattr(const char *lsm, const char *name, void *value, > return -EINVAL; > } > > +int security_netlink_receive(struct sock *sk, struct sk_buff *skb) > +{ > + return call_int_hook(netlink_receive, 0, sk, skb); > +} > + > int security_netlink_send(struct sock *sk, struct sk_buff *skb) > { > return call_int_hook(netlink_send, 0, sk, skb); > -- > 2.23.0.rc1.153.gdeed80330f-goog >
