Message ID | 20100512222230.GL4433@outflux.net |
---|---|
State | Accepted |
Delegated to: | Leann Ogasawara |
Headers | show |
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
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),
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(-)