Re: [PATCH v9 2/3] capabilities: don't let ptrace borrow CAP_SETFCAP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] capabilities: bypass of cap_ptrace_borrows_setfcap() via target
  task
--

commit 13b53bed949001ddba3ad97bb6d9dfe1459dfe6f
Author: Josef Bacik <josef@toxicpanda.com>

capabilities: don't let ptrace borrow CAP_SETFCAP

This commit denies cross-namespace PTRACE_MODE_ATTACH and PTRACE_TRACEME
when a tracer lacking CAP_SETFCAP and CAP_SYS_PTRACE attempts to trace a
target task that holds CAP_SETFCAP or can regain it from its bounding set
in a user namespace where setns() would refuse the tracer.

Link: https://lore.kernel.org/all/fd159e68-c613-4c7d-bfba-8af0a3f3a96e@hallyn.com/

> diff --git a/security/commoncap.c b/security/commoncap.c
> index 0ece141c68a69..d550a4e5d36a3 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -176,14 +176,35 @@ int cap_settime(const struct timespec64 *ts, const struct timezone *tz)
>  	return 0;
>  }
>  
> +/*
> + * See cred_borrows_setfcap().  Within one namespace the usual rules apply: a
> + * tracer with CAP_SYS_PTRACE can take control of tasks with CAP_SETFCAP
> + * anyway.  A target that still has CAP_SETFCAP in its bounding set gets it
> + * back from an execve() the tracer can make it do.
> + */
> +static bool cap_ptrace_borrows_setfcap(const struct cred *cred,
> +				       const struct cred *child_cred)
> +{
> +	if (cred->user_ns == child_cred->user_ns)
> +		return false;
> +	if (!cap_raised(child_cred->cap_permitted, CAP_SETFCAP) &&
> +	    !cap_raised(child_cred->cap_bset, CAP_SETFCAP))
> +		return false;
> +	return !cap_raised(cred->cap_permitted, CAP_SYS_PTRACE) &&
> +	       cred_borrows_setfcap(cred, child_cred->user_ns);
> +}

[Severity: High]
Can a tracer still make the target task obtain or exercise CAP_SETFCAP
over the tracer's root user when CAP_SETFCAP is cleared in both
child_cred->cap_permitted and child_cred->cap_bset?

If CAP_SETFCAP remains raised in child_cred->cap_inheritable, making a
root target task in child_cred->user_ns call execve() restores CAP_SETFCAP
in new->cap_permitted via handle_privileged_root(), which sets
new->cap_permitted = cap_combine(old->cap_bset, old->cap_inheritable).

Even if child_cred->cap_inheritable is empty, the tracer can make the
target task:
- call unshare(CLONE_NEWUSER), where set_cred_user_ns() resets
  cap_permitted, cap_effective and cap_bset to CAP_FULL_SET (and map a
  non-zero UID mapped to the tracer's root kuid to 0 in the new
  namespace),
- write "0 0 1" to /proc/self/uid_map using CAP_SYS_ADMIN if
  child_cred->user_ns is still unmapped and parent_could_setfcap is true,
  or
- ptrace another task in child_cred->user_ns that still holds CAP_SETFCAP,
  since cap_ptrace_borrows_setfcap() allows same-namespace ptrace.

In each case, the tracer can still write file capabilities valid for its
root user.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-b4-setfcap-userns-v9-0-0d3f00976ce2@toxicpanda.com?part=2