2009-07-18 21:50:28

by Athanasius

[permalink] [raw]
Subject: Re: [[email protected]: Re: [patch 2/8] personality: fix PER_CLEAR_ON_SETID (CVE-2009-1895)]

On Sat, Jul 18, 2009 at 01:48:06PM -0700, Linus Torvalds wrote:
> On Sat, 18 Jul 2009, Greg KH wrote:
> >
> > and you have the whole idea of personalities being some kind of security
> > mechanism exposed as a joke.
>
> It's _not_ a "security mechanism". It never was.
...
> In the absense of raised capabilities, the personality flags don't matter:
> because they aren't security. If you have a personality flag that says "I
> want to mmap at virtual address zero", you're still going to be limited by
> the security layer, and if the security layer says "nope, you can't do
> that", then your personality doesn't matter.
>
> See?

I can understand and appreciate that, yes.

However the content of 'cat /proc/execdomains' is mis-leading for
the default Execution Domain. The string '0-0' implies either that you
can only set 1 of 3 personalities whilst this Execution Domain is current
OR that this Execution Domain will only be used whilst the set personality
is one of those 3. But neither is actually true as this default Execution
Domain (being the only one in vanilla kernel tree) is a special case.
If you don't see a valid reason to change personality(2) behaviour (thus
still allowing setting aribtrary personality values) then surely it would
make more sense for the default domain to set pers_high to PER_MASK ?
I'd suggest it actually be 0xffffffff but the field is only a char.

--
- Athanasius = Athanasius(at)miggy.org / http://www.miggy.org/
Finger athan(at)fysh.org for PGP key
"And it's me who is my enemy. Me who beats me up.
Me who makes the monsters. Me who strips my confidence." Paula Cole - ME


2009-07-19 02:08:59

by Julien TINNES

[permalink] [raw]
Subject: Re: [[email protected]: Re: [patch 2/8] personality: fix PER_CLEAR_ON_SETID (CVE-2009-1895)]

Athanasius a ?crit :
> On Sat, Jul 18, 2009 at 01:48:06PM -0700, Linus Torvalds wrote:
>> On Sat, 18 Jul 2009, Greg KH wrote:
>>> and you have the whole idea of personalities being some kind of security
>>> mechanism exposed as a joke.
>> It's _not_ a "security mechanism". It never was.
> ...
>> In the absense of raised capabilities, the personality flags don't matter:
>> because they aren't security. If you have a personality flag that says "I
>> want to mmap at virtual address zero", you're still going to be limited by
>> the security layer, and if the security layer says "nope, you can't do
>> that", then your personality doesn't matter.
>>
>> See?
>
> I can understand and appreciate that, yes.
>
> However the content of 'cat /proc/execdomains' is mis-leading for
> the default Execution Domain. The string '0-0' implies either that you
> can only set 1 of 3 personalities whilst this Execution Domain is current
> OR that this Execution Domain will only be used whilst the set personality
> is one of those 3. But neither is actually true as this default Execution
> Domain (being the only one in vanilla kernel tree) is a special case.
> If you don't see a valid reason to change personality(2) behaviour (thus
> still allowing setting aribtrary personality values) then surely it would
> make more sense for the default domain to set pers_high to PER_MASK ?
> I'd suggest it actually be 0xffffffff but the field is only a char.
>

So I think we all agree that this is not a security boundary and that
there is no security problem here.
A process should be able to change it's own personality, there is no
issue with this as long as we restrict the set of personalities which
are preserved when the process gets new privileges.

Currently, all the personalities are being implemented inside one
execution domain, the default one.

To address your concerns:
- pers_low and pers_high are compared to the *base* personality (without
flags), so it's correct to define them as uchar. pers should perhaps be
defined as uchar as well in lookup_exec_domain
- since all personalities are implemented inside the default execution
domain, it would indeed make sense to set default_exec_domain.pers_high
to PER_MASK. It would not change anything in practice, but it would make
sense.

Julien

2009-07-19 12:27:07

by Athanasius

[permalink] [raw]
Subject: Re: [[email protected]: Re: [patch 2/8] personality: fix PER_CLEAR_ON_SETID (CVE-2009-1895)]

On Sat, Jul 18, 2009 at 06:38:05PM -0700, Julien TINNES wrote:
> A process should be able to change it's own personality, there is no
> issue with this as long as we restrict the set of personalities which
> are preserved when the process gets new privileges.

And it's that "as long as we ..." that still bothers me. I've *never*
had any need for any use of this personality feature and this net/tun.c
exploit has proven there can be security gotchas with it. I'd prefer if
the whole thing were a kernel config option so I can easily turn it off
and have peace of mind that no future security bug discovered will
affect me.
No, I'd rather not look into using something like SELinux to turn off
one syscall, as that's introducing a whole extra layer of complexity.
Indeed the same exploit can instead make use of SELinux being misconfigured
by some vendors.

If the feature didn't already exist and was now proposed what are the
chances it would make it into the mainline kernel without having a
config option control it ? I'm wondering what its chances would be of
being accepted at all given the tentacles it seems to throw in all
directions (search for any of the actual personality feature flags in
the kernel source).
I'd also hazard that such ABI-compatibility with binaries from other
OSes is a feature the great majority of Linux users have never used and
now never will.

--
- Athanasius = Athanasius(at)miggy.org / http://www.miggy.org/
Finger athan(at)fysh.org for PGP key
"And it's me who is my enemy. Me who beats me up.
Me who makes the monsters. Me who strips my confidence." Paula Cole - ME

2009-07-19 19:28:59

by Linus Torvalds

[permalink] [raw]
Subject: Re: [[email protected]: Re: [patch 2/8] personality: fix PER_CLEAR_ON_SETID (CVE-2009-1895)]


On Sun, 19 Jul 2009, Athanasius wrote:
>
> And it's that "as long as we ..." that still bothers me. I've *never*
> had any need for any use of this personality feature and this net/tun.c
> exploit has proven there can be security gotchas with it.

I do agree. Some of those features may not be worth the cost.

That said, this particular feature made sense at the time it was
implemented. Some people really _did_ care about running SVR4 binaries on
Linux. There was a time when it was seen as a feature, and important
enough to work with. So that "map a zero page at NULL" was an important
thing that we wanted such binaries to be able to depend on.

These days? We could probably get rid of that idiotic feature. It's simply
not important enough any more. Does anybody really care? At the same time,
over years we've grown _other_ personality flags, and some of them are
still relevant.

Some binaries are unhappy with address space randomizations. Sometimes
it's because of outright bugs (that just were hidden by non-randomized VM
layout) - but that doesn't really help, does it? If you depend on that
binary, as a user you want the ability to say "run this binary in a mode
where it works".

Other binaries are unhappy with address space randomization because they
need to get the absolute maximum contiguous VM space for some big array.
Ok, so that's less of an issue in 64-bit mode, but there really are
programs out there that link everything statically and want to run at a
low virtual address so that they can get 2.5GB of virtual memory for one
single big allocation. I've written crap like that myself. I'm not _proud_
of it, but I could easily see that programs like that could be unhappy if
the system wiggles mmap's around for security issues.

So I do agree that we can probably get rid of some really dated
personality bits. But I don't think we can really get rid of the concept.
Because compatibility is always of paramount importance.

Linus

2009-07-19 19:39:21

by Athanasius

[permalink] [raw]
Subject: Re: [[email protected]: Re: [patch 2/8] personality: fix PER_CLEAR_ON_SETID (CVE-2009-1895)]

On Sun, Jul 19, 2009 at 12:27:05PM -0700, Linus Torvalds wrote:
> On Sun, 19 Jul 2009, Athanasius wrote:
> >
> > And it's that "as long as we ..." that still bothers me. I've *never*
> > had any need for any use of this personality feature and this net/tun.c
> > exploit has proven there can be security gotchas with it.
>
> I do agree. Some of those features may not be worth the cost.
>
...
>
> So I do agree that we can probably get rid of some really dated
> personality bits. But I don't think we can really get rid of the concept.
> Because compatibility is always of paramount importance.

Would you agree that having these features default-off would be best?
That way a user or sysadmin isn't suddenly surprised by different
behaviour. And those users who do need the functionality can turn it
on. Whether that be via compile-time option or a sysctl I leave up to
the people who know more about Linux Kernel coding than I. However, I'd
guess in the interests of vendor-kernel flexibility it should tend
towards the latter.
And, of course, this is what I *thought* Execution Domains were for
when looking at the code. Have only the default one and you're limited
pretty much to 'vanilla Linux'. Actually have available a module for
another personality and you allow its selection by users.

Put the choice in the hands of all users (read sysadmins even if its
their personal machine) rather than only in the hands of those who can
be bothered to recompile the kernel with an option, and currently
needing to hand-edit the source themselves to change the behaviour.

--
- Athanasius = Athanasius(at)miggy.org / http://www.miggy.org/
Finger athan(at)fysh.org for PGP key
"And it's me who is my enemy. Me who beats me up.
Me who makes the monsters. Me who strips my confidence." Paula Cole - ME

2009-07-19 19:48:49

by Linus Torvalds

[permalink] [raw]
Subject: Re: [[email protected]: Re: [patch 2/8] personality: fix PER_CLEAR_ON_SETID (CVE-2009-1895)]



On Sun, 19 Jul 2009, Athanasius wrote:
>
> Would you agree that having these features default-off would be best?

I'm actually pretty sure that some glibc versions in particular have
actively used some of the personality bits.

I have this fairly clear memory of seeing 'personality()' system calls in
strace output from regular programs.

I just tested it, and I'm not seeing it in my current distro for normal
programs, but the point is, I'm pretty sure that we have real binaries out
there that depend on personality bits.

So we can't default-off the whole personality thing. We would have to
default-off just a subset of the bits. Would that be useful? Perhaps.

Linus

2009-07-19 19:55:25

by Andi Kleen

[permalink] [raw]
Subject: Re: [[email protected]: Re: [patch 2/8] personality: fix PER_CLEAR_ON_SETID (CVE-2009-1895)]

Linus Torvalds <[email protected]> writes:
>
> Other binaries are unhappy with address space randomization because they
> need to get the absolute maximum contiguous VM space for some big array.
> Ok, so that's less of an issue in 64-bit mode, but there really are
> programs out there that link everything statically and want to run at a
> low virtual address so that they can get 2.5GB of virtual memory for one
> single big allocation. I've written crap like that myself. I'm not _proud_
> of it, but I could easily see that programs like that could be unhappy if
> the system wiggles mmap's around for security issues.

Another common reason for not supporting randomized mappings is
when the program loads a "core file" that has pointers to data
on each boot, as a faster way to initialize data structures.
That's common with LISP like languages for example, but even
e.g. gcc's pre compiled headers implementation works like this.

> Because compatibility is always of paramount importance.

If you want to give it a security angle: not supporting
an old application anymore is a very severe DoS attack
for people using it.

-Andi
--
[email protected] -- Speaking for myself only.

2009-07-19 22:01:41

by Alan

[permalink] [raw]
Subject: Re: [[email protected]: Re: [patch 2/8] personality: fix PER_CLEAR_ON_SETID (CVE-2009-1895)]

> These days? We could probably get rid of that idiotic feature. It's simply
> not important enough any more. Does anybody really care? At the same time,
> over years we've grown _other_ personality flags, and some of them are
> still relevant.

If it bugs you enough then a sysfs flag of "personality flags to honour"
is pretty trivial.

Alan