Subject: Re: [RFC PATCH v2 2/2] selinux: Support for the new TUN LSM hooks

From: "Serge E. Hallyn" <[email-masked]>
Date: Wed, 12 Aug 2009 18:07:49 -0500

Quoting Paul Moore ([email-masked]):
> On Wednesday 12 August 2009 06:14:40 pm Serge E. Hallyn wrote:
> > Quoting Paul Moore ([email-masked]):
> > > +static int selinux_tun_dev_attach(struct sock *sk)
> > > +{
> > > +	struct sk_security_struct *sksec = sk->sk_security;
> > > +	u32 sid = current_sid();
> > > +	int err;
> > > +
> > > +	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_RAWIP_SOCKET,
> >
> > Was RAWIP on purpose here?
> 
> Nope, a mistake on my part that I hadn't caught yet.  Thanks.
> 
> > > +			   TUN_SOCKET__RELABELTO, NULL);
> > > +	if (err)
> > > +		return err;
> > > +
> > > +	sksec->sid = sid;
> > > +
> > > +	return 0;
> > > +}
> >
> > IIUC it is possible for multiple processes to attach to the same
> > tun device.  Will it get confusing/incorrect to have each attach
> > potentially (if tasks have different sids) relabel?
> 
> I may be reading the code wrong, but in drivers/net/tun.c:tun_attach() the 
> code checks to see if the TUN device is already in use and if it is then the 
> attach fails with -EBUSY (check where the tun_device->tfile is examined).  I 

Ah yes, you're right - I saw the check for (ifr->ifr_flags & IFF_TUN_EXCL) in
the attach path in tun_set_iff, and missed this one.

> believe this should ensure that only one process at a time has access to the 
> TUN device so we shouldn't have to worry about a TUN socket getting relabeled 
> while it is currently in use.  As far as persistent TUN devices getting 
> relabeled when a new process attaches to them, that is what we are trying to 
> accomplish here so that the network traffic being sent via the TUN device is 
> labeled according to the currently attached process; this is consistent with 
> how SELinux currently labels locally generated outbound traffic - outbound 
> packets inherit their security label from the sending process via the 
> originating socket/sock.

Ok, thanks.  To my untrained eye the class addition looks right too, so
with the trivial change:

Acked-by: Serge Hallyn <[email-masked]>

thanks,
-serge