From ec6f844243d22d3bcb71b9f91e0929d4ae26842d Mon Sep 17 00:00:00 2001 From: Anthony Ryan Date: Sun, 31 May 2026 20:25:50 -0400 Subject: [PATCH 1/2] Add missing header to lib/find_new_sub_*ids.c Reviewed-by: Alejandro Colomar --- lib/find_new_sub_gids.c | 1 + lib/find_new_sub_uids.c | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/find_new_sub_gids.c b/lib/find_new_sub_gids.c index 3971ce922e..34e7d2eac9 100644 --- a/lib/find_new_sub_gids.c +++ b/lib/find_new_sub_gids.c @@ -10,6 +10,7 @@ #include #include +#include #include "prototypes.h" #include "subordinateio.h" diff --git a/lib/find_new_sub_uids.c b/lib/find_new_sub_uids.c index 65f6815467..639b4a4dfa 100644 --- a/lib/find_new_sub_uids.c +++ b/lib/find_new_sub_uids.c @@ -10,6 +10,7 @@ #include #include +#include #include "prototypes.h" #include "subordinateio.h" From 1b4335fdd1d42222606515575b76ce7311fbab61 Mon Sep 17 00:00:00 2001 From: Anthony Ryan Date: Thu, 11 Jun 2026 19:10:21 -0400 Subject: [PATCH 2/2] Expand the use of --with-fcaps to other binaries This is defense in depth attack surface reduction. During recent kernel bugs, PoCs overwrote the kernel page cache to inject attacker payloads into setuid binaries for execution. The kernel forms the trusted computing base for shadow, so it's not technically shadow's responsibility to mitigate. But it's relatively simple change that gives us protection from both future or undiscovered bugs in shadow binaries, and limits the number of useful setuid targets to attackers weaponizing future kernel bugs. ----- Downstream distro packagers expressed an interest in the ability to toggle between setuid and filecaps without changing configure arguments. For that reason, we look at the euid to determine which exceution mode we should be running in (and skip some calls like setuid to reduce required capabilities). The --with-fcaps argument now changes what our Makefile does, but runtime behaviour doesn't depend on the configuration argument. ----- In places where we previously called setuid(0) unconditionally, we now check the euid to decide if we're running in a setuid or filecaps context. When (euid == 0) we call setuid(0) to lock the real and saved-set-user-ID to avoid changing any behaviours in the existing code in setuid mode. When (euid != 0) we skip setuid(0) and assume that the necessary file capabilities have been put in place for the binaries to run without it. ----- One of the code comments removed in this change indicates that one of the reasons for setuid was to protect against keyboard signals attempting to interrupt changes in mid-execution. For this reason I looked inot the signal handling and found that we are already protecting againt corruption caused by interrupting utilities in mid-execution. The heavy lifting protecting us from this is happening in commonio_close() with it's atomic rename. Looking thcough the code I found that the worst case for an uncatchable signal appears to be a stael lockfile being left behind, which appears harmless and will be cleaned up on subsequent use. ----- While looking at signal handing, I did add handling for SIGXFSZ, which was not a concern previously, but will be more relevant in filecaps mode because without CAP_SYS_RESOURCE or setuid(0) our setrlimit() calls in pwd_init() no longer change anything. ----- Next I tried to determine if the absence of CAP_SYS_RESOURCE could cause other problems, and if I was making a mistake by omitting that capability. RLIMIT_CORE is the main security focused one, and it's permitted without setuid, since we're reducing limits rather than increasing. The other setrlimits no longer raise limits to RLIM_INFINITY in filcaps mode, but I feel like it's a reasonable trade-off in 2026 to assume that distros that are deliberately opting into improved security can at least do the courtesy of providing sane rlimits. The other rlimits worth checking were do_user_limits() but they all appear to be focused on lowering limits relative to system defaults, so I don't believe any of those are impacted by the loss of the ability to raise resource limits. ----- In terms of the capabilities required: cap_chown & cap_fowner are required by fmkstemp_set_perms() which is common to many tools. And then all the other setcaps are based on the actions the individual tools need. If a tool only needs read-only access cap_dac_read_search is adequate to allow it to read. But writing privileged files requires cap_dac_override cap_sys_resource was omitted because the only behavioral change (of requiring sane ulimit/rlimit) felt reasonable to cut another privilege for this security focused opt-in. --- configure.ac | 2 +- lib/pwd_init.c | 1 + src/Makefile.am | 23 +++++++++++++++++++---- src/chfn.c | 8 +------- src/chsh.c | 8 +------- src/gpasswd.c | 2 +- src/passwd.c | 2 +- 7 files changed, 25 insertions(+), 21 deletions(-) diff --git a/configure.ac b/configure.ac index 77ceb52f71..d6137f82eb 100644 --- a/configure.ac +++ b/configure.ac @@ -550,7 +550,7 @@ fi AM_CONDITIONAL([USE_PAM], [test "X$with_libpam" = "Xyes"]) AC_ARG_WITH([fcaps], - [AS_HELP_STRING([--with-fcaps], [use file capabilities instead of suid binaries for newuidmap/newgidmap @<:@default=no@:>@])], + [AS_HELP_STRING([--with-fcaps], [use file capabilities instead of suid binaries where possible @<:@default=no@:>@])], [with_fcaps=$withval], [with_fcaps=no]) AM_CONDITIONAL([FCAPS], [test "x$with_fcaps" = "xyes"]) diff --git a/lib/pwd_init.c b/lib/pwd_init.c index bd2cfd3ff5..e253da4cec 100644 --- a/lib/pwd_init.c +++ b/lib/pwd_init.c @@ -51,6 +51,7 @@ void pwd_init (void) signal (SIGTERM, SIG_IGN); signal (SIGTSTP, SIG_IGN); signal (SIGTTOU, SIG_IGN); + signal (SIGXFSZ, SIG_IGN); umask (077); } diff --git a/src/Makefile.am b/src/Makefile.am index c935e05b30..423a6576f4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -61,17 +61,22 @@ noinst_PROGRAMS = sulogin suidusbins = suidbins = -suidubins = chage chfn chsh gpasswd newgrp if WITH_SU suidbins += su endif + +privubins = chage chfn chsh gpasswd newgrp if !WITH_TCB -suidubins += passwd +privubins += passwd endif if ENABLE_SUBIDS -if !FCAPS -suidubins += newgidmap newuidmap +privubins += newgidmap newuidmap endif + +if FCAPS +suidubins = +else +suidubins = $(privubins) endif if WITH_TCB @@ -143,6 +148,16 @@ if WITH_TCB chmod $(sgidperms) $(DESTDIR)$(ubindir)/$$i; \ done endif +if FCAPS + setcap cap_dac_read_search+ep $(DESTDIR)$(ubindir)/chage + setcap cap_setgid,cap_dac_read_search+ep $(DESTDIR)$(ubindir)/newgrp + setcap cap_chown,cap_dac_override,cap_fowner+ep $(DESTDIR)$(ubindir)/chfn + setcap cap_chown,cap_dac_override,cap_fowner+ep $(DESTDIR)$(ubindir)/chsh + setcap cap_chown,cap_dac_override,cap_fowner+ep $(DESTDIR)$(ubindir)/gpasswd +if !WITH_TCB + setcap cap_chown,cap_dac_override,cap_fowner+ep $(DESTDIR)$(ubindir)/passwd +endif +endif if ENABLE_SUBIDS if FCAPS setcap cap_setuid+ep $(DESTDIR)$(ubindir)/newuidmap diff --git a/src/chfn.c b/src/chfn.c index 71875253e9..763ec14805 100644 --- a/src/chfn.c +++ b/src/chfn.c @@ -395,13 +395,7 @@ static void update_gecos(const char *user, char *gecos, const struct option_flag process_selinux = !flags->chroot; - /* - * Before going any further, raise the ulimit to prevent colliding - * into a lowered ulimit, and set the real UID to root to protect - * against unexpected signals. Any keyboard signals are set to be - * ignored. - */ - if (setuid (0) != 0) { + if (geteuid () == 0 && setuid (0) != 0) { fputs (_("Cannot change ID to root.\n"), stderr); SYSLOG(LOG_ERR, "can't setuid(0)"); fail_exit (E_NOPERM, process_selinux); diff --git a/src/chsh.c b/src/chsh.c index fafa9759d6..3d057053e1 100644 --- a/src/chsh.c +++ b/src/chsh.c @@ -375,13 +375,7 @@ static void update_shell (const char *user, char *newshell, const struct option_ process_selinux = !flags->chroot; - /* - * Before going any further, raise the ulimit to prevent - * colliding into a lowered ulimit, and set the real UID - * to root to protect against unexpected signals. Any - * keyboard signals are set to be ignored. - */ - if (setuid (0) != 0) { + if (geteuid () == 0 && setuid (0) != 0) { SYSLOG(LOG_ERR, "can't setuid(0)"); fputs (_("Cannot change ID to root.\n"), stderr); fail_exit (1, process_selinux); diff --git a/src/gpasswd.c b/src/gpasswd.c index bb53087c6f..06351a17b7 100644 --- a/src/gpasswd.c +++ b/src/gpasswd.c @@ -1088,7 +1088,7 @@ int main (int argc, char **argv) * output, etc. */ output: - if (setuid (0) != 0) { + if (geteuid () == 0 && setuid (0) != 0) { fputs (_("Cannot change ID to root.\n"), stderr); SYSLOG(LOG_ERR, "can't setuid(0)"); closelog (); diff --git a/src/passwd.c b/src/passwd.c index b2cac80b00..00a77067e9 100644 --- a/src/passwd.c +++ b/src/passwd.c @@ -1197,7 +1197,7 @@ main(int argc, char **argv) exit (E_SUCCESS); } #endif /* USE_PAM */ - if (setuid (0) != 0) { + if (geteuid () == 0 && setuid (0) != 0) { (void) fputs (_("Cannot change ID to root.\n"), stderr); SYSLOG(LOG_ERR, "can't setuid(0)"); closelog ();