Re: [RFC PATCH 2/2] tun: fix LSM/SELinux labeling of tun/tap devices

Jason Wang
2012-12-04T13:24:50Z

On Monday, December 03, 2012 11:22:29 AM Paul Moore wrote:
> On Monday, December 03, 2012 06:15:42 PM Jason Wang wrote:
> > On 11/30/2012 06:06 AM, Paul Moore wrote:
> > > This patch corrects some problems with LSM/SELinux that were introduced
> > > with the multiqueue patchset. The problem stems from the fact that the
> > > multiqueue work changed the relationship between the tun device and its
> > > associated socket; before the socket persisted for the life of the
> > > device, however after the multiqueue changes the socket only persisted
> > > for the life of the userspace connection (fd open). For non-persistent
> > > devices this is not an issue, but for persistent devices this can cause
> > > the tun device to lose its SELinux label.
> > >
> > > We correct this problem by adding an opaque LSM security blob to the
> > > tun device struct which allows us to have the LSM security state, e.g.
> > > SELinux labeling information, persist for the lifetime of the tun
> > > device.
>
> ...
>
> > > -static int selinux_tun_dev_attach(struct sock *sk)
> > > +static int selinux_tun_dev_attach(struct sock *sk, void *security)
> > >
> > > {
> > >
> > > + struct tun_security_struct *tunsec = security;
> > >
> > > struct sk_security_struct *sksec = sk->sk_security;
> > > u32 sid = current_sid();
> > > int err;
> > >
> > > + /* we don't currently perform any NetLabel based labeling here ...
> > >
> > > err = avc_has_perm(sid, sksec->sid, SECCLASS_TUN_SOCKET,
> > >
> > > TUN_SOCKET__RELABELFROM, NULL);
> > >
> > > if (err)
> > >
> > > return err;
> > >
> > > - err = avc_has_perm(sid, sid, SECCLASS_TUN_SOCKET,
> > > + err = avc_has_perm(sid, tunsec->sid, SECCLASS_TUN_SOCKET,
> > >
> > > TUN_SOCKET__RELABELTO, NULL);
> > >
> > > if (err)
> > >
> > > return err;
> > >
> > > - sksec->sid = sid;
> > > + sksec->sid = tunsec->sid;
> > > + sksec->sclass = SECCLASS_TUN_SOCKET;
> >
> > I'm not sure whether this is correct, looks like we need to differ between
> > TUNSETQUEUE and TUNSETIFF. When userspace call TUNSETIFF for persistent
> > device, looks like we need change the sid of tunsec like in the past.
>
> It may be that I'm misunderstanding TUNSETQUEUE and/or TUNSETIFF. Can you
> elaborate as to why they should be different?

If I understand correctly, before multiqueue patchset, TUNSETIFF is used to:

1) Create the tun/tap network device
2) For persistent device, re-attach the fd to the network device / socket. In
this case, we call selinux_tun_dev_attch() to relabel the socket sid (in fact
also the device's since the socket were persistent also) to the sid of process
that calls TUNSETIFF.

So, after the changes of multiqueue, we need try to preserve those policy. The
interesting part is the introducing of TUNSETQUEUE, it's used to attach more
file descriptors/sockets to a tun/tap device after at least one file descriptor
were attached to the tun/tap device through TUNSETIFF. So I think maybe we
need differ those two ioctls. This patch looks fine for TUNSETQUEUE, but for
TUNSETIFF, we need relabel the tunsec to the process that calling TUNSETIFF
for persistent device?

btw. Current code does allow calling TUNSETQUEUE to a persistent tun/tap
device with no file attached. It should be a bug and need to be fixed.
>
> One thing that I think we probably should change is the relabelto/from
> permissions in the function above (selinux_tun_dev_attach()); in the case
> where the socket does not yet have a label, e.g. 'sksec->sid == 0', we
> should probably skip the relabel permissions since we want to assign the
> TUN device label regardless in this case.

I'm not familiar with the selinux, have a quick glance of the code, looks like
the label has been initialized to SECINITSID_KERNEL in
selinux_socket_post_create().

Thanks