Skip dropping the bounding set without SECURE_NOROOT.
If we're asked to skip setting *and* locking the SECURE_NOROOT
securebit, also skip dropping the bounding set. If the caller wants to
regain all capabilities when executing a set-user-ID-root program,
allow them to do so. The default behavior (i.e. the behavior without
|securebits_skip_mask| set) will still put the jailed process tree in a
capabilities-only environment.
This will allow giving powerd on Chrome OS some capabilities without
breaking other things.
Bug: 78629772
Test: New unit tests.
Test: Ad-hoc with fork+exec program + setuid program + -B 0x3
Test: Setuid program is able to keep all caps.
Change-Id: I36f79a42666720a65d88ec48454b56695f25b64b
diff --git a/libminijail.c b/libminijail.c
index cf6f261..d641b42 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -1736,12 +1736,28 @@
die("can't apply initial cleaned capset");
/*
- * Instead of dropping bounding set first, do it here in case
+ * Instead of dropping the bounding set first, do it here in case
* the caller had a more permissive bounding set which could
* have been used above to raise a capability that wasn't already
* present. This requires CAP_SETPCAP, so we raised/kept it above.
+ *
+ * However, if we're asked to skip setting *and* locking the
+ * SECURE_NOROOT securebit, also skip dropping the bounding set.
+ * If the caller wants to regain all capabilities when executing a
+ * set-user-ID-root program, allow them to do so. The default behavior
+ * (i.e. the behavior without |securebits_skip_mask| set) will still put
+ * the jailed process tree in a capabilities-only environment.
+ *
+ * We check the negated skip mask for SECURE_NOROOT and
+ * SECURE_NOROOT_LOCKED. If the bits are set in the negated mask they
+ * will *not* be skipped in lock_securebits(), and therefore we should
+ * drop the bounding set.
*/
- drop_capbset(j->caps, last_valid_cap);
+ if (secure_noroot_set_and_locked(~j->securebits_skip_mask)) {
+ drop_capbset(j->caps, last_valid_cap);
+ } else {
+ warn("SECURE_NOROOT not set, not dropping bounding set");
+ }
/* If CAP_SETPCAP wasn't specifically requested, now we remove it. */
if ((j->caps & (one << CAP_SETPCAP)) == 0) {
diff --git a/minijail0_cli.c b/minijail0_cli.c
index e366533..4aba5dc 100644
--- a/minijail0_cli.c
+++ b/minijail0_cli.c
@@ -371,6 +371,7 @@
" -B <mask>: Skip setting securebits in <mask> when restricting capabilities (-c).\n"
" By default, SECURE_NOROOT, SECURE_NO_SETUID_FIXUP, and \n"
" SECURE_KEEP_CAPS (together with their respective locks) are set.\n"
+ " There are eight securebits in total.\n"
" -k <...>: Mount <src> at <dest> in chroot.\n"
" <flags> and <data> can be specified as in mount(2).\n"
" Multiple instances allowed.\n"
diff --git a/system.c b/system.c
index ad69717..5e2a6f6 100644
--- a/system.c
+++ b/system.c
@@ -43,6 +43,12 @@
_Static_assert(SECURE_ALL_BITS == 0x55, "SECURE_ALL_BITS == 0x55.");
#endif
+int secure_noroot_set_and_locked(uint64_t mask)
+{
+ return (mask & (SECBIT_NOROOT | SECBIT_NOROOT_LOCKED)) ==
+ (SECBIT_NOROOT | SECBIT_NOROOT_LOCKED);
+}
+
int lock_securebits(uint64_t skip_mask)
{
/*
@@ -54,6 +60,7 @@
unsigned long securebits =
(SECURE_BITS_NO_AMBIENT | SECURE_LOCKS_NO_AMBIENT) & ~skip_mask;
if (!securebits) {
+ warn("not locking any securebits");
return 0;
}
int securebits_ret = prctl(PR_SET_SECUREBITS, securebits);
diff --git a/system.h b/system.h
index b816f5f..a748cc4 100644
--- a/system.h
+++ b/system.h
@@ -38,6 +38,7 @@
#define PR_CAP_AMBIENT_CLEAR_ALL 4
#endif
+int secure_noroot_set_and_locked(uint64_t mask);
int lock_securebits(uint64_t skip_mask);
unsigned int get_last_valid_cap(void);
diff --git a/system_unittest.cc b/system_unittest.cc
index d2484b5..8d2a873 100644
--- a/system_unittest.cc
+++ b/system_unittest.cc
@@ -6,6 +6,7 @@
*/
#include <limits.h>
+#include <linux/securebits.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -63,6 +64,22 @@
} // namespace
+TEST(secure_noroot_set_and_locked, zero_mask) {
+ ASSERT_EQ(secure_noroot_set_and_locked(0), 0);
+}
+
+TEST(secure_noroot_set_and_locked, set) {
+ ASSERT_EQ(secure_noroot_set_and_locked(issecure_mask(SECURE_NOROOT) |
+ issecure_mask(SECURE_NOROOT_LOCKED)),
+ 1);
+}
+
+TEST(secure_noroot_set_and_locked, not_set) {
+ ASSERT_EQ(secure_noroot_set_and_locked(issecure_mask(SECURE_KEEP_CAPS) |
+ issecure_mask(SECURE_NOROOT_LOCKED)),
+ 0);
+}
+
// Sanity check for the cap range.
TEST(get_last_valid_cap, basic) {
unsigned int cap = get_last_valid_cap();
