| Message ID | 1228486339.20274.3.camel@localhost.localdomain |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 5 Dec 2008, Stephen Smalley wrote: > I suspect we need the following un-tested diff to map all of these proc/ > filesystem types to "proc" for the policy lookup at filesystem mount > time. I finally got a bootable linux-next, but it seems that the proc/net patch is no longer in there. Any idea if it's coming back? The patch below looks ok, but it needs testing (and I'd suggest perhaps including it any future version of the proc/net patch). > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 9155fa9..3c3ceb7 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -703,7 +703,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, > sbsec->proc = 1; > > /* Determine the labeling behavior to use for this filesystem type. */ > - rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid); > + rc = security_fs_use(sbsec->proc ? "proc" : sb->s_type->name, &sbsec->behavior, &sbsec->sid); > if (rc) { > printk(KERN_WARNING "%s: security_fs_use(%s) returned %d\n", > __func__, sb->s_type->name, rc); > > -- > Stephen Smalley > National Security Agency >
On Thu, Dec 11, 2008 at 09:41:20PM +1100, James Morris wrote: > On Fri, 5 Dec 2008, Stephen Smalley wrote: > > > I suspect we need the following un-tested diff to map all of these proc/ > > filesystem types to "proc" for the policy lookup at filesystem mount > > time. > > I finally got a bootable linux-next, but it seems that the proc/net patch > is no longer in there. > > Any idea if it's coming back? The patch below looks ok, but it needs > testing Yes, please, someone test it. > (and I'd suggest perhaps including it any future version of the proc/net patch). I placed it into proc-wip branch to not screw testers with SELinux meanwhile git-remote add proc git://git.kernel.org/pub/scm/linux/kernel/git/adobriyan/proc.git > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -703,7 +703,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, > > sbsec->proc = 1; > > > > /* Determine the labeling behavior to use for this filesystem type. */ > > - rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid); > > + rc = security_fs_use(sbsec->proc ? "proc" : sb->s_type->name, &sbsec->behavior, &sbsec->sid); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 12 Dec 2008, Alexey Dobriyan wrote:
> Yes, please, someone test it.
Still getting avc denials:
avc: denied { mount } for pid=2308 comm="dhclient" name="/"
dev=proc/net ino=4026531842 scontext=unconfined_u:system_r:dhcpc_t:s0-s0:c0.c1023
tcontext=system_u:object_r:proc_t:s0 tclass=filesystem
type=SYSCALL msg=audit(1229073699.174:53): arch=c000003e syscall=2
success=no exit=-2 a0=45bef7 a1=80000 a2=1b6 a3=7f296
e71c6f0 items=0 ppid=2259 pid=2308 auid=0 uid=0 gid=0 euid=0 suid=0
fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 ses=1 comm="
dhclient" exe="/sbin/dhclient"
subj=unconfined_u:system_r:dhcpc_t:s0-s0:c0.c1023 key=(null)
It seems the problem is that the /proc/net mountpoint is now labeled as
proc_t.
On Fri, 12 Dec 2008, James Morris wrote: > It seems the problem is that the /proc/net mountpoint is now labeled as > proc_t. Actually, that's not the problem, it's the same on a normal kernel.
James Morris <jmorris@namei.org> writes: > On Fri, 12 Dec 2008, James Morris wrote: > >> It seems the problem is that the /proc/net mountpoint is now labeled as >> proc_t. > > Actually, that's not the problem, it's the same on a normal kernel. To confirm what you are saying. There is something wrong with the bug fix? Eric -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2008-12-12 at 20:26 +1100, James Morris wrote: > On Fri, 12 Dec 2008, Alexey Dobriyan wrote: > > > Yes, please, someone test it. > > Still getting avc denials: > > avc: denied { mount } for pid=2308 comm="dhclient" name="/" > dev=proc/net ino=4026531842 scontext=unconfined_u:system_r:dhcpc_t:s0-s0:c0.c1023 > tcontext=system_u:object_r:proc_t:s0 tclass=filesystem > type=SYSCALL msg=audit(1229073699.174:53): arch=c000003e syscall=2 > success=no exit=-2 a0=45bef7 a1=80000 a2=1b6 a3=7f296 > e71c6f0 items=0 ppid=2259 pid=2308 auid=0 uid=0 gid=0 euid=0 suid=0 > fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 ses=1 comm=" > dhclient" exe="/sbin/dhclient" > subj=unconfined_u:system_r:dhcpc_t:s0-s0:c0.c1023 key=(null) > > It seems the problem is that the /proc/net mountpoint is now labeled as > proc_t. No, that's a check against the filesystem (superblock) label, not the mountpoint directory. proc_net_follow_link() creates a new mount, so we end up hitting security_sb_kern_mount() => selinux_sb_kern_mount() and triggering this permission check in the context of the current process on what is supposed to be a kernel-internal mount of /proc/net. Maybe pass flags down to security_sb_kern_mount() and skip the check in the MS_KERNMOUNT case?
On Fri, 12 Dec 2008, Eric W. Biederman wrote: > James Morris <jmorris@namei.org> writes: > > > On Fri, 12 Dec 2008, James Morris wrote: > > > >> It seems the problem is that the /proc/net mountpoint is now labeled as > >> proc_t. > > > > Actually, that's not the problem, it's the same on a normal kernel. > > To confirm what you are saying. There is something wrong with the bug fix? There's still a problem, yes.
On Fri, 12 Dec 2008, Stephen Smalley wrote: > Maybe pass flags down to security_sb_kern_mount() and skip the check in > the MS_KERNMOUNT case? Seems like a good solution.
These patches address the issues encountered in the recent discussion: "[E1000-devel] networking probs in next-20081203" <https://kerneltrap.org/mailarchive/linux-netdev/2008/12/4/4315684/thread> where making proc/net into its own filesystem to be mounted on a per-namespace basis caused SELinux labeling to stop working. The solution is to first ensure that the filesystem is correctly labeled, and then to also allow filesystems being mounted by the kernel to bypass SELinux permission checks (these operations should always be allowed). The mount flags are now passed to security_sb_kern_mount(), so that the security module can check whether MS_KERNMOUNT is set. Please review and ack if ok. These patches are against git://git.kernel.org/pub/scm/linux/kernel/git/adobriyan/proc.git#proc-wip
From: James Morris <jmorris@namei.org> Date: Fri, 19 Dec 2008 12:04:07 +1100 (EST) > Please review and ack if ok. Although the changes are outside of networking, they look fine to me, ACK -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9155fa9..3c3ceb7 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -703,7 +703,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, sbsec->proc = 1; /* Determine the labeling behavior to use for this filesystem type. */ - rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid); + rc = security_fs_use(sbsec->proc ? "proc" : sb->s_type->name, &sbsec->behavior, &sbsec->sid); if (rc) { printk(KERN_WARNING "%s: security_fs_use(%s) returned %d\n", __func__, sb->s_type->name, rc);