lkml.org 
[lkml]   [2015]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH RFC] Allow introspection to already attached ptracer in __ptrace_may_access
Casey Schaufler schreef op 6/01/2015 om 1:17:
> On 1/5/2015 3:47 PM, Kees Cook wrote:
>> On Wed, Dec 24, 2014 at 5:28 AM, Stijn Volckaert
>> <Stijn.Volckaert@elis.ugent.be> wrote:
>>> Hello,
>>>
>>> I ran across the following problem recently but I'm not entirely sure
>>> whether this should be fixed in ptrace or in Yama. I'm working on a
>>> ptrace-based monitor that forks off its own tracee during startup. The
>>> monitor attaches to the tracee and then lets the tracee perform an execve
>>> call. This is much like running a program in gdb.
>>>
>>> My monitor is multi-threaded and uses one monitor thread for every tracee
>>> thread so whenever the tracee forks/vforks/clones, I fire up a new monitor
>>> thread, detach the old monitor thread from the tracee thread and attach the
>>> new monitor thread to the tracee thread.
>>>
>>> I have recently stumbled upon several applications in which the main process
>>> A forks off process B and then immediately exits. Under normal circumstances
>>> the following would happen:
>>>
>>> Monitor[0] --- FORKS OFF ---> Monitor[0]'
>>> Monitor[0] --- PTRACE_ATTACH ---> Monitor[0]'
>>> Monitor[0]' --- EXECVE ---> Process A
>>>
>>> Process A --- FORKS OFF ---> Process B
>>> Monitor[0] --- PTRACE_DETACH ---> Process B
>>> Monitor[1] --- PTRACE_ATTACH ---> Process B
>>>
>>> With Yama enabled (and the scope set to YAMA_SCOPE_RELATIONAL) however, a
>>> few interesting things can (and usually do) happen:
>>>
>>> 1) If Process A dies before Monitor[1] is attached to Process B, the attach
>>> will fail since from Yama's point of view, Process B is no longer a
>>> descendant of Monitor[1]. This problem is probably hard to fix
>>> but I've circumvented it by delaying the death of Process A until Process B
>>> is attached to Monitor[1].
>> Just to make sure I understand this better, "Monitor" is the initial
>> process, and [0] and [1] are separate threads within that process? I
>> would expect B to have Monitor as its parent after A died, but I must
>> be misunderstanding something.
>>
>> Regardless, your "interesting thing 1" is certainly a side-effect of
>> YAMA_SCOPE_RELATIONAL trying to do its job.
Correct. Process B is obviously still descendant from the process that
is ptracing it so you'd
think that this would work with YAMA_SCOPE_RELATIONAL. However, after
process A dies,
/sbin/init becomes the new parent of Process B. If you want to see this
yourselves, you can
reproduce this problem with this little test program:

$ cat ptrace_bug.c
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <string.h>

void daemon_process() {
printf("pre-open\n");
int fd = open("test.txt", O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
if (fd != -1) {
write(fd, "post-open\n", strlen("post-open\n"));
close(fd);
}
exit(0);
}

int main(int argc, char** argv) {
if (fork() == 0)
daemon_process();
return 0;
}
$ gcc -o ptrace_bug ptrace_bug.c

If I strace this on a machine which I've patched with my proposed patch
below, I see the following
output for the "daemon_process":

$ strace -o strace_out -ff ./ptrace_bug
$ cat strace_out.2815
...
write(1, "pre-open\n", 9) = 9
open("test.txt", O_RDWR|O_CREAT, 0600) = 3
write(3, "post-open\n", 10) = 10
...

However, on a machine which I haven't patched and with
YAMA_SCOPE_RELATIONAL, I see:
...
write(1, 0x7f74338c0000, 9) = 9
open(0x4007bd, O_RDWR|O_CREAT, 0600) = 3
write(3, 0x4007c6, 10) = 10
...

This is the exact same problem: you can still read the regs and
stop/resume tracees but
you can't do process_vm_{read,write}v or the ptrace ops that depend on it.

If I now add a pause() call just before the exit of the daemon process,
you'll see the following
after process A dies:

$ ps u
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
stijn 29772 0.0 0.0 4852 820 pts/0 S+ 09:41 0:00 strace
-o strace_out -ff ./ptrace_bug
stijn 29778 0.0 0.0 4300 92 pts/0 S+ 09:41 0:00
./ptrace_bug

$ cat /proc/29778/status
Name: ptrace_bug
State: S (sleeping)
Tgid: 29778
Ngid: 0
Pid: 29778
PPid: 1
TracerPid: 29772

PPid is 1 so /sbin/init is now the real parent. As far as I can tell,
this whole reparenting goes on
in kernel/exit.c due to:

do_exit
-> exit_notify
-> forget_original_parent
-> find_new_reaper

From my viewpoint, it would make more sense to reparent Process B to
the ptracer that originally
forked off Process A. However, looking at the exit.c code, what actually
goes on does seem like
the intended behavior.

>>> 2) More interestingly though, even if Process B does get attached to
>>> Monitor[1], as soon as Process A dies, all process_vm_readv and
>>> process_vm_writev calls on Process B start failing. Any other ptrace
>>> operations peformed on Process B do succeed.
>>>
>>> process_vm_readv|writev use __ptrace_may_access to check whether the
>>> operation is permitted, whereas other ptrace operations (with the exception
>>> of PTRACE_ATTACH) use ptrace_check_attach.
>> Right, process_vm_{read,write}v use PTRACE_MODE_ATTACH (which is what
>> Yama interposes via the LSM entry point in __ptrace_may_access).
>>
>>> To fix this problem, __ptrace_may_access should be forced to return 0 if the
>>> calling process is already attached to the target process.
>>>
>>> The question now is whether or not it's the security module's responsibility
>>> to check whether a tracee relationship is already in place or if ptrace
>>> itself should do it. For the latter case, which seems more logical to me,
>>> you could use the patch below.
>>>
>>> What do you guys think?
>>>
>>> Regards,
>>> Stijn Volckaert
>>>
>>> --
>>>
>>> Signed-off-by: Stijn Volckaert <Stijn.Volckaert@elis.ugent.be>
>>>
>>> --- a/kernel/ptrace.c 2014-12-24 13:53:23.055346526 +0100
>>> +++ b/kernel/ptrace.c 2014-12-24 14:17:20.617824840 +0100
>>> @@ -232,6 +232,9 @@ static int __ptrace_may_access(struct ta
>>> /* Don't let security modules deny introspection */
>>> if (same_thread_group(task, current))
>>> return 0;
>>> + /* Don't deny introspection to already attached ptracer */
>>> + if (!ptrace_check_attach(task, true))
>>> + return 0;
>>> rcu_read_lock();
>>> tcred = __task_cred(task);
>>> if (uid_eq(cred->uid, tcred->euid) &&
>>>
>> I'm nervous to add this (or Oleg's suggestion) generally to
>> __ptrace_may_access, as it would mean other LSMs would stop seeing
>> access checks even when attached. It does seem silly to deny ptrace
>> checks when already attached, but it does change the behavior here.
> An LSM may chose to do checks on a per access basis. Think in terms
> of access checks on read/write instead of open. Smack and SELinux
> do this for some network checks. It is reasonable to think that there
> is a case where a security attribute (or access rule) could change
> between the attach and the access.
>
> Example: You allow the access when the developer mode switch is
> set, but not when it isn't. Someone flips the switch.
That makes sense I guess. However, the problem in my case was not that
the security policy
had change between the "open" and the "read/write" check. Instead, after
the death of
Process A, the security check now lacks the context to see the
relationship between
the tracer and the tracee.

>> If the other LSM folks don't see a problem here, then it should live
>> in the general case. Otherwise, I'm happy to add this check only in
>> Yama. The existing Yama scopes should ignore attach requests when
>> already attached.
>>
>> -Kees
>>



\
 
 \ /
  Last update: 2015-01-06 10:21    [W:0.086 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site