networking probs in next-20081203

networking probs in next-20081203

[Andrew Morton]

x86_64, CONFIG_E1000E=y.

Current Linus mainline is OK, but with linux-next I'm seeing the following:

- During boot I see a "cannot open /proc/net/dev" fly past.  But when
  it gets to the login prompt, I can read /proc/net/dev.

- Networking won't come up.  `ifup eth0' says "determining ip address
  ...  failed", and the failure is immediate.

- eth0 appears in the ifconfig oputput as normal - it just won't come up.

- nothing interesting in dmesg


Here's the dmesg: http://userweb.kernel.org/~akpm/dmesg-dead-net.txt
(that dmesg was generated with E1000=y, E1000E=y, but that doesn't seem
to matter)

The .config: http://userweb.kernel.org/~akpm/config-dead-net.txt

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: networking probs in next-20081203

[Alexey Dobriyan]

On Wed, Dec 03, 2008 at 05:18:34PM -0800, Andrew Morton wrote:
> x86_64, CONFIG_E1000E=y.
> 
> Current Linus mainline is OK, but with linux-next I'm seeing the following:
> 
> - During boot I see a "cannot open /proc/net/dev" fly past.  But when
>   it gets to the login prompt, I can read /proc/net/dev.

I _suspect_ it might me something with /proc/net changes (again) and initcall
ordering.

Andrew, as rejectless quick test, you can do

	git checkout -f 26b0d981e7dd1d5b4852ed5719bd1388c7994163	# sic

which is the last tree merged before proc tree in that next tree.

> - Networking won't come up.  `ifup eth0' says "determining ip address
>   ...  failed", and the failure is immediate.
> 
> - eth0 appears in the ifconfig oputput as normal - it just won't come up.
> 
> - nothing interesting in dmesg
> 
> 
> Here's the dmesg: http://userweb.kernel.org/~akpm/dmesg-dead-net.txt
> (that dmesg was generated with E1000=y, E1000E=y, but that doesn't seem
> to matter)
> 
> The .config: http://userweb.kernel.org/~akpm/config-dead-net.txt

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: networking probs in next-20081203

[Kok, Auke]

Alexey Dobriyan wrote:
> On Wed, Dec 03, 2008 at 05:18:34PM -0800, Andrew Morton wrote:
>> x86_64, CONFIG_E1000E=y.
>>
>> Current Linus mainline is OK, but with linux-next I'm seeing the following:
>>
>> - During boot I see a "cannot open /proc/net/dev" fly past.  But when
>>   it gets to the login prompt, I can read /proc/net/dev.
> 
> I _suspect_ it might me something with /proc/net changes (again) and initcall
> ordering.
> 
> Andrew, as rejectless quick test, you can do
> 
> 	git checkout -f 26b0d981e7dd1d5b4852ed5719bd1388c7994163	# sic
> 
> which is the last tree merged before proc tree in that next tree.
> 
>> - Networking won't come up.  `ifup eth0' says "determining ip address
>>   ...  failed", and the failure is immediate.
>>
>> - eth0 appears in the ifconfig oputput as normal - it just won't come up.
>>
>> - nothing interesting in dmesg
>>
>>
>> Here's the dmesg: http://userweb.kernel.org/~akpm/dmesg-dead-net.txt
>> (that dmesg was generated with E1000=y, E1000E=y, but that doesn't seem
>> to matter)
>>
>> The .config: http://userweb.kernel.org/~akpm/config-dead-net.txt

perhaps this line has something to do with it?

[   50.539326] type=1400 audit(1228348149.537:4): avc:  denied  { mount } for
pid=11636 comm="ifconfig" name="/" dev=proc/net ino=4026531842
scontext=system_u:system_r:ifconfig_t:s0 tcontext=system_u:object_r:unlabeled_t:s0
tclass=filesystem

maybe try disabling selinux?

Auke

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: networking probs in next-20081203

[Alexey Dobriyan]

On Thu, Dec 04, 2008 at 09:41:24AM -0800, Kok, Auke wrote:
> Alexey Dobriyan wrote:
> > On Wed, Dec 03, 2008 at 05:18:34PM -0800, Andrew Morton wrote:
> >> x86_64, CONFIG_E1000E=y.
> >>
> >> Current Linus mainline is OK, but with linux-next I'm seeing the following:
> >>
> >> - During boot I see a "cannot open /proc/net/dev" fly past.  But when
> >>   it gets to the login prompt, I can read /proc/net/dev.
> > 
> > I _suspect_ it might me something with /proc/net changes (again) and initcall
> > ordering.
> > 
> > Andrew, as rejectless quick test, you can do
> > 
> > 	git checkout -f 26b0d981e7dd1d5b4852ed5719bd1388c7994163	# sic
> > 
> > which is the last tree merged before proc tree in that next tree.
> > 
> >> - Networking won't come up.  `ifup eth0' says "determining ip address
> >>   ...  failed", and the failure is immediate.
> >>
> >> - eth0 appears in the ifconfig oputput as normal - it just won't come up.
> >>
> >> - nothing interesting in dmesg
> >>
> >>
> >> Here's the dmesg: http://userweb.kernel.org/~akpm/dmesg-dead-net.txt
> >> (that dmesg was generated with E1000=y, E1000E=y, but that doesn't seem
> >> to matter)
> >>
> >> The .config: http://userweb.kernel.org/~akpm/config-dead-net.txt
> 
> perhaps this line has something to do with it?
> 
> [   50.539326] type=1400 audit(1228348149.537:4): avc:  denied  { mount } for
> pid=11636 comm="ifconfig" name="/" dev=proc/net ino=4026531842
> scontext=system_u:system_r:ifconfig_t:s0 tcontext=system_u:object_r:unlabeled_t:s0
> tclass=filesystem

Of course!

> maybe try disabling selinux?

This will work. :^)

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [E1000-devel] networking probs in next-20081203

[Stephen Smalley]

On Thu, 2008-12-04 at 20:52 +0300, Alexey Dobriyan wrote:
> On Thu, Dec 04, 2008 at 09:41:24AM -0800, Kok, Auke wrote:
> > Alexey Dobriyan wrote:
> > > On Wed, Dec 03, 2008 at 05:18:34PM -0800, Andrew Morton wrote:
> > >> x86_64, CONFIG_E1000E=y.
> > >>
> > >> Current Linus mainline is OK, but with linux-next I'm seeing the following:
> > >>
> > >> - During boot I see a "cannot open /proc/net/dev" fly past.  But when
> > >>   it gets to the login prompt, I can read /proc/net/dev.
> > > 
> > > I _suspect_ it might me something with /proc/net changes (again) and initcall
> > > ordering.
> > > 
> > > Andrew, as rejectless quick test, you can do
> > > 
> > > 	git checkout -f 26b0d981e7dd1d5b4852ed5719bd1388c7994163	# sic
> > > 
> > > which is the last tree merged before proc tree in that next tree.
> > > 
> > >> - Networking won't come up.  `ifup eth0' says "determining ip address
> > >>   ...  failed", and the failure is immediate.
> > >>
> > >> - eth0 appears in the ifconfig oputput as normal - it just won't come up.
> > >>
> > >> - nothing interesting in dmesg
> > >>
> > >>
> > >> Here's the dmesg: http://userweb.kernel.org/~akpm/dmesg-dead-net.txt
> > >> (that dmesg was generated with E1000=y, E1000E=y, but that doesn't seem
> > >> to matter)
> > >>
> > >> The .config: http://userweb.kernel.org/~akpm/config-dead-net.txt
> > 
> > perhaps this line has something to do with it?
> > 
> > [   50.539326] type=1400 audit(1228348149.537:4): avc:  denied  { mount } for
> > pid=11636 comm="ifconfig" name="/" dev=proc/net ino=4026531842
> > scontext=system_u:system_r:ifconfig_t:s0 tcontext=system_u:object_r:unlabeled_t:s0
> > tclass=filesystem

The significant line from the dmesg output is this:
[   32.113405] SELinux: initialized (dev proc/net, type proc/net), not configured for labeling

> Of course!
> 
> > maybe try disabling selinux?
> 
> This will work. :^)

SELinux didn't change here.  /proc/net did.

-- 
Stephen Smalley
National Security Agency

Re: [E1000-devel] networking probs in next-20081203

[David Miller]

From: Stephen Smalley <sds@tycho.nsa.gov>
Date: Thu, 04 Dec 2008 13:11:20 -0500

> On Thu, 2008-12-04 at 20:52 +0300, Alexey Dobriyan wrote:
> > On Thu, Dec 04, 2008 at 09:41:24AM -0800, Kok, Auke wrote:
> > > maybe try disabling selinux?
> > 
> > This will work. :^)
> 
> SELinux didn't change here.  /proc/net did.

We've been through this before...

And it is a usability issue that people can't change how procfs
directories work without requiring the user to update their selinux
policies first.

Re: networking probs in next-20081203

[Stephen Smalley]

On Thu, 2008-12-04 at 10:21 -0800, David Miller wrote:
> From: Stephen Smalley <sds@tycho.nsa.gov>
> Date: Thu, 04 Dec 2008 13:11:20 -0500
> 
> > On Thu, 2008-12-04 at 20:52 +0300, Alexey Dobriyan wrote:
> > > On Thu, Dec 04, 2008 at 09:41:24AM -0800, Kok, Auke wrote:
> > > > maybe try disabling selinux?
> > > 
> > > This will work. :^)
> > 
> > SELinux didn't change here.  /proc/net did.
> 
> We've been through this before...

Yep, and we altered SELinux so that they could freely change proc
directories into symlinks to support the earlier proc/net change.  But
now proc/net has turned into its own separate filesystem, with its own
filesystem type, which is unknown to SELinux.  Thus causing it to be
left unlabeled and inaccessible to confined domains.

> And it is a usability issue that people can't change how procfs
> directories work without requiring the user to update their selinux
> policies first.

Introducing a new filesystem type (proc/net) without teaching SELinux
how to handle it is always going to produce denials on accessing that
filesystem.  If they left the filesystem type string as "proc" it
wouldn't be a problem.  Or they can adjust the SELinux code to
automagically handle it.  Regardless, we didn't break anything.

-- 
Stephen Smalley
National Security Agency


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: networking probs in next-20081203

[Stephen Smalley]

On Thu, 2008-12-04 at 14:32 -0500, Stephen Smalley wrote:
> On Thu, 2008-12-04 at 10:21 -0800, David Miller wrote:
> > From: Stephen Smalley <sds@tycho.nsa.gov>
> > Date: Thu, 04 Dec 2008 13:11:20 -0500
> > 
> > > On Thu, 2008-12-04 at 20:52 +0300, Alexey Dobriyan wrote:
> > > > On Thu, Dec 04, 2008 at 09:41:24AM -0800, Kok, Auke wrote:
> > > > > maybe try disabling selinux?
> > > > 
> > > > This will work. :^)
> > > 
> > > SELinux didn't change here.  /proc/net did.
> > 
> > We've been through this before...
> 
> Yep, and we altered SELinux so that they could freely change proc
> directories into symlinks to support the earlier proc/net change.  But
> now proc/net has turned into its own separate filesystem, with its own
> filesystem type, which is unknown to SELinux.  Thus causing it to be
> left unlabeled and inaccessible to confined domains.
> 
> > And it is a usability issue that people can't change how procfs
> > directories work without requiring the user to update their selinux
> > policies first.
> 
> Introducing a new filesystem type (proc/net) without teaching SELinux
> how to handle it is always going to produce denials on accessing that
> filesystem.  If they left the filesystem type string as "proc" it
> wouldn't be a problem.

Actually, that isn't quite correct as it wouldn't generate the same
name/key for lookup.

>   Or they can adjust the SELinux code to
> automagically handle it.  Regardless, we didn't break anything.

Looking back, I see that they did in fact change selinux as part of the
patch to make proc/net its own filesystem, although unfortunately not in
a way that will quite work.  But no selinux maintainer was ever cc'd on
that patch.

-- 
Stephen Smalley
National Security Agency


------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: [E1000-devel] networking probs in next-20081203

[Eric W. Biederman]

Stephen Smalley <sds@tycho.nsa.gov> writes:

> On Thu, 2008-12-04 at 14:32 -0500, Stephen Smalley wrote:
>> On Thu, 2008-12-04 at 10:21 -0800, David Miller wrote:
>> > From: Stephen Smalley <sds@tycho.nsa.gov>
>> > Date: Thu, 04 Dec 2008 13:11:20 -0500
>> > 
>> > > On Thu, 2008-12-04 at 20:52 +0300, Alexey Dobriyan wrote:
>> > > > On Thu, Dec 04, 2008 at 09:41:24AM -0800, Kok, Auke wrote:
>> > > > > maybe try disabling selinux?
>> > > > 
>> > > > This will work. :^)
>> > > 
>> > > SELinux didn't change here.  /proc/net did.
>> > 
>> > We've been through this before...
>> 
>> Yep, and we altered SELinux so that they could freely change proc
>> directories into symlinks to support the earlier proc/net change.  But
>> now proc/net has turned into its own separate filesystem, with its own
>> filesystem type, which is unknown to SELinux.  Thus causing it to be
>> left unlabeled and inaccessible to confined domains.
>> 
>> > And it is a usability issue that people can't change how procfs
>> > directories work without requiring the user to update their selinux
>> > policies first.
>> 
>> Introducing a new filesystem type (proc/net) without teaching SELinux
>> how to handle it is always going to produce denials on accessing that
>> filesystem.  If they left the filesystem type string as "proc" it
>> wouldn't be a problem.
>
> Actually, that isn't quite correct as it wouldn't generate the same
> name/key for lookup.
>
>>   Or they can adjust the SELinux code to
>> automagically handle it.  Regardless, we didn't break anything.
>
> Looking back, I see that they did in fact change selinux as part of the
> patch to make proc/net its own filesystem, although unfortunately not in
> a way that will quite work.  But no selinux maintainer was ever cc'd on
> that patch.

Yes.  Apologies.  The whole thing started with some stupid security
drama and so things tried to happen outside of the normal channels
and things just went wrong, and got lost...

When I resent and started things through the normal channels I forgot
to cc you guys.  My apologies.

Which piece of selinux magic did I miss?

In particular can you tell if this was a code bug or a logic bug?

Eric

Re: [E1000-devel] networking probs in next-20081203

[James Morris]

On Thu, 4 Dec 2008, Eric W. Biederman wrote:

> Which piece of selinux magic did I miss?

The problem is that SELinux doesn't know anything about the new filesystem 
type, and specifically, to treat it like procfs.  There are a couple 
workarounds we can try to prevent this specific problem from cropping up 
again.

> In particular can you tell if this was a code bug or a logic bug?

I wouldn't say it was a bug, more a consequence of necessarily imperfect 
encapsulation of the security code via LSM.  It's just something we have to 
keep an eye out for.



- James
-- 
James Morris
<jmorris@namei.org>

Re: [E1000-devel] networking probs in next-20081203

[Eric W. Biederman]

James Morris <jmorris@namei.org> writes:

> On Thu, 4 Dec 2008, Eric W. Biederman wrote:
>
>> Which piece of selinux magic did I miss?
>
> The problem is that SELinux doesn't know anything about the new filesystem 
> type, and specifically, to treat it like procfs.  There are a couple 
> workarounds we can try to prevent this specific problem from cropping up 
> again.

The thing is I believe I changed the internal filesystem test to
strncmp(fstype, "proc", 4);

Which should match both proc and proc/net

And likewise I thought I provided the same name by for the magic label
lookup by name.

>> In particular can you tell if this was a code bug or a logic bug?
>
> I wouldn't say it was a bug, more a consequence of necessarily imperfect 
> encapsulation of the security code via LSM.  It's just something we have to 
> keep an eye out for.

Yes.  Was the piece I missed in the LSM rules loaded from user space?

Eric

Re: networking probs in next-20081203

[Stephen Smalley]

On Thu, 2008-12-04 at 13:00 -0800, Eric W. Biederman wrote:
> Stephen Smalley <sds@tycho.nsa.gov> writes:
> 
> > On Thu, 2008-12-04 at 14:32 -0500, Stephen Smalley wrote:
> >> On Thu, 2008-12-04 at 10:21 -0800, David Miller wrote:
> >> > From: Stephen Smalley <sds@tycho.nsa.gov>
> >> > Date: Thu, 04 Dec 2008 13:11:20 -0500
> >> > 
> >> > > On Thu, 2008-12-04 at 20:52 +0300, Alexey Dobriyan wrote:
> >> > > > On Thu, Dec 04, 2008 at 09:41:24AM -0800, Kok, Auke wrote:
> >> > > > > maybe try disabling selinux?
> >> > > > 
> >> > > > This will work. :^)
> >> > > 
> >> > > SELinux didn't change here.  /proc/net did.
> >> > 
> >> > We've been through this before...
> >> 
> >> Yep, and we altered SELinux so that they could freely change proc
> >> directories into symlinks to support the earlier proc/net change.  But
> >> now proc/net has turned into its own separate filesystem, with its own
> >> filesystem type, which is unknown to SELinux.  Thus causing it to be
> >> left unlabeled and inaccessible to confined domains.
> >> 
> >> > And it is a usability issue that people can't change how procfs
> >> > directories work without requiring the user to update their selinux
> >> > policies first.
> >> 
> >> Introducing a new filesystem type (proc/net) without teaching SELinux
> >> how to handle it is always going to produce denials on accessing that
> >> filesystem.  If they left the filesystem type string as "proc" it
> >> wouldn't be a problem.
> >
> > Actually, that isn't quite correct as it wouldn't generate the same
> > name/key for lookup.
> >
> >>   Or they can adjust the SELinux code to
> >> automagically handle it.  Regardless, we didn't break anything.
> >
> > Looking back, I see that they did in fact change selinux as part of the
> > patch to make proc/net its own filesystem, although unfortunately not in
> > a way that will quite work.  But no selinux maintainer was ever cc'd on
> > that patch.
> 
> Yes.  Apologies.  The whole thing started with some stupid security
> drama and so things tried to happen outside of the normal channels
> and things just went wrong, and got lost...
> 
> When I resent and started things through the normal channels I forgot
> to cc you guys.  My apologies.
> 
> Which piece of selinux magic did I miss?
> 
> In particular can you tell if this was a code bug or a logic bug?

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.

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


------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: networking probs in next-20081203

[James Morris]

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
> 

-- 
James Morris
<jmorris@namei.org>

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: networking probs in next-20081203

[Alexey Dobriyan]

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);

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: networking probs in next-20081203

[James Morris]

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.

-- 
James Morris
<jmorris@namei.org>

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: networking probs in next-20081203

[James Morris]

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>

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: networking probs in next-20081203

[Eric W. Biederman]

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

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: [E1000-devel] networking probs in next-20081203

[James Morris]

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.

-- 
James Morris
<jmorris@namei.org>

Re: [E1000-devel] networking probs in next-20081203

[Stephen Smalley]

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?

-- 
Stephen Smalley
National Security Agency

Re: networking probs in next-20081203

[James Morris]

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.

-- 
James Morris
<jmorris@namei.org>

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

[PATCH 0/3][RFC] Fix security and SELinux handling of proc/* filesystems

[James Morris]

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


-- 
James Morris
<jmorris@namei.org>

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

[PATCH 1/3][RFC] SELinux: correctly detect proc filesystems of the form "proc/foo"

[James Morris]

From: Stephen Smalley <sds@tycho.nsa.gov>

Map all of these proc/ filesystem types to "proc" for the policy lookup at
filesystem mount time.

Signed-off-by: James Morris <jmorris@namei.org>
---
 security/selinux/hooks.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d337748..470763a 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -672,7 +672,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);
-- 
1.6.0.4


------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: [PATCH 1/3][RFC] SELinux: correctly detect proc filesystems of the form "proc/foo"

[David P. Quigley]

On Fri, 2008-12-19 at 12:05 +1100, James Morris wrote:
> From: Stephen Smalley <sds@tycho.nsa.gov>
> 
> Map all of these proc/ filesystem types to "proc" for the policy lookup at
> filesystem mount time.
> 
> Signed-off-by: James Morris <jmorris@namei.org>
> ---
>  security/selinux/hooks.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index d337748..470763a 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -672,7 +672,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);

I'm  not sure if you took the patches I sent out for condensing the
flags and adding a /proc/mount notification for label support but if you
did then your patches haven't been updated to use them. sbsec->proc
should no longer exist and instead it should be sbsec->flags &
SE_SBPROC.

Dave


------------------------------------------------------------------------------

[PATCH 2/3][RFC] security: pass mount flags to security_sb_kern_mount()

[James Morris]

Pass mount flags to security_sb_kern_mount(), so security modules
can determine if a mount operation is being performed by the kernel.

Signed-off-by: James Morris <jmorris@namei.org>
---
 fs/super.c                 |    2 +-
 include/linux/security.h   |    6 +++---
 security/capability.c      |    2 +-
 security/security.c        |    4 ++--
 security/selinux/hooks.c   |    2 +-
 security/smack/smack_lsm.c |    3 ++-
 6 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 400a760..ddba069 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -914,7 +914,7 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void
 		goto out_free_secdata;
 	BUG_ON(!mnt->mnt_sb);
 
- 	error = security_sb_kern_mount(mnt->mnt_sb, secdata);
+ 	error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
  	if (error)
  		goto out_sb;
 
diff --git a/include/linux/security.h b/include/linux/security.h
index c13f1ce..dff563b 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1327,7 +1327,7 @@ struct security_operations {
 	int (*sb_alloc_security) (struct super_block *sb);
 	void (*sb_free_security) (struct super_block *sb);
 	int (*sb_copy_data) (char *orig, char *copy);
-	int (*sb_kern_mount) (struct super_block *sb, void *data);
+	int (*sb_kern_mount) (struct super_block *sb, int flags, void *data);
 	int (*sb_show_options) (struct seq_file *m, struct super_block *sb);
 	int (*sb_statfs) (struct dentry *dentry);
 	int (*sb_mount) (char *dev_name, struct path *path,
@@ -1596,7 +1596,7 @@ int security_bprm_secureexec(struct linux_binprm *bprm);
 int security_sb_alloc(struct super_block *sb);
 void security_sb_free(struct super_block *sb);
 int security_sb_copy_data(char *orig, char *copy);
-int security_sb_kern_mount(struct super_block *sb, void *data);
+int security_sb_kern_mount(struct super_block *sb, int flags, void *data);
 int security_sb_show_options(struct seq_file *m, struct super_block *sb);
 int security_sb_statfs(struct dentry *dentry);
 int security_sb_mount(char *dev_name, struct path *path,
@@ -1877,7 +1877,7 @@ static inline int security_sb_copy_data(char *orig, char *copy)
 	return 0;
 }
 
-static inline int security_sb_kern_mount(struct super_block *sb, void *data)
+static inline int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
 {
 	return 0;
 }
diff --git a/security/capability.c b/security/capability.c
index 2458748..0f6612d 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -64,7 +64,7 @@ static int cap_sb_copy_data(char *orig, char *copy)
 	return 0;
 }
 
-static int cap_sb_kern_mount(struct super_block *sb, void *data)
+static int cap_sb_kern_mount(struct super_block *sb, int flags, void *data)
 {
 	return 0;
 }
diff --git a/security/security.c b/security/security.c
index c0acfa7..5a37d79 100644
--- a/security/security.c
+++ b/security/security.c
@@ -266,9 +266,9 @@ int security_sb_copy_data(char *orig, char *copy)
 }
 EXPORT_SYMBOL(security_sb_copy_data);
 
-int security_sb_kern_mount(struct super_block *sb, void *data)
+int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
 {
-	return security_ops->sb_kern_mount(sb, data);
+	return security_ops->sb_kern_mount(sb, flags, data);
 }
 
 int security_sb_show_options(struct seq_file *m, struct super_block *sb)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 470763a..3897758 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2452,7 +2452,7 @@ out:
 	return rc;
 }
 
-static int selinux_sb_kern_mount(struct super_block *sb, void *data)
+static int selinux_sb_kern_mount(struct super_block *sb, int flags, void *data)
 {
 	struct avc_audit_data ad;
 	int rc;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 6e2dc0b..f23e927 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -248,11 +248,12 @@ static int smack_sb_copy_data(char *orig, char *smackopts)
 /**
  * smack_sb_kern_mount - Smack specific mount processing
  * @sb: the file system superblock
+ * @flags: the mount flags
  * @data: the smack mount options
  *
  * Returns 0 on success, an error code on failure
  */
-static int smack_sb_kern_mount(struct super_block *sb, void *data)
+static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data)
 {
 	struct dentry *root = sb->s_root;
 	struct inode *inode = root->d_inode;
-- 
1.6.0.4

Re: [PATCH 2/3][RFC] security: pass mount flags to security_sb_kern_mount()

[Stephen Smalley]

On Fri, 2008-12-19 at 12:06 +1100, James Morris wrote:
> Pass mount flags to security_sb_kern_mount(), so security modules
> can determine if a mount operation is being performed by the kernel.
> 
> Signed-off-by: James Morris <jmorris@namei.org>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  fs/super.c                 |    2 +-
>  include/linux/security.h   |    6 +++---
>  security/capability.c      |    2 +-
>  security/security.c        |    4 ++--
>  security/selinux/hooks.c   |    2 +-
>  security/smack/smack_lsm.c |    3 ++-
>  6 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 400a760..ddba069 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -914,7 +914,7 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void
>  		goto out_free_secdata;
>  	BUG_ON(!mnt->mnt_sb);
>  
> - 	error = security_sb_kern_mount(mnt->mnt_sb, secdata);
> + 	error = security_sb_kern_mount(mnt->mnt_sb, flags, secdata);
>   	if (error)
>   		goto out_sb;
>  
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c13f1ce..dff563b 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1327,7 +1327,7 @@ struct security_operations {
>  	int (*sb_alloc_security) (struct super_block *sb);
>  	void (*sb_free_security) (struct super_block *sb);
>  	int (*sb_copy_data) (char *orig, char *copy);
> -	int (*sb_kern_mount) (struct super_block *sb, void *data);
> +	int (*sb_kern_mount) (struct super_block *sb, int flags, void *data);
>  	int (*sb_show_options) (struct seq_file *m, struct super_block *sb);
>  	int (*sb_statfs) (struct dentry *dentry);
>  	int (*sb_mount) (char *dev_name, struct path *path,
> @@ -1596,7 +1596,7 @@ int security_bprm_secureexec(struct linux_binprm *bprm);
>  int security_sb_alloc(struct super_block *sb);
>  void security_sb_free(struct super_block *sb);
>  int security_sb_copy_data(char *orig, char *copy);
> -int security_sb_kern_mount(struct super_block *sb, void *data);
> +int security_sb_kern_mount(struct super_block *sb, int flags, void *data);
>  int security_sb_show_options(struct seq_file *m, struct super_block *sb);
>  int security_sb_statfs(struct dentry *dentry);
>  int security_sb_mount(char *dev_name, struct path *path,
> @@ -1877,7 +1877,7 @@ static inline int security_sb_copy_data(char *orig, char *copy)
>  	return 0;
>  }
>  
> -static inline int security_sb_kern_mount(struct super_block *sb, void *data)
> +static inline int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
>  {
>  	return 0;
>  }
> diff --git a/security/capability.c b/security/capability.c
> index 2458748..0f6612d 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -64,7 +64,7 @@ static int cap_sb_copy_data(char *orig, char *copy)
>  	return 0;
>  }
>  
> -static int cap_sb_kern_mount(struct super_block *sb, void *data)
> +static int cap_sb_kern_mount(struct super_block *sb, int flags, void *data)
>  {
>  	return 0;
>  }
> diff --git a/security/security.c b/security/security.c
> index c0acfa7..5a37d79 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -266,9 +266,9 @@ int security_sb_copy_data(char *orig, char *copy)
>  }
>  EXPORT_SYMBOL(security_sb_copy_data);
>  
> -int security_sb_kern_mount(struct super_block *sb, void *data)
> +int security_sb_kern_mount(struct super_block *sb, int flags, void *data)
>  {
> -	return security_ops->sb_kern_mount(sb, data);
> +	return security_ops->sb_kern_mount(sb, flags, data);
>  }
>  
>  int security_sb_show_options(struct seq_file *m, struct super_block *sb)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 470763a..3897758 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2452,7 +2452,7 @@ out:
>  	return rc;
>  }
>  
> -static int selinux_sb_kern_mount(struct super_block *sb, void *data)
> +static int selinux_sb_kern_mount(struct super_block *sb, int flags, void *data)
>  {
>  	struct avc_audit_data ad;
>  	int rc;
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 6e2dc0b..f23e927 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -248,11 +248,12 @@ static int smack_sb_copy_data(char *orig, char *smackopts)
>  /**
>   * smack_sb_kern_mount - Smack specific mount processing
>   * @sb: the file system superblock
> + * @flags: the mount flags
>   * @data: the smack mount options
>   *
>   * Returns 0 on success, an error code on failure
>   */
> -static int smack_sb_kern_mount(struct super_block *sb, void *data)
> +static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data)
>  {
>  	struct dentry *root = sb->s_root;
>  	struct inode *inode = root->d_inode;
-- 
Stephen Smalley
National Security Agency


------------------------------------------------------------------------------

[PATCH 3/3][RFC] SELinux: don't check permissions for kernel mounts

[James Morris]

Don't bother checking permissions when the kernel performs an internal 
mount, as this should always be allowed.

Signed-off-by: James Morris <jmorris@namei.org>
---
 security/selinux/hooks.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3897758..4a44903 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2461,6 +2461,10 @@ static int selinux_sb_kern_mount(struct super_block *sb, int flags, void *data)
 	if (rc)
 		return rc;
 
+	/* Allow all mounts performed by the kernel */
+	if (flags & MS_KERNMOUNT)
+		return 0;
+
 	AVC_AUDIT_DATA_INIT(&ad, FS);
 	ad.u.fs.path.dentry = sb->s_root;
 	return superblock_has_perm(current, sb, FILESYSTEM__MOUNT, &ad);
-- 
1.6.0.4

Re: [PATCH 3/3][RFC] SELinux: don't check permissions for kernel mounts

[Stephen Smalley]

On Fri, 2008-12-19 at 12:07 +1100, James Morris wrote:
> Don't bother checking permissions when the kernel performs an internal 
> mount, as this should always be allowed.
> 
> Signed-off-by: James Morris <jmorris@namei.org>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  security/selinux/hooks.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3897758..4a44903 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2461,6 +2461,10 @@ static int selinux_sb_kern_mount(struct super_block *sb, int flags, void *data)
>  	if (rc)
>  		return rc;
>  
> +	/* Allow all mounts performed by the kernel */
> +	if (flags & MS_KERNMOUNT)
> +		return 0;
> +
>  	AVC_AUDIT_DATA_INIT(&ad, FS);
>  	ad.u.fs.path.dentry = sb->s_root;
>  	return superblock_has_perm(current, sb, FILESYSTEM__MOUNT, &ad);
-- 
Stephen Smalley
National Security Agency


------------------------------------------------------------------------------

Re: [PATCH 0/3][RFC] Fix security and SELinux handling of proc/* filesystems

[David Miller]

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