From 90c7a6b4c0e57b4c3036883825109ae641940be6 Mon Sep 17 00:00:00 2001 From: user01010111 Date: Tue, 1 Sep 2026 17:26:49 +1200 Subject: [PATCH 1/2] common: warn when become_user() leaves root credentials Check the real and effective UIDs after the POSIX credential transition, and warn once if either remains zero. Keep non-root starts and the intentionally privileged upsmon -p path unchanged. Fixes #3471. AI assistance: OpenAI Codex gpt-5.6-sol at high reasoning was used for investigation, implementation and validation. The human contributor remains responsible for the change. Signed-off-by: user01010111 --- common/common.c | 12 +++++++++++- tests/Makefile.am | 13 +++++++++++-- tests/become-user-root-warning-test.sh | 23 +++++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) create mode 100755 tests/become-user-root-warning-test.sh diff --git a/common/common.c b/common/common.c index 8e8538cba8..919bb9d2b2 100644 --- a/common/common.c +++ b/common/common.c @@ -983,6 +983,8 @@ void become_user(struct passwd *pw) /* if we can't switch users, then don't even try */ intmax_t initial_uid = getuid(); intmax_t initial_euid = geteuid(); + intmax_t final_uid; + intmax_t final_euid; if (!pw) { upsdebugx(1, "Can not become_user(), skipped"); @@ -1019,8 +1021,16 @@ void become_user(struct passwd *pw) if (setuid(pw->pw_uid) == -1) fatal_with_errno(EXIT_FAILURE, "setuid"); + final_uid = getuid(); + final_euid = geteuid(); + if ((final_uid == 0) || (final_euid == 0)) { + upslogx(LOG_WARNING, "Warning: running as root (UID=%jd EUID=%jd)", + final_uid, final_euid); + return; + } + upsdebugx(1, "Succeeded to become_user(%s): now UID=%jd GID=%jd", - pw->pw_name, (intmax_t)getuid(), (intmax_t)getgid()); + pw->pw_name, final_uid, (intmax_t)getgid()); #else /* WIN32 */ /* NUT_WIN32_INCOMPLETE_MAYBE_NOT_APPLICABLE(); */ upsdebugx(1, "Can not become_user(%s): not implemented on this platform", diff --git a/tests/Makefile.am b/tests/Makefile.am index eef6e490bf..069674fa1b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -15,7 +15,7 @@ SUBDIRS = . NIT # handle that properly all: $(TESTS) $(check_PROGRAMS) $(check_SCRIPTS) -EXTRA_DIST = nut-driver-enumerator-test.sh nut-driver-enumerator-test--ups.conf +EXTRA_DIST = become-user-root-warning-test.sh nut-driver-enumerator-test.sh nut-driver-enumerator-test--ups.conf EXTRA_DIST += cppunit-warnings.h cppunit-warnings-end.h TESTS = @@ -54,6 +54,9 @@ $(top_builddir)/clients/libnutclient.la \ $(top_builddir)/clients/libnutclientstub.la: dummy @dotMAKE@ +@cd $(@D) && $(MAKE) $(AM_MAKEFLAGS) $(@F) +$(top_builddir)/clients/upslog: dummy @dotMAKE@ + +@cd $(@D) && $(MAKE) $(AM_MAKEFLAGS) $(@F) + # Builds from root dir arrange stuff decently. Make sure parallel builds # started from scratch right in this dir get dependencies in proper order # (sub-makes are independent as far as trying to write into same files): @@ -310,10 +313,16 @@ memcheck: @echo " SKIP $@ : valgrind was not detected on this system by configure script" >&2 endif !HAVE_VALGRIND +CHECK_LOCAL_TARGETS = become-user-root-warning-test if WITH_VALGRIND -check-local: memcheck +CHECK_LOCAL_TARGETS += memcheck endif WITH_VALGRIND +check-local: $(CHECK_LOCAL_TARGETS) + +become-user-root-warning-test: $(top_builddir)/clients/upslog + $(AM_V_at)$(SHELL) $(srcdir)/become-user-root-warning-test.sh + dummy: BUILT_SOURCES = $(LINKED_SOURCE_FILES) diff --git a/tests/become-user-root-warning-test.sh b/tests/become-user-root-warning-test.sh new file mode 100755 index 0000000000..57016f57c6 --- /dev/null +++ b/tests/become-user-root-warning-test.sh @@ -0,0 +1,23 @@ +#!/bin/sh + +if test "`id -u`" != 0; then + echo "SKIP: root privileges are required to test become_user(root)" >&2 + exit 0 +fi + +testdir="`mktemp -d "${TMPDIR-/tmp}/nut-become-user-root-warning.XXXXXX"`" || exit +trap 'rm -rf "$testdir"' EXIT HUP INT TERM + +upslog=${UPSLOG-../clients/upslog} +warning='Warning: running as root (UID=0 EUID=0)' + +if output="`"$upslog" -F -u root -p "$testdir/upslog" -W 1 -d 1 -i 1 \ + -s dummy@127.0.0.1:1 -l - 2>&1`"; then + count="`printf '%s\n' "$output" | grep -F "$warning" | wc -l`" + if test "$count" -eq 1; then + exit 0 + fi +fi + +printf '%s\n' "$output" >&2 +exit 1 From 75d5b309ad2048f5041736cffb867eaf0050ad8a Mon Sep 17 00:00:00 2001 From: user01010111 Date: Tue, 1 Sep 2026 20:00:23 +1200 Subject: [PATCH 2/2] tests: improve root-warning test portability Honour EXEEXT, pass the exact build-tree upslog path, and use the current UID-0 account name. Treat an empty TMPDIR as unset and add the requested NEWS entry. Refs #3471. AI assistance: OpenAI Codex gpt-5.6-sol at high reasoning was used for investigation, implementation and validation. The human contributor remains responsible for the change. Signed-off-by: user01010111 --- NEWS.adoc | 2 ++ tests/Makefile.am | 8 ++++---- tests/become-user-root-warning-test.sh | 7 ++++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index 8da073ce8e..06cf73e1e6 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -293,6 +293,8 @@ https://github.com/networkupstools/nut/milestone/13 not do so for single-driver runs -- addressed with this release. [#3302] - common code: + * POSIX daemons now warn if their real or effective UID remains zero after + the common credential switch. [issue #3471, PR #3609] * Refactored `common::background()` method used by numerous NUT daemons to handle parent and child code paths by separately addressable methods. This got used in the shared drivers `main` code file to preclude losing diff --git a/tests/Makefile.am b/tests/Makefile.am index 069674fa1b..b2fe3dcff7 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -54,8 +54,8 @@ $(top_builddir)/clients/libnutclient.la \ $(top_builddir)/clients/libnutclientstub.la: dummy @dotMAKE@ +@cd $(@D) && $(MAKE) $(AM_MAKEFLAGS) $(@F) -$(top_builddir)/clients/upslog: dummy @dotMAKE@ - +@cd $(@D) && $(MAKE) $(AM_MAKEFLAGS) $(@F) +$(top_builddir)/clients/upslog$(EXEEXT): dummy @dotMAKE@ + +@cd $(@D) && $(MAKE) $(AM_MAKEFLAGS) upslog$(EXEEXT) # Builds from root dir arrange stuff decently. Make sure parallel builds # started from scratch right in this dir get dependencies in proper order @@ -320,8 +320,8 @@ endif WITH_VALGRIND check-local: $(CHECK_LOCAL_TARGETS) -become-user-root-warning-test: $(top_builddir)/clients/upslog - $(AM_V_at)$(SHELL) $(srcdir)/become-user-root-warning-test.sh +become-user-root-warning-test: $(top_builddir)/clients/upslog$(EXEEXT) + $(AM_V_at)UPSLOG='$(abs_top_builddir)/clients/upslog$(EXEEXT)' $(SHELL) $(srcdir)/become-user-root-warning-test.sh dummy: diff --git a/tests/become-user-root-warning-test.sh b/tests/become-user-root-warning-test.sh index 57016f57c6..ffcbff952d 100755 --- a/tests/become-user-root-warning-test.sh +++ b/tests/become-user-root-warning-test.sh @@ -1,17 +1,18 @@ #!/bin/sh if test "`id -u`" != 0; then - echo "SKIP: root privileges are required to test become_user(root)" >&2 + echo "SKIP: root privileges are required to test become_user()" >&2 exit 0 fi -testdir="`mktemp -d "${TMPDIR-/tmp}/nut-become-user-root-warning.XXXXXX"`" || exit +testdir="`mktemp -d "${TMPDIR:-/tmp}/nut-become-user-root-warning.XXXXXX"`" || exit trap 'rm -rf "$testdir"' EXIT HUP INT TERM upslog=${UPSLOG-../clients/upslog} +root_user="`id -un`" || exit warning='Warning: running as root (UID=0 EUID=0)' -if output="`"$upslog" -F -u root -p "$testdir/upslog" -W 1 -d 1 -i 1 \ +if output="`"$upslog" -F -u "$root_user" -p "$testdir/upslog" -W 1 -d 1 -i 1 \ -s dummy@127.0.0.1:1 -l - 2>&1`"; then count="`printf '%s\n' "$output" | grep -F "$warning" | wc -l`" if test "$count" -eq 1; then