Prev: omap_hsmmc: Add erase capability
Next: [announce] gujin gpl bootloader version 2.8.2
From: Alan Cox on 16 Jun 2010 19:00
> As Linux grows in popularity, it will become a larger target for
> malware. One particularly troubling weakness of the Linux process
> interfaces is that a single user is able to examine the memory and
> running state of any of their processes. For example, if one application

And this will help how - or don't you care about procfs.

> + /* require ptrace target be a child of ptracer on attach */
> + if (mode == PTRACE_MODE_ATTACH && ptrace_scope &&
> + !capable(CAP_SYS_PTRACE)) {
> + struct task_struct *walker = task;
> + int rc = 0;
> +
> + read_lock(&tasklist_lock);
> + while (walker->pid > 0) {
> + if (walker == current)
> + break;
> + walker = walker->parent;
> + }
> + if (walker->pid == 0)
> + rc = -EPERM;
> + read_unlock(&tasklist_lock);
> + if (rc)
> + return rc;
> + }


But even if it wasn't pointless this would be the wrong way to do it.

Other distributions do this sensibly by using things like SELinux which
can describe the relationships in ways that matter and also arbitrate
other access paths beyond ptrace which can be used for the same purpose.

And even if you don't care about using the same security stuff the rest
of the world is using to solve the problem this like the other half baked
stuff you posted for links belongs as a security module.

If you'd put it all in security/ubuntu/grsecurity or similar probably
nobody would care too much. The hooks are there so you can do different
things with security policy without making a mess for anyone else.

See ptrace_access_check, ptrace_traceme, and do remember /proc/mem -
which you'll find if you use the proper security hooks is already covered
for you.

So NAK. If you want to use bits of grsecurity then please just write
yourselves a grsecurity kernel module that uses the security hooks
properly and stop messing up the core code. It's all really quite simple,
the infrastrucuture is there, so use it.

Alan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Roland McGrath on 16 Jun 2010 19:20
This constraint seems fairly insane to me, but I don't really care about
people using sysctl to enable insane things if that's what floats your
boat. GDB's "attach", "strace -p", etc. are pretty normal (and highly
useful) things for ordinary users debugging their own programs.

I tend to think that this constraint offers more a delusion of security
than much real protection. But I'm too lazy to try to come up with a more
contorted exploit that this wouldn't prevent, so I won't belabor the point.

I think those who are actually paranoid would use something more meaningful
via the LSM ptrace check, like SELinux with a policy that only permits
known debugger applications to use ptrace. Aside from SELinux, it could
also be done with a new capability like CAP_PTRACE_MINE and filesystem
capabilities on installed debugger application binaries, for example.

You've described this as allowing ptrace on "children", but really it's
"unorphaned descendants", i.e. also children of children, etc.

I don't think "task->pid > 0" is a sort of check that is used elsewhere in
the kernel for this. Perhaps "task == &init_task" would be better.

It's not immediately obvious to me how this interacts with pid_ns, or
should. Probably it shouldn't pay attention to pid_ns, as it doesn't.
But I think it merits an explicit statement of intent about that.

I suspect you really want to test same_thread_group(walker, current).
You don't actually mean to rule out a debugger that forks children with
one thread and calls ptrace with another, do you?

Oh, and surely you meant real_parent. Off hand I think that might only
really add up to a different constraint if you had some ptrace attaches
already live when you did set the sysctl flag. But I have the vague
sensation I'm overlooking some other arcane case. And it just doesn't
logically match the stated intent of the thing to depend on parent
rather than real_parent.


Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kees Cook on 16 Jun 2010 19:30
Hi Alan,

On Thu, Jun 17, 2010 at 12:01:20AM +0100, Alan Cox wrote:
> > As Linux grows in popularity, it will become a larger target for
> > malware. One particularly troubling weakness of the Linux process
> > interfaces is that a single user is able to examine the memory and
> > running state of any of their processes. For example, if one application
>
> And this will help how - or don't you care about procfs.

I'm not sure I follow this comment. Sensitive things in /proc/$PID/* are
already protected by ptrace_may_access() with mode == ATTACH.

> Other distributions do this sensibly by using things like SELinux which
> can describe the relationships in ways that matter and also arbitrate
> other access paths beyond ptrace which can be used for the same purpose.

Certainly. PTRACE can already be confined by SELinux and AppArmor. I'm
looking for a general approach that doesn't require a system builder to
create MAC policies for unknown software. I want to define a common core
behavior.

> And even if you don't care about using the same security stuff the rest
> of the world is using to solve the problem this like the other half baked
> stuff you posted for links belongs as a security module.

The LSM isn't stackable, so I can't put it there and choose this and
SELinux (for the case of software-without-a-policy).

> If you'd put it all in security/ubuntu/grsecurity or similar probably
> nobody would care too much. The hooks are there so you can do different
> things with security policy without making a mess for anyone else.

I'm not clear how this is "a mess for anyone else" when it defaults to
the classic PTRACE behavior. PTRACE itself is dangerous, so it's not
unreasonable to start inching away from it.

> So NAK. If you want to use bits of grsecurity then please just write
> yourselves a grsecurity kernel module that uses the security hooks
> properly and stop messing up the core code. It's all really quite simple,
> the infrastrucuture is there, so use it.

There is no infrastructure to selectively choose these general-purpose
features. This is why there is a sysctl. It's a global behavioral
change.

Since LSMs aren't arbitrarily stackable, asking me to move the code into
a new LSM isn't a particularly actionable suggestion.

-Kees

--
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kees Cook on 16 Jun 2010 19:50
Hi,

On Wed, Jun 16, 2010 at 04:10:06PM -0700, Roland McGrath wrote:
> This constraint seems fairly insane to me, but I don't really care about
> people using sysctl to enable insane things if that's what floats your
> boat. GDB's "attach", "strace -p", etc. are pretty normal (and highly
> useful) things for ordinary users debugging their own programs.

Right, but I don't think "ordinary users" debug their own programs.
Ordinary developers and sysadmin do, absolutely, and for them, this
sysctl would probably stay set to 0.

> I tend to think that this constraint offers more a delusion of security
> than much real protection. But I'm too lazy to try to come up with a more
> contorted exploit that this wouldn't prevent, so I won't belabor the point.

Well, I don't want to present it as something it's not. It's only
designed to block access to what is immediately in memory. It certainly
won't stop an attacker from tricking a user into divulging credentials
directly or launching a process and then ptracing it, but it would put
a stop to an automated worm that did not need to go phishing.

> I think those who are actually paranoid would use something more meaningful
> via the LSM ptrace check, like SELinux with a policy that only permits
> known debugger applications to use ptrace. Aside from SELinux, it could
> also be done with a new capability like CAP_PTRACE_MINE and filesystem
> capabilities on installed debugger application binaries, for example.

This has been the area I've run into the most. I like the idea of a
semi-privileged capability like you suggest. It would solve a number
of iffy spots, like KDE and Chrome that fork/exec a debugger from the
crashing process and attach back to it. Those programs could be given
fscap for CAP_PTRACE_MINE, or something. Though, honestly, just trying to
get rid of PTRACE seems like the better place to spend time.

> You've described this as allowing ptrace on "children", but really it's
> "unorphaned descendants", i.e. also children of children, etc.

Right, I should say "descendants", which is the correct intent.

> I don't think "task->pid > 0" is a sort of check that is used elsewhere in
> the kernel for this. Perhaps "task == &init_task" would be better.

Is this correct for pid_ns? I thought pid 1 (regardless of NS) would have
a NULL parent?

> It's not immediately obvious to me how this interacts with pid_ns, or
> should. Probably it shouldn't pay attention to pid_ns, as it doesn't.
> But I think it merits an explicit statement of intent about that.

Okay, I can do that.

> I suspect you really want to test same_thread_group(walker, current).
> You don't actually mean to rule out a debugger that forks children with
> one thread and calls ptrace with another, do you?

Won't they ultimately have the same parent, though?

> Oh, and surely you meant real_parent. Off hand I think that might only
> really add up to a different constraint if you had some ptrace attaches
> already live when you did set the sysctl flag. But I have the vague
> sensation I'm overlooking some other arcane case. And it just doesn't
> logically match the stated intent of the thing to depend on parent
> rather than real_parent.

Oh, yes. That seems right. I can fix that.

-Kees

--
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Roland McGrath on 16 Jun 2010 20:20
> Though, honestly, just trying to get rid of PTRACE seems like the better
> place to spend time.

Crushing irony of telling *me* this duly noted. ;-)
I am not really sure what deeply different set of security constraints
you envision on any other kind of new debugger interface that would be
any different for the concerns you've expressed, though.

> > I don't think "task->pid > 0" is a sort of check that is used elsewhere in
> > the kernel for this. Perhaps "task == &init_task" would be better.
>
> Is this correct for pid_ns? I thought pid 1 (regardless of NS) would have
> a NULL parent?

Don't ask me. I just mentioned pid_ns to get those who really know about
it to feel obliged to review your code.

> > I suspect you really want to test same_thread_group(walker, current).
> > You don't actually mean to rule out a debugger that forks children with
> > one thread and calls ptrace with another, do you?
>
> Won't they ultimately have the same parent, though?

Sure, those debugger threads will have the same parent, such as the shell
that spawned the debugger. But your "security" check is that the caller of
ptrace is a direct ancestor of the tracee. The ancestry of that ptrace
caller is immaterial.


Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2 3 4 5 6 7
Prev: omap_hsmmc: Add erase capability
Next: [announce] gujin gpl bootloader version 2.8.2