While it's more secure to not dump core at all if the program has
switched euid, it's also very unpractical. Since only programs
started from root, being setuid root or have CAP_SETUID it's far
more practical to dump as root.root mode 600. This is the bahavior
of Solaris.
The current implementation does not ensure that an existing core
file is only readable as root, i.e. after dumping the ownership
and mode is unchanged.
Besides mm->dumpable to avoid recursive core dumps, on setuid files
the dumpable flag still prevents a core dump while seteuid & co will
result in a core only readable as root.
Peter W?chtler <[email protected]> wrote:
>
> While it's more secure to not dump core at all if the program has
> switched euid, it's also very unpractical. Since only programs
> started from root, being setuid root or have CAP_SETUID it's far
> more practical to dump as root.root mode 600. This is the bahavior
> of Solaris.
>
> The current implementation does not ensure that an existing core
> file is only readable as root, i.e. after dumping the ownership
> and mode is unchanged.
>
> Besides mm->dumpable to avoid recursive core dumps, on setuid files
> the dumpable flag still prevents a core dump while seteuid & co will
> result in a core only readable as root.
It's a bit sad to add another function call level to sys_unlink() simply
because the core dumping code needs it.
Is it not possible to call sys_unlink() directly from there? Something like
long kernel_unlink(const char *name)
{
mm_segment_t old_fs = get_fs();
long ret;
set_fs(KERNEL_DS);
ret = sys_unlink(name);
set_fs(old_fs);
return ret;
}
On Thursday, April 22, 2004, at 03:18AM, Andrew Morton <[email protected]> wrote:
>Peter W�chtler <[email protected]> wrote:
>>
>> While it's more secure to not dump core at all if the program has
>> switched euid, it's also very unpractical. Since only programs
>> started from root, being setuid root or have CAP_SETUID it's far
>> more practical to dump as root.root mode 600. This is the bahavior
>> of Solaris.
>>
>> The current implementation does not ensure that an existing core
>> file is only readable as root, i.e. after dumping the ownership
>> and mode is unchanged.
>>
>> Besides mm->dumpable to avoid recursive core dumps, on setuid files
>> the dumpable flag still prevents a core dump while seteuid & co will
>> result in a core only readable as root.
>
>It's a bit sad to add another function call level to sys_unlink() simply
>because the core dumping code needs it.
>
>Is it not possible to call sys_unlink() directly from there? Something like
>
>long kernel_unlink(const char *name)
>{
> mm_segment_t old_fs = get_fs();
> long ret;
>
> set_fs(KERNEL_DS);
> ret = sys_unlink(name);
> set_fs(old_fs);
> return ret;
>}
And you're asking me? ;)
While getname() has a check for user/kernelspace - do you really
care about "the overhead" for a function call level with 1 argument?
Uhm, probably if you even care to write this..
What is the cost for switching the segments?
But I agree that sys_unlink should be the fast call and dumping core
is the exception :)
would fastcall do_unlink() help? I guess the arg is then passed in a
register
Peter Waechtler <[email protected]> wrote:
>
> >Is it not possible to call sys_unlink() directly from there? Something like
> >
> >long kernel_unlink(const char *name)
> >{
> > mm_segment_t old_fs = get_fs();
> > long ret;
> >
> > set_fs(KERNEL_DS);
> > ret = sys_unlink(name);
> > set_fs(old_fs);
> > return ret;
> >}
>
> And you're asking me? ;)
Well someone has to know ;)
> While getname() has a check for user/kernelspace - do you really
> care about "the overhead" for a function call level with 1 argument?
> Uhm, probably if you even care to write this..
It's very small, but heck, we unlink a lot more often than we dump core.
Most of us, that is.
> What is the cost for switching the segments?
Teeny.
> But I agree that sys_unlink should be the fast call and dumping core
> is the exception :)
>
> would fastcall do_unlink() help? I guess the arg is then passed in a
> register
I've never been able to measure any size or space benefit for fastcall, and
we do it via compiler options kernel-wide nowadays.
The above will work fine. You can probably just open-code it at the place
where you're unlinking the file.
(why are you trying to unlink the old file anyway?)
On Thursday, April 22, 2004, at 10:55AM, Andrew Morton <[email protected]> wrote:
>Peter Waechtler <[email protected]> wrote:
>>
>> But I agree that sys_unlink should be the fast call and dumping core
>> is the exception :)
>>
>> would fastcall do_unlink() help? I guess the arg is then passed in a
>> register
>
>I've never been able to measure any size or space benefit for fastcall, and
>we do it via compiler options kernel-wide nowadays.
>
>The above will work fine. You can probably just open-code it at the place
>where you're unlinking the file.
>
>(why are you trying to unlink the old file anyway?)
>
For security measure :O
I tried on solaris: touch the core file as user, open it and wait, dump core
as root -> nope, couldn't read the damn core - it was unlinked and created!
So do I want.
I will sent the new patch from home.
Peter Waechtler <[email protected]> wrote:
>
> >(why are you trying to unlink the old file anyway?)
> >
>
> For security measure :O
> I tried on solaris: touch the core file as user, open it and wait, dump core
> as root -> nope, couldn't read the damn core - it was unlinked and created!
hm, OK. There's a window in which someone can come in and recreate the
file, but the open is using O_EXCL|O_CREATE so that seems safe enough.
Am Do, 2004-04-22 um 11.56 schrieb Andrew Morton:
> Peter Waechtler <[email protected]> wrote:
> >
> > >(why are you trying to unlink the old file anyway?)
> > >
> >
> > For security measure :O
> > I tried on solaris: touch the core file as user, open it and wait, dump core
> > as root -> nope, couldn't read the damn core - it was unlinked and created!
>
> hm, OK. There's a window in which someone can come in and recreate the
> file, but the open is using O_EXCL|O_CREATE so that seems safe enough.
So here is the updated patch with an open coded call to sys_unlink
* Peter W?chtler ([email protected]) wrote:
> Am Do, 2004-04-22 um 11.56 schrieb Andrew Morton:
> > Peter Waechtler <[email protected]> wrote:
> > >
> > > >(why are you trying to unlink the old file anyway?)
> > > >
> > >
> > > For security measure :O
> > > I tried on solaris: touch the core file as user, open it and wait, dump core
> > > as root -> nope, couldn't read the damn core - it was unlinked and created!
> >
> > hm, OK. There's a window in which someone can come in and recreate the
> > file, but the open is using O_EXCL|O_CREATE so that seems safe enough.
>
> So here is the updated patch with an open coded call to sys_unlink
This patch breaks various ptrace() checks.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
On Thu, 22 Apr 2004, Peter W?chtler wrote:
>
> > hm, OK. There's a window in which someone can come in and recreate the
> > file, but the open is using O_EXCL|O_CREATE so that seems safe enough.
>
> So here is the updated patch with an open coded call to sys_unlink
Aughr.
Wouldn't it be much nicer to just refuse to overwrite files owned by
anybody else?
In other words, I'd much rather see a patch that is a much simpler one,
which just says: if we opened an existing file, we won't touch it if we
weren't the owners of it.
That should be safe for root _and_ it should be safe for people who
already had a file descriptor open previously (hey, if the previous
root-owned core-file was world readable, then what else is new?)
Tell me why this isn't simpler?
Linus
---
--- 1.111/fs/exec.c Wed Apr 21 02:11:57 2004
+++ edited/fs/exec.c Thu Apr 22 13:03:27 2004
@@ -1378,6 +1378,8 @@
inode = file->f_dentry->d_inode;
if (inode->i_nlink > 1)
goto close_fail; /* multiple links - don't dump */
+ if (inode->i_uid != current->euid || inode->i_gid != current->egid)
+ goto close_fail;
if (d_unhashed(file->f_dentry))
goto close_fail;
On Thursday, April 22, 2004, at 10:05PM, Linus Torvalds <[email protected]> wrote:
>
>
>On Thu, 22 Apr 2004, Peter W�chtler wrote:
>>
>> > hm, OK. There's a window in which someone can come in and recreate the
>> > file, but the open is using O_EXCL|O_CREATE so that seems safe enough.
>>
>> So here is the updated patch with an open coded call to sys_unlink
>
>Aughr.
>
>Wouldn't it be much nicer to just refuse to overwrite files owned by
>anybody else?
>
>In other words, I'd much rather see a patch that is a much simpler one,
>which just says: if we opened an existing file, we won't touch it if we
>weren't the owners of it.
>
>That should be safe for root _and_ it should be safe for people who
>already had a file descriptor open previously (hey, if the previous
>root-owned core-file was world readable, then what else is new?)
>
the previous core was owned by user.donttellyourwisdom
The root process happily dumps it's core into it, doesn't change
ownership nor permissions.
>Tell me why this isn't simpler?
>
I can't it's simpler.
If you are interested in the core (you don't because you don't make
errors, you don't even use debuggers but change the VM ;> )
you get it, otherwise you have an old one.
>
>---
>--- 1.111/fs/exec.c Wed Apr 21 02:11:57 2004
>+++ edited/fs/exec.c Thu Apr 22 13:03:27 2004
>@@ -1378,6 +1378,8 @@
> inode = file->f_dentry->d_inode;
> if (inode->i_nlink > 1)
> goto close_fail; /* multiple links - don't dump */
>+ if (inode->i_uid != current->euid || inode->i_gid != current->egid)
>+ goto close_fail;
> if (d_unhashed(file->f_dentry))
> goto close_fail;
>
>
>
On Thursday, April 22, 2004, at 09:53PM, Chris Wright <[email protected]> wrote:
>* Peter W�chtler ([email protected]) wrote:
>> Am Do, 2004-04-22 um 11.56 schrieb Andrew Morton:
>> > Peter Waechtler <[email protected]> wrote:
>> > >
>> > > >(why are you trying to unlink the old file anyway?)
>> > > >
>> > >
>> > > For security measure :O
>> > > I tried on solaris: touch the core file as user, open it and wait, dump core
>> > > as root -> nope, couldn't read the damn core - it was unlinked and created!
>> >
>> > hm, OK. There's a window in which someone can come in and recreate the
>> > file, but the open is using O_EXCL|O_CREATE so that seems safe enough.
>>
>> So here is the updated patch with an open coded call to sys_unlink
>
>This patch breaks various ptrace() checks.
>
>thanks,
please
Care to share your wisdom? No? Then please don't reply
On Thursday, April 22, 2004, at 09:53PM, Chris Wright <[email protected]> wrote:
>* Peter W�chtler ([email protected]) wrote:
>> Am Do, 2004-04-22 um 11.56 schrieb Andrew Morton:
>> > Peter Waechtler <[email protected]> wrote:
>> > >
>> > > >(why are you trying to unlink the old file anyway?)
>> > > >
>> > >
>> > > For security measure :O
>> > > I tried on solaris: touch the core file as user, open it and wait, dump core
>> > > as root -> nope, couldn't read the damn core - it was unlinked and created!
>> >
>> > hm, OK. There's a window in which someone can come in and recreate the
>> > file, but the open is using O_EXCL|O_CREATE so that seems safe enough.
>>
>> So here is the updated patch with an open coded call to sys_unlink
>
>This patch breaks various ptrace() checks.
>
I guess the mm->dumpable flag was misused then as something like mm->switchUid.
I can add this flag and make the ptrace paths use that.
* Peter Waechtler ([email protected]) wrote:
> On Thursday, April 22, 2004, at 09:53PM, Chris Wright <[email protected]> wrote:
> >This patch breaks various ptrace() checks.
>
> Care to share your wisdom? No? Then please don't reply
- current->mm->dumpable = 0;
+ current->mm->dump_as_root = 1;
Changes like that break ptrace authentication checks. Look more closely
at what mm->dumpable is used for, you'll see checks like:
if (!task->mm->dumpable && !capable(CAP_SYS_PTRACE))
goto out;
BTW, putting the patch inline makes it easier to comment directly on the
patch.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
> While it's more secure to not dump core at all if the
> program has switched euid, it's also very unpractical.
> Since only programs started from root, being setuid
> root or have CAP_SETUID it's far more practical to
> dump as root.root mode 600. This is the bahavior
> of Solaris.
Solaris can keep their security holes.
Consider a setuid core dump on removable media which
is user-controlled.
Also consider filesystems that don't store full security
data, like vfat and smb/cifs.
Core dumps to remote filesystems are a problem in
general, because the server might not implement the
type of security you expect it to implement.
Here's a better idea: add a sysctl for insecure core
dumps. When set, dump all cores as root.root mode 444.
Ignore directory permissions when doing so, so that
forcing dumps into a MacOS-style /cores directory does
not require that users be able to access it normally.
This lets appropriately authorized users debug setuid
apps and get support for them without adding security
holes like Solaris has.
Am Fr, 2004-04-23 um 19.10 schrieb Chris Wright:
> * Peter Waechtler ([email protected]) wrote:
> > On Thursday, April 22, 2004, at 09:53PM, Chris Wright <[email protected]> wrote:
> > >This patch breaks various ptrace() checks.
> >
> > Care to share your wisdom? No? Then please don't reply
>
> - current->mm->dumpable = 0;
> + current->mm->dump_as_root = 1;
>
> Changes like that break ptrace authentication checks. Look more closely
> at what mm->dumpable is used for, you'll see checks like:
>
> if (!task->mm->dumpable && !capable(CAP_SYS_PTRACE))
> goto out;
added checks like
if ((task->mm->dump_as_root || !task->mm->dumpable)
&& !capable(CAP_SYS_PTRACE))
in kernel/ptrace.c and fs/proc/base.c
I guess the checks will get more and more complicated, perhaps
we should invent a special flag dont_ptrace or so
Am Fr, 2004-04-23 um 17.35 schrieb Albert Cahalan:
> > While it's more secure to not dump core at all if the
> > program has switched euid, it's also very unpractical.
> > Since only programs started from root, being setuid
> > root or have CAP_SETUID it's far more practical to
> > dump as root.root mode 600. This is the bahavior
> > of Solaris.
>
> Solaris can keep their security holes.
>
I checked (older) versions on
HP-UX, True64, AiX, MacOsX
HP-UX didn't dump core on a seteuid 0->n prog
Aix,MacOsX and True64 dumped core with ownership of user
I could check Irix
> Consider a setuid core dump on removable media which
> is user-controlled.
>
boot into rescue system...
> Also consider filesystems that don't store full security
> data, like vfat and smb/cifs.
>
> Core dumps to remote filesystems are a problem in
> general, because the server might not implement the
> type of security you expect it to implement.
>
mkdir /var/cores
chmod a+rwx,o+t /var/cores
echo /var/cores/%e.core.%p > /proc/sys/kernel/core_pattern
>
> Here's a better idea: add a sysctl for insecure core
> dumps. When set, dump all cores as root.root mode 444.
> Ignore directory permissions when doing so, so that
> forcing dumps into a MacOS-style /cores directory does
> not require that users be able to access it normally.
> This lets appropriately authorized users debug setuid
> apps and get support for them without adding security
> holes like Solaris has.
It's tunable via coreadm
troll, troll
On Fri, 2004-04-23 at 15:14, Peter W?chtler wrote:
> Am Fr, 2004-04-23 um 17.35 schrieb Albert Cahalan:
> > > While it's more secure to not dump core at all if the
> > > program has switched euid, it's also very unpractical.
> > > Since only programs started from root, being setuid
> > > root or have CAP_SETUID it's far more practical to
> > > dump as root.root mode 600. This is the bahavior
> > > of Solaris.
> >
> > Solaris can keep their security holes.
> >
>
> I checked (older) versions on
>
> HP-UX, True64, AiX, MacOsX
>
> HP-UX didn't dump core on a seteuid 0->n prog
> Aix,MacOsX and True64 dumped core with ownership of user
> I could check Irix
It would be nice of you to do so, then issue some
sort of security alert.
> > Consider a setuid core dump on removable media which
> > is user-controlled.
>
> boot into rescue system...
If you're suggesting that user-controlled media implies
the ability to boot via that media, you're very wrong.
a. LinuxBIOS
b. OpenFirmware (Mac or Sun) with boot password
c. no floppy, and a Zip drive on BIOS-disabled SCSI
d. parallel port Zip drive
e. FireWire w/o BIOS support
(the "rip computer open" option is either physically
blocked or there is a human nearby that would notice)
> > Also consider filesystems that don't store full security
> > data, like vfat and smb/cifs.
> >
> > Core dumps to remote filesystems are a problem in
> > general, because the server might not implement the
> > type of security you expect it to implement.
> >
>
> mkdir /var/cores
> chmod a+rwx,o+t /var/cores
> echo /var/cores/%e.core.%p > /proc/sys/kernel/core_pattern
Sure, but you shouldn't assume that all admins know
to do this. The default must be secure.
> > Here's a better idea: add a sysctl for insecure core
> > dumps. When set, dump all cores as root.root mode 444.
> > Ignore directory permissions when doing so, so that
> > forcing dumps into a MacOS-style /cores directory does
> > not require that users be able to access it normally.
> > This lets appropriately authorized users debug setuid
> > apps and get support for them without adding security
> > holes like Solaris has.
>
> It's tunable via coreadm
>
> troll, troll
You said "This is the bahavior of Solaris." and I took
that for the truth. Now you say it's tunable. Well, OK
then, so Solaris _doesn't_ have an insecure config as
it comes from Sun? I have no problem with supporting an
insecure config, but you were suggesting that it would
be the new default (and only) behavior.