On Wed, Oct 10, 2018 at 02:54:22PM +0200, Christian Brauner wrote:

On Tue, Oct 09, 2018 at 06:26:47PM +0200, Jann Horn wrote:

quoted

On Tue, Oct 9, 2018 at 6:20 PM Christian Brauner [off-list ref] wrote:

quoted

On Tue, Oct 09, 2018 at 05:26:26PM +0200, Jann Horn wrote:

quoted

On Tue, Oct 9, 2018 at 4:09 PM Christian Brauner [off-list ref] wrote:

quoted

On Tue, Oct 09, 2018 at 03:50:53PM +0200, Jann Horn wrote:

quoted

On Tue, Oct 9, 2018 at 3:49 PM Christian Brauner [off-list ref] wrote:

quoted

On Tue, Oct 09, 2018 at 03:36:04PM +0200, Jann Horn wrote:

quoted

On Tue, Oct 9, 2018 at 3:29 PM Christian Brauner [off-list ref] wrote:

quoted

One more thing. Citing from [1]

quoted

I think there's a security problem here. Imagine the following scenario:

1. task A (uid==0) sets up a seccomp filter that uses SECCOMP_RET_USER_NOTIF
2. task A forks off a child B
3. task B uses setuid(1) to drop its privileges
4. task B becomes dumpable again, either via prctl(PR_SET_DUMPABLE, 1)
or via execve()
5. task C (the attacker, uid==1) attaches to task B via ptrace
6. task C uses PTRACE_SECCOMP_NEW_LISTENER on task B

Sorry, to be late to the party but would this really pass
__ptrace_may_access() in ptrace_attach()? It doesn't seem obvious to me
that it would... Doesn't look like it would get past:

        tcred = __task_cred(task);
        if (uid_eq(caller_uid, tcred->euid) &&
            uid_eq(caller_uid, tcred->suid) &&
            uid_eq(caller_uid, tcred->uid)  &&
            gid_eq(caller_gid, tcred->egid) &&
            gid_eq(caller_gid, tcred->sgid) &&
            gid_eq(caller_gid, tcred->gid))
                goto ok;
        if (ptrace_has_cap(tcred->user_ns, mode))
                goto ok;
        rcu_read_unlock();
        return -EPERM;
ok:
        rcu_read_unlock();
        mm = task->mm;
        if (mm &&
            ((get_dumpable(mm) != SUID_DUMP_USER) &&
             !ptrace_has_cap(mm->user_ns, mode)))
            return -EPERM;

Which specific check would prevent task C from attaching to task B? If
the UIDs match, the first "goto ok" executes; and you're dumpable, so
you don't trigger the second "return -EPERM".

You'd also need CAP_SYS_PTRACE in the mm->user_ns which you shouldn't
have if you did a setuid to an unpriv user. (But I always find that code
confusing.)

Only if the target hasn't gone through execve() since setuid().

Sorry if I want to know this in excessive detail but I'd like to
understand this properly so bear with me :)
- If task B has setuid()ed and prctl(PR_SET_DUMPABLE, 1)ed but not
  execve()ed then C won't pass ptrace_has_cap(mm->user_ns, mode).

Yeah.

quoted

- If task B has setuid()ed, exeved()ed it will get its dumpable flag set
  to /proc/sys/fs/suid_dumpable

Not if you changed all UIDs (e.g. by calling setuid() as root). In
that case, setup_new_exec() calls "set_dumpable(current->mm,
SUID_DUMP_USER)".

Actually, looking at this when C is trying to PTRACE_ATTACH to B as an
unprivileged user even if B execve()ed and it is dumpable C still
wouldn't have CAP_SYS_PTRACE in the mm->user_ns unless it already is
privileged over mm->user_ns which means it must be in an ancestor
user_ns.

Huh? Why would you need CAP_SYS_PTRACE for anything here? You can
ptrace another process running under your UID just fine, no matter
what the namespaces are. I'm not sure what you're saying.

Sorry, I was out the door yesterday when answering this and was too
brief. I forgot to mention: /proc/sys/kernel/yama/ptrace_scope. It
should be enabled by default on nearly all distros and even if not -
which is an administrators choice - you can usually easily enable it via
sysctl.

1 ("restricted ptrace") [default value]
When  performing an operation that requires a PTRACE_MODE_ATTACH check,
the calling process must either have the CAP_SYS_PTRACE capability in
the user namespace of the target process or it must have a prede‐ fined
relationship with the target process.  By default, the predefined
relationship is that the target process must be a descendant of the
caller.

If you don't have it set you're already susceptible to all kinds of
other attacks and I'm still not convinced we need to bring out the big
capable(CAP_SYS_ADMIN) gun here.

That being said, given that Tycho agreed to leave in the native
seccomp() way of retrieving an fd from the task without the sys_admin
restriction [1] which we prefer and if we merge it with aforementioned
feature I care way less about whether or not the restriction is upheld
for ptrace() or not.

[1]: https://lists.linuxfoundation.org/pipermail/containers/2018-October/039553.html