| Message ID | 20190821134547.96929-1-jeffv@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] rtnetlink: gate MAC address with an LSM hook | expand |
On Wed, Aug 21, 2019 at 3:45 PM Jeff Vander Stoep <jeffv@google.com> 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@google.com> > --- > 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 >
On 8/21/2019 6:45 AM, Jeff Vander Stoep 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. Is the MAC address the only bit of skb data that you might want to control with MAC? ( Sorry, couldn't help it ;) ) Just musing, but might it make more sense to leave the core code unmodified and clear the MAC address in the skb inside the LSM? If you did it that way you could address any other data you want to control using the same hook. I would hate to see separate LSM hooks for each of several bits of data. On the other hand, I wouldn't want you to violate any layering policies in the networking code. That would be wrong. > > [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> > --- > 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; > + > + } 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);
On Wed, Aug 21, 2019 at 4:34 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > On 8/21/2019 6:45 AM, Jeff Vander Stoep 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. > > Is the MAC address the only bit of skb data that you might > want to control with MAC? ( Sorry, couldn't help it ;) ) > Just musing, but might it make more sense to leave the core > code unmodified and clear the MAC address in the skb inside > the LSM? If you did it that way you could address any other > data you want to control using the same hook. I would hate > to see separate LSM hooks for each of several bits of data. > On the other hand, I wouldn't want you to violate any layering > policies in the networking code. That would be wrong. I considered that approach, but having the LSM modifying the skb like that without the networking code's knowledge did seem like a layering violation, and fragile. It's also different than how LSM hooks typically operate - generally they return decisions and the calling code is responsible for taking appropriate action. I'm currently only interested in the MAC, but this approach can be extended to other fields. The selinux patch just splits up the read permission into two levels, privileged and unprivileged which is consistent with how netlink audit sockets are handled. > > > > > [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> > > --- > > 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; > > + > > + } 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); >
From: Jeff Vander Stoep <jeffv@google.com> Date: Wed, 21 Aug 2019 15:45:47 +0200 > 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@google.com> I'm sure the MAC address will escape into userspace via other means, dumping pieces of networking config in other contexts, etc. I mean, if I can get a link dump, I can dump the neighbor table as well. I kinda think this is all very silly whack-a-mole kind of stuff, to be quite honest. And like others have said, tomorrow you'll be like "oh crap, we should block X too" and we'll get another hook, another config knob, another rulset update, etc.
On Fri, Aug 23, 2019 at 1:19 AM David Miller <davem@davemloft.net> wrote: > > From: Jeff Vander Stoep <jeffv@google.com> > Date: Wed, 21 Aug 2019 15:45:47 +0200 > > > 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@google.com> > > I'm sure the MAC address will escape into userspace via other means, > dumping pieces of networking config in other contexts, etc. I mean, > if I can get a link dump, I can dump the neighbor table as well. These are already gated by existing LSM hooks and capability checks. They are not allowed on mandatory access control systems unless explicitly granted. > > I kinda think this is all very silly whack-a-mole kind of stuff, to > be quite honest. We evaluated mechanisms for the MAC to reach unprivileged apps. A number of researchers have published on this as well such as: https://www.usenix.org/conference/usenixsecurity19/presentation/reardon Three "leaks" were identified, two have already been fixed. -ioctl(SIOCGIFHWADDR). Fixed using finer grained LSM checks on socket ioctls (similar to this change). -IPv6 IP addresses. Fixed by no longer including the MAC as part of the IP address. -RTM_NEWLINK netlink route messages. The last mole to be whacked. > > And like others have said, tomorrow you'll be like "oh crap, we should > block X too" and we'll get another hook, another config knob, another > rulset update, etc. This seems like an issue inherent with permissions/capabilities. I don’t think we should abandon the concept of permissions because someone can forget to add a check. Likewise, if someone adds new code to the kernel and omits a capable(CAP_NET_*) check, I would expect it to be fixed like any other bug without the idea of capability checks being tossed out. We need to do something because this information is being abused. Any recommendations? This seemed like the simplest approach, but I can definitely appreciate that it has downsides. I could make this really generic by adding a single hook to the end of sock_msgrecv() which would allow an LSM to modify the message to omit the MAC address and any other information that we deem as sensitive in the future. Basically what Casey was suggesting. Thoughts on that approach? Thanks for your help on this.
From: Jeffrey Vander Stoep <jeffv@google.com> Date: Fri, 23 Aug 2019 13:41:38 +0200 > I could make this really generic by adding a single hook to the end of > sock_msgrecv() which would allow an LSM to modify the message to omit > the MAC address and any other information that we deem as sensitive in the > future. Basically what Casey was suggesting. Thoughts on that approach? Editing the SKB in place is generally frowned upon, and it could be cloned and in used by other code paths even, so would need to be copied or COW'd.
On Fri, Aug 23, 2019 at 7:41 AM Jeffrey Vander Stoep <jeffv@google.com> wrote: > On Fri, Aug 23, 2019 at 1:19 AM David Miller <davem@davemloft.net> wrote: > > From: Jeff Vander Stoep <jeffv@google.com> > > Date: Wed, 21 Aug 2019 15:45:47 +0200 > > > > > 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@google.com> > > > > I'm sure the MAC address will escape into userspace via other means, > > dumping pieces of networking config in other contexts, etc. I mean, > > if I can get a link dump, I can dump the neighbor table as well. > > These are already gated by existing LSM hooks and capability checks. > They are not allowed on mandatory access control systems unless explicitly > granted. > > > I kinda think this is all very silly whack-a-mole kind of stuff, to > > be quite honest. > > We evaluated mechanisms for the MAC to reach unprivileged apps. > A number of researchers have published on this as well such as: > https://www.usenix.org/conference/usenixsecurity19/presentation/reardon > > Three "leaks" were identified, two have already been fixed. > -ioctl(SIOCGIFHWADDR). Fixed using finer grained LSM checks > on socket ioctls (similar to this change). > -IPv6 IP addresses. Fixed by no longer including the MAC as part > of the IP address. > -RTM_NEWLINK netlink route messages. The last mole to be whacked. > > > And like others have said, tomorrow you'll be like "oh crap, we should > > block X too" and we'll get another hook, another config knob, another > > rulset update, etc. > > This seems like an issue inherent with permissions/capabilities. I don’t > think we should abandon the concept of permissions because someone > can forget to add a check. Likewise, if someone adds new code to the > kernel and omits a capable(CAP_NET_*) check, I would expect it to be > fixed like any other bug without the idea of capability checks being tossed > out. > > We need to do something because this information is being abused. Any > recommendations? This seemed like the simplest approach, but I can > definitely appreciate that it has downsides. > > I could make this really generic by adding a single hook to the end of > sock_msgrecv() which would allow an LSM to modify the message to omit > the MAC address and any other information that we deem as sensitive in the > future. Basically what Casey was suggesting. Thoughts on that approach? I apologize for the delay in responding; I'm blaming LSS-NA travel. I'm also not a big fan of inserting the hook in rtnl_fill_ifinfo(); as presented it is way too specific for a LSM hook for me to be happy. However, I do agree that giving the LSMs some control over netlink messages makes sense. As others have pointed out, it's all a matter of where to place the hook. If we only care about netlink messages which leverage nlattrs I suppose one option that I haven't seen mentioned would be to place a hook in nla_put(). While it is a bit of an odd place for a hook, it would allow the LSM easy access to the skb and attribute type to make decisions, and all of the callers should already be checking the return code (although we would need to verify this). One notable drawback (not the only one) is that the hook is going to get hit multiple times for each message. -- paul moore www.paul-moore.com
On Tue, Aug 27, 2019 at 04:47:04PM -0400, Paul Moore wrote: > > I'm also not a big fan of inserting the hook in rtnl_fill_ifinfo(); as > presented it is way too specific for a LSM hook for me to be happy. > However, I do agree that giving the LSMs some control over netlink > messages makes sense. As others have pointed out, it's all a matter > of where to place the hook. > > If we only care about netlink messages which leverage nlattrs I > suppose one option that I haven't seen mentioned would be to place a > hook in nla_put(). While it is a bit of an odd place for a hook, it > would allow the LSM easy access to the skb and attribute type to make > decisions, and all of the callers should already be checking the > return code (although we would need to verify this). One notable > drawback (not the only one) is that the hook is going to get hit > multiple times for each message. For most messages, "multiple times" would mean tens, for many even hundreds of calls. For each, you would have to check corresponding socket (and possibly also genetlink header) to see which netlink based protocol it is and often even parse existing part of the message to get the context (because the same numeric attribute type can mean something completely different if it appears in a nested attribute). Also, nla_put() (or rather __nla_put()) is not used for all attributes, one may also use nla_reserve() and then compose the attribute date in place. Michal Kubecek
On Thu, Aug 29, 2019 at 3:45 AM Michal Kubecek <mkubecek@suse.cz> wrote: > On Tue, Aug 27, 2019 at 04:47:04PM -0400, Paul Moore wrote: > > > > I'm also not a big fan of inserting the hook in rtnl_fill_ifinfo(); as > > presented it is way too specific for a LSM hook for me to be happy. > > However, I do agree that giving the LSMs some control over netlink > > messages makes sense. As others have pointed out, it's all a matter > > of where to place the hook. > > > > If we only care about netlink messages which leverage nlattrs I > > suppose one option that I haven't seen mentioned would be to place a > > hook in nla_put(). While it is a bit of an odd place for a hook, it > > would allow the LSM easy access to the skb and attribute type to make > > decisions, and all of the callers should already be checking the > > return code (although we would need to verify this). One notable > > drawback (not the only one) is that the hook is going to get hit > > multiple times for each message. > > For most messages, "multiple times" would mean tens, for many even > hundreds of calls. For each, you would have to check corresponding > socket (and possibly also genetlink header) to see which netlink based > protocol it is and often even parse existing part of the message to get > the context (because the same numeric attribute type can mean something > completely different if it appears in a nested attribute). > > Also, nla_put() (or rather __nla_put()) is not used for all attributes, > one may also use nla_reserve() and then compose the attribute date in > place. I never said it was a great idea, just an idea ;) Honestly I'm just trying to spur some discussion on this so we can hopefully arrive at a solution which allows a LSM to control kernel generated netlink messages that we can all accept.
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; + + } 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);
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@google.com> --- 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(-)