From 7c8ab7915f984ddce576847d4b49557c8039e34a Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Sat, 27 Dec 2025 00:49:48 +0100 Subject: [PATCH] Fix #458: follow-up to be2a0ec, capabilities breaks root access A user reports inability to re-start getty on the console after logging out from a serial console. After some digging it was found that the tty was owned by the last user logged in and 600. Even thougn getty runs as root, it did not have permission to re-open the device node. Turns out there was a minor bug in the new capability code that cleared all capabilities from the root user. A surprising amount of programs worked just fine, but restarting getty gave it away. The fix is to only call cap_setuid() when capabilities are set for the service, otherwise we just fall back to setuid(). Also, refactor service_register() wrt. capabilities a bit so that we can give users an early warning if the configuration is invalid, by adding a parse_caps() helper function that calls cap_iab_from_text() to verify. Signed-off-by: Joachim Wiberg --- src/service.c | 48 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/src/service.c b/src/service.c index fc262aaa..490797bf 100644 --- a/src/service.c +++ b/src/service.c @@ -496,14 +496,21 @@ static void compose_cmdline(svc_t *svc, char *buf, size_t len) static void set_uid(uid_t uid, svc_t *svc) { #ifdef HAVE_LIBCAP - if (cap_setuid(uid)) { - err(1, "%s: failed cap_setuid(%d)", svc_ident(svc, NULL, 0), uid); - return; - } - - /* After dropping privileges, set the specific capabilities we need */ + /* + * Only use cap_setuid() if capabilities are configured, + * otherwise it would (by design) drop all capabilities, + * breaking root services + */ if (svc->capabilities[0]) { - cap_iab_t cap_iab = cap_iab_from_text(svc->capabilities); + cap_iab_t cap_iab; + + if (cap_setuid(uid)) { + err(1, "%s: failed cap_setuid(%d)", svc_ident(svc, NULL, 0), uid); + return; + } + + /* After dropping privileges, set the specific capabilities we need */ + cap_iab = cap_iab_from_text(svc->capabilities); if (!cap_iab) { err(1, "%s: failed parsing capabilities '%s'", svc_ident(svc, NULL, 0), svc->capabilities); @@ -516,11 +523,10 @@ static void set_uid(uid_t uid, svc_t *svc) svc_ident(svc, NULL, 0)); } cap_free(cap_iab); - } -#else + } else +#endif if (setuid(uid)) err(1, "%s: failed setuid(%d)", svc_ident(svc, NULL, 0), uid); -#endif } static pid_t service_fork(svc_t *svc) @@ -1421,6 +1427,26 @@ static void parse_env(svc_t *svc, char *env) strlcpy(svc->env, env, sizeof(svc->env)); } +static void parse_caps(svc_t *svc, char *caps) +{ +#ifdef HAVE_LIBCAP + cap_iab_t cap_iab; + + cap_iab = cap_iab_from_text(caps) + if (!cap_iab) { + err(1, "%s: failed parsing capabilities '%s'", svc_ident(svc, NULL, 0), caps); + memset(svc->capabilities, 0, sizeof(svc->capabilities)); + return; + } + + cap_free(cap_iab); + strlcpy(svc->capabilities, caps, sizeof(svc->capabilities)); +#else + (void)svc; + (void)caps; +#endif +} + /* * the @cgroup argument can be, e.g., .system,mem.max:1234 or just the * default group with some cfg, e.g., :mem.max:1234 as a side effect, @@ -2111,7 +2137,7 @@ int service_register(int type, char *cfg, struct rlimit rlimit[], char *file) else memset(svc->env, 0, sizeof(svc->env)); if (caps) - strlcpy(svc->capabilities, caps, sizeof(svc->capabilities)); + parse_caps(svc, caps); else memset(svc->capabilities, 0, sizeof(svc->capabilities)); if (file)