lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 05 Dec 2012 14:17:30 +0800 From: Jason Wang <jasowang@...hat.com> To: "Michael S. Tsirkin" <mst@...hat.com> CC: Paul Moore <pmoore@...hat.com>, netdev@...r.kernel.org, linux-security-module@...r.kernel.org, selinux@...ho.nsa.gov Subject: Re: [RFC PATCH 2/2] tun: fix LSM/SELinux labeling of tun/tap devices On 12/04/2012 11:24 PM, Michael S. Tsirkin wrote: > On Tue, Dec 04, 2012 at 09:24:43PM +0800, Jason Wang wrote: >> 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? > Basically, it looks like currently once you get a tun fd, > you can attach it to any device even if normally > selinux would prevent you from accessing it. Yes some checking during TUNSETQUEUE is missed. > If we reuse selinux_tun_dev_attach, we won't need to > change selinux policy, with a new capability we will need to change it > to allow libvirt to do TUNSETQUEUE. > Also needed for qemu too since it may call TUNSETQUEUE when guest wants to change the number of queues. >> 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. > Is this a problem? You can always > attach > set queue > detach > > and it would be hard to prevent this ... Currently, the following steps is allowed: 1. fd1 = open("/dev/net/tun"); 2. tunsetiff(fd1, "tap0"); 3. tunsetpersistent("tap0"); 4. close(fd1); 5. fd2 = open("/dev/net/tun"); 6. tunsetqueue(fd2, "tap0); Looks like step 6 should be forbidden since: - no fd/sockets were attached to the device, we need use TUNSETIFF instead to keep the API as we used do in single queue tun - we need update the security information in tun_struct just like what we discussed in this mail - it may also miss checks in TUNSETIFF >>> 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists