Re: [RFC PATCH v1 1/2] lsm: Add hooks to the TUN driver

["Serge E. Hallyn" <serue@us.ibm.com>]

Quoting Paul Moore (paul.moore@hp.com):
...
>  static int tun_attach(struct tun_struct *tun, struct file *file)
>  {
>  	struct tun_file *tfile = file->private_data;
> -	const struct cred *cred = current_cred();
> -	int err;
> +	int err = 0;
> 
>  	ASSERT_RTNL();
> 
> -	/* Check permissions */
> -	if (((tun->owner != -1 && cred->euid != tun->owner) ||
> -	     (tun->group != -1 && !in_egroup_p(tun->group))) &&
> -		!capable(CAP_NET_ADMIN))
> -		return -EPERM;
...

> @@ -935,6 +930,13 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>  		else
>  			return -EINVAL;
> 
> +		if ((tun->owner != -1 && cred->euid != tun->owner) ||
> +		    (tun->group != -1 && !in_egroup_p(tun->group)))
> +			return -EPERM;
> +		err = security_tun_dev_attach(tun->sk);
> +		if (err < 0)
> +			return err;
> +

...

> +/**
> + * cap_tun_dev_attach - Determine if attaching to an TUN device is allowed
> + *
> + * Determine if the user is allowed to attach to an existing persistent TUN
> + * device, historically this has always required the CAP_NET_ADMIN permission.
> + */
> +int cap_tun_dev_attach(void)
> +{
> +	if (!capable(CAP_NET_ADMIN))
> +		return -EPERM;
> +	return 0;
> +}

The checks before and after this patch are not equivalent.  Post-patch,
one must always have CAP_NET_ADMIN to do the attach, whereas pre-patch
you only needed those if current_cred() did not own the tun device.  Is
that intentional?

Also as Eric said this patch needs to set the cap_ hooks.  This patch
isn't yet introducing the selinux hooks, so iiuc actually this patch should
always oops if CONFIG_SECURITY=y.

-serge