Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Jeffrey Vander Stoep <jeffv@google.com>
To: netdev@vger.kernel.org,
	LSM List <linux-security-module@vger.kernel.org>,
	selinux@vger.kernel.org
Subject: Re: [PATCH 1/2] rtnetlink: gate MAC address with an LSM hook
Date: Wed, 21 Aug 2019 15:55:52 +0200
Message-ID: <CABXk95A7EwpBd1Snek9gvZRGKNsgD7q4cCmHp8xwQ93_gDx-Gg@mail.gmail.com> (raw)
In-Reply-To: <20190821134547.96929-1-jeffv@google.com>

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
>

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-21 13:45 Jeff Vander Stoep
2019-08-21 13:55 ` Jeffrey Vander Stoep [this message]
2019-08-21 14:34 ` Casey Schaufler
2019-08-21 14:52   ` Jeffrey Vander Stoep
2019-08-22 23:19 ` David Miller
2019-08-23 11:41   ` Jeffrey Vander Stoep
2019-08-23 21:41     ` David Miller
2019-08-27 20:47     ` Paul Moore
2019-08-29  7:45       ` Michal Kubecek
2019-08-30 21:46         ` Paul Moore
2019-08-23  4:24 ` kbuild test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABXk95A7EwpBd1Snek9gvZRGKNsgD7q4cCmHp8xwQ93_gDx-Gg@mail.gmail.com \
    --to=jeffv@google.com \
    --cc=linux-security-module@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git