[PATCH 1/2] capabilities: Check PR_SET_SECUREBITS with ns_capable()

["Mickaël Salaün" <mic@digikod.net>]

Per-process securebits were introduced with a direct cap_capable() check
for CAP_SETPCAP.  The capable LSM hook and SELinux capability policy
already existed, and SELinux forwarded task_prctl operations to
commoncap.  The direct call therefore bypassed SELinux policy from the
start.  It now also bypasses AppArmor, Landlock, and BPF LSM programs
attached to the capable hook.

A task with CAP_SETPCAP in its effective set can consequently change
privileged securebits despite an LSM denial.  This can alter how its
capabilities evolve across UID and execution transitions without the
expected LSM denial or audit record.

Use ns_capable(), as cap_prctl_drop() already does for PR_CAPBSET_DROP.
Call it only for a no-op, which requires privilege for legacy reasons,
or when a privileged bit changes.  Changes limited to
SECURE_ALL_UNPRIVILEGED and their lock bits do not need the capability,
so they must not invoke an LSM check or set PF_SUPERPRIV.  Successful
privileged checks now set PF_SUPERPRIV, as other ns_capable() callers
do.  Without other LSMs, the same requests remain allowed as before.

The check now reaches every capable hook before commoncap commits the
new credentials.  A later task_prctl hook cannot provide equivalent
mediation because commoncap runs first and commits the change itself.

Cc: Christian Brauner <brauner@kernel.org>
Cc: Günther Noack <gnoack@google.com>
Cc: James Morris <jmorris@namei.org>
Cc: John Johansen <john.johansen@canonical.com>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Matt Bobrowski <matt@bobrowski.net>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: stable@vger.kernel.org
Reported-by: Ada Smith <adasmithgt@gmail.com>
Fixes: 3898b1b4ebff ("capabilities: implement per-process securebits")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 security/commoncap.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index d47ab3022343..1947128396e2 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1283,6 +1283,23 @@ static int cap_prctl_drop(unsigned long cap)
 	return commit_creds(new);
 }
 
+/*
+ * Doing anything with securebits requires privilege (go read about the
+ * "sendmail capabilities bug"), except for unprivileged bits.  Indeed, the
+ * SECURE_ALL_UNPRIVILEGED bits are not restrictions enforced by the kernel but
+ * by user space on itself.  For legacy reasons, a non-change requires
+ * privilege too.  Only checking CAP_SETPCAP when it is needed keeps LSMs from
+ * seeing (and logging) a capability check for unprivileged changes.
+ */
+static bool securebits_need_setpcap(unsigned long old, unsigned long new)
+{
+	const unsigned long unpriv_and_locks = SECURE_ALL_UNPRIVILEGED |
+					       SECURE_ALL_UNPRIVILEGED << 1;
+	const unsigned long changed = old ^ new;
+
+	return !changed || (changed & ~unpriv_and_locks);
+}
+
 /**
  * cap_task_prctl - Implement process control functions for this security module
  * @option: The process control function requested
@@ -1346,28 +1363,9 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
 			/* cannot change a locked bit */
 			return -EPERM;
 
-		/*
-		 * Doing anything requires privilege (go read about the
-		 * "sendmail capabilities bug"), except for unprivileged bits.
-		 * Indeed, the SECURE_ALL_UNPRIVILEGED bits are not
-		 * restrictions enforced by the kernel but by user space on
-		 * itself.
-		 */
-		if (cap_capable(current_cred(), current_cred()->user_ns,
-				CAP_SETPCAP, CAP_OPT_NONE) != 0) {
-			const unsigned long unpriv_and_locks =
-				SECURE_ALL_UNPRIVILEGED |
-				SECURE_ALL_UNPRIVILEGED << 1;
-			const unsigned long changed = old->securebits ^ arg2;
-
-			/* For legacy reason, denies non-change. */
-			if (!changed)
-				return -EPERM;
-
-			/* Denies privileged changes. */
-			if (changed & ~unpriv_and_locks)
-				return -EPERM;
-		}
+		if (securebits_need_setpcap(old->securebits, arg2) &&
+		    !ns_capable(current_user_ns(), CAP_SETPCAP))
+			return -EPERM;
 
 		new = prepare_creds();
 		if (!new)
-- 
2.56.0