UBUNTU: SAUCE: ptrace: restrict ptrace scope to children

As Linux grows in popularity, it will become a growing 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
(e.g. Empathy) was compromised, it would be possible for an attacker to
attach to other processes (e.g. Firefox) to extract additional credentials
and continue to expand the scope of their attack.

For a solution, some applications use prctl() to specifically disallow
such PTRACE attachment (e.g. ssh-agent). A more general solution is to
only allow PTRACE directly from a parent to a child process (i.e. direct
gdb and strace still work), or as the root user (i.e. gdb BIN PID,
and strace -p PID still work as root).

This patch is based on the patch in grsecurity. I have added a sysctl to
toggle the behavior back to the old scope via /proc/sys/kernel/ptrace_scope.

Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
 kernel/ptrace.c |   24 ++++++++++++++++++++++++
 kernel/sysctl.c |   10 ++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

On Wed, 2010-05-12 at 15:22 -0700, Kees Cook wrote:

> As Linux grows in popularity, it will become a growing 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
> (e.g. Empathy) was compromised, it would be possible for an attacker to
> attach to other processes (e.g. Firefox) to extract additional credentials
> and continue to expand the scope of their attack.
> 
This is completely possible anyway, even with your patch.  I would do
the following:

 - send SIGSTOP to the compositor to disable screen updates

 - send command to firefox to save browser state and exit
   (or SIGKILL)

 - fork/exec firefox again (will reappear on the screen as it was
   before)

 - firefox is now your child, ptrace

 - send SIGCONT to the compositor to resume screen updates

Firefox is now being ptraced, but the user never knows what happens.


So your patch adds inconvenience for no additional security, thus I
object to this.

Scott

Hi Scott,

On Thu, May 13, 2010 at 10:13:27AM +0200, Scott James Remnant wrote:
> On Wed, 2010-05-12 at 15:22 -0700, Kees Cook wrote:
> 
> > As Linux grows in popularity, it will become a growing 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
> > (e.g. Empathy) was compromised, it would be possible for an attacker to
> > attach to other processes (e.g. Firefox) to extract additional credentials
> > and continue to expand the scope of their attack.
> > 
> This is completely possible anyway, even with your patch.  I would do
> the following:
> 
>  - send SIGSTOP to the compositor to disable screen updates
> 
>  - send command to firefox to save browser state and exit
>    (or SIGKILL)
> 
>  - fork/exec firefox again (will reappear on the screen as it was
>    before)
> 
>  - firefox is now your child, ptrace
> 
>  - send SIGCONT to the compositor to resume screen updates
> 
> Firefox is now being ptraced, but the user never knows what happens.

This patch is specifically for blocking access to processes that hold
credentials in memory but don't set prctl[1].  This change pro-actively
helps protect such scenarios.

Firefox was just an example.  I could have just as well used other stuff
that doesn't save state as well.  I totally admit that trying to defend
the user from the user is a losing game, but I do feel that layered
defense is still worth it when it is both configurable and doesn't cause
problems for the general case.

> So your patch adds inconvenience for no additional security, thus I
> object to this.

Having developers/admins use sudo or add a file to /etc/sysctl.d to
restore the original PTRACE behavior isn't much of an inconvenience.

-Kees

[1] https://launchpad.net/bugs/572045

On Thu, 2010-05-13 at 01:33 -0700, Kees Cook wrote:

> This patch is specifically for blocking access to processes that hold
> credentials in memory but don't set prctl[1].  This change pro-actively
> helps protect such scenarios.
> 
Why don't you just fix the apps concerned to use prctl() or to lock
their memory?

> > So your patch adds inconvenience for no additional security, thus I
> > object to this.
> 
> Having developers/admins use sudo or add a file to /etc/sysctl.d to
> restore the original PTRACE behavior isn't much of an inconvenience.
> 
I disagree in the strongest terms.

If security gets in the way of people using their computers, they will
simply disable it.

  Fedora's SELinux policy

  Windows' UAC

Being able to debug processes running as your user is an important tool
for developers.  This will annoy the hell out of them, and they will
disable your patch.

That means we'll get less testing of your patch, and potential problems
will be hidden from us until release.

I really dislike these meme that it's acceptable for developers to have
to add toggles or files to restore a system to a state they can work
with; not because we should be developer focussed, but because our
developers are still our primary testers.


I would far prefer to individual apps jailed from each other entirely,
empathy shouldn't be *able* to exec firefox, etc.

Scott

On Thu, May 13, 2010 at 10:39:48AM +0200, Scott James Remnant wrote:
> On Thu, 2010-05-13 at 01:33 -0700, Kees Cook wrote:
> > This patch is specifically for blocking access to processes that hold
> > credentials in memory but don't set prctl[1].  This change pro-actively
> > helps protect such scenarios.
> > 
> Why don't you just fix the apps concerned to use prctl() or to lock
> their memory?

Well, I intend to, which is why the bug exists, but there will always be
new software.  That's why this is considered "pro-active" defense.

> > Having developers/admins use sudo or add a file to /etc/sysctl.d to
> > restore the original PTRACE behavior isn't much of an inconvenience.
> > 
> I disagree in the strongest terms.

Understood.

> If security gets in the way of people using their computers, they will
> simply disable it.
> 
>   Fedora's SELinux policy
>   Windows' UAC

I couldn't agree more, and am very aware of these situations.  I just
disagree with you about the level of inconvenience and the size of the
affected audience.

> Being able to debug processes running as your user is an important tool
> for developers.  This will annoy the hell out of them, and they will
> disable your patch.

They will disable the sysctl setting or gain CAP_SYS_PTRACE temporarily.

> That means we'll get less testing of your patch, and potential problems
> will be hidden from us until release.

I believe the set of people testing maverick that does not disable this
feature system-wide will be sufficiently large to gain meaningful testing.

> I really dislike these meme that it's acceptable for developers to have
> to add toggles or files to restore a system to a state they can work
> with; not because we should be developer focussed, but because our
> developers are still our primary testers.

It sounds like you object to the default setting, not the feature itself.

> I would far prefer to individual apps jailed from each other entirely,
> empathy shouldn't be *able* to exec firefox, etc.

I totally agree.  This isn't possible now, so I am proposing improvements
to the existing methodologies.

-Kees

On 05/13/2010 02:59 AM, Kees Cook wrote:
> On Thu, May 13, 2010 at 10:39:48AM +0200, Scott James Remnant wrote:
>> On Thu, 2010-05-13 at 01:33 -0700, Kees Cook wrote:
>>> This patch is specifically for blocking access to processes that hold
>>> credentials in memory but don't set prctl[1].  This change pro-actively
>>> helps protect such scenarios.
>>>
>> Why don't you just fix the apps concerned to use prctl() or to lock
>> their memory?
>
> Well, I intend to, which is why the bug exists, but there will always be
> new software.  That's why this is considered "pro-active" defense.
>
>>> Having developers/admins use sudo or add a file to /etc/sysctl.d to
>>> restore the original PTRACE behavior isn't much of an inconvenience.
>>>
>> I disagree in the strongest terms.
>
> Understood.
>
>> If security gets in the way of people using their computers, they will
>> simply disable it.
>>
>>    Fedora's SELinux policy
>>    Windows' UAC
>
> I couldn't agree more, and am very aware of these situations.  I just
> disagree with you about the level of inconvenience and the size of the
> affected audience.
>
>> Being able to debug processes running as your user is an important tool
>> for developers.  This will annoy the hell out of them, and they will
>> disable your patch.
>
> They will disable the sysctl setting or gain CAP_SYS_PTRACE temporarily.
>
>> That means we'll get less testing of your patch, and potential problems
>> will be hidden from us until release.
>
> I believe the set of people testing maverick that does not disable this
> feature system-wide will be sufficiently large to gain meaningful testing.
>
>> I really dislike these meme that it's acceptable for developers to have
>> to add toggles or files to restore a system to a state they can work
>> with; not because we should be developer focussed, but because our
>> developers are still our primary testers.
>
> It sounds like you object to the default setting, not the feature itself.
>
>> I would far prefer to individual apps jailed from each other entirely,
>> empathy shouldn't be *able* to exec firefox, etc.
>
> I totally agree.  This isn't possible now, so I am proposing improvements
> to the existing methodologies.
>
> -Kees
>

Is it safe to assume that developers that might encounter this minor 
restriction would also have ubuntu-dev-tools installed? We could add 
something to that package that swizzles /proc/sys/kernel/ptrace_scope, 
thereby avoiding inconvenience to developers while still providing a 
good test for the normal install.

rtg

On 05/12/2010 04:22 PM, Kees Cook wrote:
> As Linux grows in popularity, it will become a growing 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
> (e.g. Empathy) was compromised, it would be possible for an attacker to
> attach to other processes (e.g. Firefox) to extract additional credentials
> and continue to expand the scope of their attack.
>
> For a solution, some applications use prctl() to specifically disallow
> such PTRACE attachment (e.g. ssh-agent). A more general solution is to
> only allow PTRACE directly from a parent to a child process (i.e. direct
> gdb and strace still work), or as the root user (i.e. gdb BIN PID,
> and strace -p PID still work as root).
>
> This patch is based on the patch in grsecurity. I have added a sysctl to
> toggle the behavior back to the old scope via /proc/sys/kernel/ptrace_scope.
>
> Signed-off-by: Kees Cook<kees.cook@canonical.com>
> ---
>   kernel/ptrace.c |   24 ++++++++++++++++++++++++
>   kernel/sysctl.c |   10 ++++++++++
>   2 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 42ad8ae..ad80b43 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -24,6 +24,8 @@
>   #include<linux/uaccess.h>
>   #include<linux/regset.h>
>
> +/* sysctl for defining allowed scope of PTRACE */
> +int ptrace_scope = 1;
>
>   /*
>    * ptrace a task: make the debugger its new parent and
> @@ -129,6 +131,10 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>   	 * ptrace_attach denies several cases that /proc allows
>   	 * because setting up the necessary parent/child relationship
>   	 * or halting the specified task is impossible.
> +	 *
> +	 * PTRACE scope can be define as:
> +	 *  0 - classic: CAP_SYS_PTRACE and same uid can ptrace non-setuid
> +	 *  1 - restricted: as above, but only children of ptracing process
>   	 */
>   	int dumpable = 0;
>   	/* Don't let security modules deny introspection */
> @@ -152,6 +158,24 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>   		dumpable = get_dumpable(task->mm);
>   	if (!dumpable&&  !capable(CAP_SYS_PTRACE))
>   		return -EPERM;
> +	if (ptrace_scope&&  !capable(CAP_SYS_PTRACE)) {
> +		/* require ptrace target be a child of ptracer */
> +		struct task_struct *tmp = task;
> +		struct task_struct *curtemp = current;
> +		int rc = 0;
> +
> +		read_lock(&tasklist_lock);
> +		while (tmp->pid>  0) {
> +			if (tmp == curtemp)
> +				break;
> +			tmp = tmp->parent;
> +		}
> +		if (tmp->pid == 0)
> +			rc = -EPERM;
> +		read_unlock(&tasklist_lock);
> +		if (rc)
> +			return rc;
> +	}
>
>   	return security_ptrace_access_check(task, mode);
>   }
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 4f3ffd0..992eba9 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -84,6 +84,7 @@ extern int sysctl_panic_on_oom;
>   extern int sysctl_oom_kill_allocating_task;
>   extern int sysctl_oom_dump_tasks;
>   extern int max_threads;
> +extern int ptrace_scope;
>   extern int core_uses_pid;
>   extern int suid_dumpable;
>   extern int weak_sticky_symlinks;
> @@ -382,6 +383,15 @@ static struct ctl_table kern_table[] = {
>   		.proc_handler	= proc_dointvec,
>   	},
>   	{
> +		.procname	= "ptrace_scope",
> +		.data		=&ptrace_scope,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		=&zero,
> +		.extra2		=&one,
> +	},
> +	{
>   		.procname	= "core_uses_pid",
>   		.data		=&core_uses_pid,
>   		.maxlen		= sizeof(int),

Arguments for the inconvenience or effectiveness of this patch aside, it 
appears to do what you'd expect. See attached.

Acked-by: Tim Gardner <tim.gardner@canonical.com>

On Wed, 2010-05-12 at 15:22 -0700, Kees Cook wrote:
> As Linux grows in popularity, it will become a growing 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
> (e.g. Empathy) was compromised, it would be possible for an attacker to
> attach to other processes (e.g. Firefox) to extract additional credentials
> and continue to expand the scope of their attack.
> 
> For a solution, some applications use prctl() to specifically disallow
> such PTRACE attachment (e.g. ssh-agent). A more general solution is to
> only allow PTRACE directly from a parent to a child process (i.e. direct
> gdb and strace still work), or as the root user (i.e. gdb BIN PID,
> and strace -p PID still work as root).
> 
> This patch is based on the patch in grsecurity. I have added a sysctl to
> toggle the behavior back to the old scope via /proc/sys/kernel/ptrace_scope.
> 
> Signed-off-by: Kees Cook <kees.cook@canonical.com>
> ---
>  kernel/ptrace.c |   24 ++++++++++++++++++++++++
>  kernel/sysctl.c |   10 ++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 42ad8ae..ad80b43 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -24,6 +24,8 @@
>  #include <linux/uaccess.h>
>  #include <linux/regset.h>
>  
> +/* sysctl for defining allowed scope of PTRACE */
> +int ptrace_scope = 1;
>  
>  /*
>   * ptrace a task: make the debugger its new parent and
> @@ -129,6 +131,10 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>  	 * ptrace_attach denies several cases that /proc allows
>  	 * because setting up the necessary parent/child relationship
>  	 * or halting the specified task is impossible.
> +	 *
> +	 * PTRACE scope can be define as:
> +	 *  0 - classic: CAP_SYS_PTRACE and same uid can ptrace non-setuid
> +	 *  1 - restricted: as above, but only children of ptracing process
>  	 */
>  	int dumpable = 0;
>  	/* Don't let security modules deny introspection */
> @@ -152,6 +158,24 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>  		dumpable = get_dumpable(task->mm);
>  	if (!dumpable && !capable(CAP_SYS_PTRACE))
>  		return -EPERM;
> +	if (ptrace_scope && !capable(CAP_SYS_PTRACE)) {
> +		/* require ptrace target be a child of ptracer */
> +		struct task_struct *tmp = task;
> +		struct task_struct *curtemp = current;

Why create a new variable just to store current? I think it would be
more readable to just use current where you use curtemp. I don't think
current should change from under you when you're here.

> +		int rc = 0;
> +
> +		read_lock(&tasklist_lock);
> +		while (tmp->pid > 0) {
> +			if (tmp == curtemp)
> +				break;
> +			tmp = tmp->parent;
> +		}
> +		if (tmp->pid == 0)
> +			rc = -EPERM;
> +		read_unlock(&tasklist_lock);
> +		if (rc)
> +			return rc;
> +	}
>  
>  	return security_ptrace_access_check(task, mode);
>  }

Hi Tim,

On Wed, May 26, 2010 at 01:31:07PM -0600, Tim Gardner wrote:
> Is it safe to assume that developers that might encounter this minor
> restriction would also have ubuntu-dev-tools installed? We could add
> something to that package that swizzles
> /proc/sys/kernel/ptrace_scope, thereby avoiding inconvenience to
> developers while still providing a good test for the normal install.

That's certainly a good idea.  I can put the question to ubuntu-devel.  The
two classes of people hit by this are "developers" and "sysadmins", both
really nebulous classes.  If ubuntu-dev-tools adds a setting to
/etc/sysctl.d/, that could work.  For sysadmins, I still think it would be
best to retrain them to either use "sudo" or temporarily turn on the
ptrace_scope setting, as the rest of their system probably shouldn't have
ptrace enabled normally.  Regardless, I expect a lively discussion.  :)

-Kees

Hi Chase,

On Wed, May 26, 2010 at 04:15:25PM -0400, Chase Douglas wrote:
> > +	if (ptrace_scope && !capable(CAP_SYS_PTRACE)) {
> > +		/* require ptrace target be a child of ptracer */
> > +		struct task_struct *tmp = task;
> > +		struct task_struct *curtemp = current;
> 
> Why create a new variable just to store current? I think it would be
> more readable to just use current where you use curtemp. I don't think
> current should change from under you when you're here.
> 
> > +		int rc = 0;
> > +
> > +		read_lock(&tasklist_lock);
> > +		while (tmp->pid > 0) {
> > +			if (tmp == curtemp)
> > +				break;
> > +			tmp = tmp->parent;
> > +		}
> > +		if (tmp->pid == 0)
> > +			rc = -EPERM;
> > +		read_unlock(&tasklist_lock);

That's a fair point -- I guess we're under lock the entire time.  I'd be
fine to change it if other people agree it's safe looking.

-Kees

On Wed, 2010-05-26 at 13:50 -0700, Kees Cook wrote:
> On Wed, May 26, 2010 at 04:15:25PM -0400, Chase Douglas wrote:
> > > +	if (ptrace_scope && !capable(CAP_SYS_PTRACE)) {
> > > +		/* require ptrace target be a child of ptracer */
> > > +		struct task_struct *tmp = task;
> > > +		struct task_struct *curtemp = current;
> > 
> > Why create a new variable just to store current? I think it would be
> > more readable to just use current where you use curtemp. I don't think
> > current should change from under you when you're here.
> > 
> > > +		int rc = 0;
> > > +
> > > +		read_lock(&tasklist_lock);
> > > +		while (tmp->pid > 0) {
> > > +			if (tmp == curtemp)
> > > +				break;
> > > +			tmp = tmp->parent;
> > > +		}
> > > +		if (tmp->pid == 0)
> > > +			rc = -EPERM;
> > > +		read_unlock(&tasklist_lock);
> 
> That's a fair point -- I guess we're under lock the entire time.  I'd be
> fine to change it if other people agree it's safe looking.

I don't think the lock here would necessarily affect current; its safe
through a separate mechanism. This code path I assume is called through
a syscall into the kernel. While this is executing, current will always
point to the task that called the syscall. If a context switch happens,
current gets changed to a different process at the same time the
execution pointer is switched to the other process somewhere else in the
kernel or userspace, but when we context switch back to this process to
continue, current once again points to the correct task.

That said, the above is probably an exercise in semantics on a
non-completely-preemptible kernel where we can't context switch while a
lock is held. Our kernels are also only voluntarily preemptible, so
there's no chance the context switch could occur anywhere in this code
anyways.

-- Chase

It seems the implementation itself is not so much questioned here than the
resulting behaviour (beside I probably would name the variable ptrace_testricted
to be clearer, though that is a bit nitpick).

From the resulting behaviour itself I am probably less strongly biased. The
inconvenience of running 'sudo strace -p#' I don't see as that bad. But on the
other hand the security gain is maybe not worth the effort. What prevents
malware from just adding a wrapper script to target applications while ptracing
them as their parent?

If the answer is nothing I think this just adds too much false sense of
security. And for Lucid SRU the default would likely need to be classic anyway
(as long as it cannot guarantee to completely get rid of the issue, which it
does not seem to be able).

Could the goal be reached through extending apparmor to impose restrictions like
that based on profiles? (maybe that already exists and I am just to be blamed
for ignorance on it).

-Stefan

On 05/13/2010 12:22 AM, Kees Cook wrote:
> As Linux grows in popularity, it will become a growing 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
> (e.g. Empathy) was compromised, it would be possible for an attacker to
> attach to other processes (e.g. Firefox) to extract additional credentials
> and continue to expand the scope of their attack.
> 
> For a solution, some applications use prctl() to specifically disallow
> such PTRACE attachment (e.g. ssh-agent). A more general solution is to
> only allow PTRACE directly from a parent to a child process (i.e. direct
> gdb and strace still work), or as the root user (i.e. gdb BIN PID,
> and strace -p PID still work as root).
> 
> This patch is based on the patch in grsecurity. I have added a sysctl to
> toggle the behavior back to the old scope via /proc/sys/kernel/ptrace_scope.
> 
> Signed-off-by: Kees Cook <kees.cook@canonical.com>
> ---
>  kernel/ptrace.c |   24 ++++++++++++++++++++++++
>  kernel/sysctl.c |   10 ++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 42ad8ae..ad80b43 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -24,6 +24,8 @@
>  #include <linux/uaccess.h>
>  #include <linux/regset.h>
>  
> +/* sysctl for defining allowed scope of PTRACE */
> +int ptrace_scope = 1;
>  
>  /*
>   * ptrace a task: make the debugger its new parent and
> @@ -129,6 +131,10 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>  	 * ptrace_attach denies several cases that /proc allows
>  	 * because setting up the necessary parent/child relationship
>  	 * or halting the specified task is impossible.
> +	 *
> +	 * PTRACE scope can be define as:
> +	 *  0 - classic: CAP_SYS_PTRACE and same uid can ptrace non-setuid
> +	 *  1 - restricted: as above, but only children of ptracing process
>  	 */
>  	int dumpable = 0;
>  	/* Don't let security modules deny introspection */
> @@ -152,6 +158,24 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>  		dumpable = get_dumpable(task->mm);
>  	if (!dumpable && !capable(CAP_SYS_PTRACE))
>  		return -EPERM;
> +	if (ptrace_scope && !capable(CAP_SYS_PTRACE)) {
> +		/* require ptrace target be a child of ptracer */
> +		struct task_struct *tmp = task;
> +		struct task_struct *curtemp = current;
> +		int rc = 0;
> +
> +		read_lock(&tasklist_lock);
> +		while (tmp->pid > 0) {
> +			if (tmp == curtemp)
> +				break;
> +			tmp = tmp->parent;
> +		}
> +		if (tmp->pid == 0)
> +			rc = -EPERM;
> +		read_unlock(&tasklist_lock);
> +		if (rc)
> +			return rc;
> +	}
>  
>  	return security_ptrace_access_check(task, mode);
>  }
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 4f3ffd0..992eba9 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -84,6 +84,7 @@ extern int sysctl_panic_on_oom;
>  extern int sysctl_oom_kill_allocating_task;
>  extern int sysctl_oom_dump_tasks;
>  extern int max_threads;
> +extern int ptrace_scope;
>  extern int core_uses_pid;
>  extern int suid_dumpable;
>  extern int weak_sticky_symlinks;
> @@ -382,6 +383,15 @@ static struct ctl_table kern_table[] = {
>  		.proc_handler	= proc_dointvec,
>  	},
>  	{
> +		.procname	= "ptrace_scope",
> +		.data		= &ptrace_scope,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= &zero,
> +		.extra2		= &one,
> +	},
> +	{
>  		.procname	= "core_uses_pid",
>  		.data		= &core_uses_pid,
>  		.maxlen		= sizeof(int),

On Thu, May 27, 2010 at 02:56:52PM +0200, Stefan Bader wrote:
> It seems the implementation itself is not so much questioned here than the
> resulting behaviour (beside I probably would name the variable ptrace_testricted
> to be clearer, though that is a bit nitpick).

My intention is to add two more additional scopes: root-only ("2"), and
init-only ("3").  This would offer the widest possible range of ptrace
security controls.  These two modes were not trivial (as I need to hook the
actual TRACEME methods too) so I didn't implement them for this early
version of the feature.

> >From the resulting behaviour itself I am probably less strongly biased. The
> inconvenience of running 'sudo strace -p#' I don't see as that bad. But on the
> other hand the security gain is maybe not worth the effort. What prevents
> malware from just adding a wrapper script to target applications while ptracing
> them as their parent?

The protection is for ephemeral credentials; it protects
authentication that happened in the past.  I have no illusions that
the trojan-horse-wrapper is unaffected by this protection, but that
is a slightly different form of attack (attacking authentications in
the future).  Currently there is nothing to prevent a process from
executing arbitrary code in an existing and already running process
owned by the same uid that hasn't been made undumpable.  In the case of
ssh or gpg-agent, this is rather bad.

As an example, an attacker can extend their reach to other systems
by opening new tunnels down existing connected ssh sessions (who did
their authentication in the past).  This is not a theoretical problem;
a tool to do this already exists[1].  Similarly, tools to do arbitrary
code injection via PTRACE exist[2], leaving the possibilities of abuse
wide open.  (I can imagine all sorts of things, jumping into running VNCs,
entering running VMs, manipulating logged-into websites, etc.)

In the case of a wrapper, the user would need to start new processes
before they could be further abused.  I view this as the difference
between a "user assisted/tricked" attack (where the victim must do
something) vs a full-blown automated worm attack (where the victim just
has to have things running).

> Could the goal be reached through extending apparmor to impose restrictions like
> that based on profiles? (maybe that already exists and I am just to be blamed
> for ignorance on it).

AppArmor (and in a similar fashion, SELinux) already restricts PTRACE
to the scope of the running profile, so things that are already confined
are okay as long as the process under confinement is attacker-controlled
(i.e. the scope of the attack against a confined process cannot be
extended with PTRACE -- but nothing stops an unconfined process from
PTRACEing a confined process).  This patch seeks to provide a global
solution to arbitrary PTRACEing, to gain a protection against abuse of
already-running processes.

-Kees

[1] http://www.storm.net.nz/projects/7
[2] http://c-skills.blogspot.com/2009/10/injectso-32bit-x86-port.html

Applied to Maverick master.  I definitely want this to land in time for
Alpha1 in order to get broader testing and feedback.

Thanks,
Leann