From a73064dda97d751047748c4539a969495bdadf73 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 16 Feb 2021 19:34:42 +0100 Subject: [PATCH 1/3] Remove setuid support This support was rarely ever used and has been disabled by default for some time. As far as the developer team is aware there's no distribution that activated this feature in their packages by default. --- .github/workflows/ci.yml | 14 +++++++------- CRT.c | 31 ------------------------------- CRT.h | 11 ----------- README | 3 --- configure.ac | 11 ----------- 5 files changed, 7 insertions(+), 63 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e468efa1..2c80da90 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,11 +63,11 @@ jobs: - name: Bootstrap run: ./autogen.sh - name: Configure - run: ./configure --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-setuid --enable-delayacct --enable-sensors --enable-capabilities + run: ./configure --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-delayacct --enable-sensors --enable-capabilities - name: Build run: make -k - name: Distcheck - run: make distcheck DISTCHECK_CONFIGURE_FLAGS='--enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-setuid --enable-delayacct --enable-sensors --enable-capabilities' + run: make distcheck DISTCHECK_CONFIGURE_FLAGS='--enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-delayacct --enable-sensors --enable-capabilities' build-ubuntu-latest-full-featured-clang: runs-on: ubuntu-latest @@ -85,11 +85,11 @@ jobs: - name: Bootstrap run: ./autogen.sh - name: Configure - run: ./configure --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-setuid --enable-delayacct --enable-sensors --enable-capabilities + run: ./configure --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-delayacct --enable-sensors --enable-capabilities - name: Build run: make -k - name: Distcheck - run: make distcheck DISTCHECK_CONFIGURE_FLAGS='--enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-setuid --enable-delayacct --enable-sensors --enable-capabilities' + run: make distcheck DISTCHECK_CONFIGURE_FLAGS='--enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-delayacct --enable-sensors --enable-capabilities' build-ubuntu-latest-gcc-static: runs-on: ubuntu-latest @@ -104,11 +104,11 @@ jobs: - name: Bootstrap run: ./autogen.sh - name: Configure - run: ./configure --enable-static --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --disable-hwloc --enable-setuid --disable-delayacct --enable-sensors --enable-capabilities + run: ./configure --enable-static --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --disable-hwloc --disable-delayacct --enable-sensors --enable-capabilities - name: Build run: make -k - name: Distcheck - run: make distcheck DISTCHECK_CONFIGURE_FLAGS='--enable-static --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --disable-hwloc --enable-setuid --disable-delayacct --enable-sensors --enable-capabilities' + run: make distcheck DISTCHECK_CONFIGURE_FLAGS='--enable-static --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --disable-hwloc --disable-delayacct --enable-sensors --enable-capabilities' build-ubuntu-latest-clang-analyzer: runs-on: ubuntu-latest @@ -126,7 +126,7 @@ jobs: - name: Bootstrap run: ./autogen.sh - name: Configure - run: scan-build-11 -analyze-headers --status-bugs ./configure --enable-debug --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-setuid --enable-delayacct --enable-sensors --enable-capabilities + run: scan-build-11 -analyze-headers --status-bugs ./configure --enable-debug --enable-werror --enable-openvz --enable-vserver --enable-ancient-vserver --enable-unicode --enable-hwloc --enable-delayacct --enable-sensors --enable-capabilities - name: Build run: scan-build-11 -analyze-headers --status-bugs make -j"$(nproc)" diff --git a/CRT.c b/CRT.c index 7e651c3b..aa115abe 100644 --- a/CRT.c +++ b/CRT.c @@ -658,37 +658,6 @@ static void CRT_handleSIGTERM(int sgn) { _exit(0); } -#ifdef HAVE_SETUID_ENABLED - -static int CRT_euid = -1; - -static int CRT_egid = -1; - -void CRT_dropPrivileges() { - CRT_egid = getegid(); - CRT_euid = geteuid(); - if (setegid(getgid()) == -1) { - CRT_fatalError("Fatal error: failed dropping group privileges"); - } - if (seteuid(getuid()) == -1) { - CRT_fatalError("Fatal error: failed dropping user privileges"); - } -} - -void CRT_restorePrivileges() { - if (CRT_egid == -1 || CRT_euid == -1) { - CRT_fatalError("Fatal error: internal inconsistency"); - } - if (setegid(CRT_egid) == -1) { - CRT_fatalError("Fatal error: failed restoring group privileges"); - } - if (seteuid(CRT_euid) == -1) { - CRT_fatalError("Fatal error: failed restoring user privileges"); - } -} - -#endif /* HAVE_SETUID_ENABLED */ - #ifndef NDEBUG static int stderrRedirectNewFd = -1; diff --git a/CRT.h b/CRT.h index e77ec3dc..ddd00169 100644 --- a/CRT.h +++ b/CRT.h @@ -160,20 +160,9 @@ extern int CRT_scrollWheelVAmount; extern ColorScheme CRT_colorScheme; -#ifdef HAVE_SETUID_ENABLED - -void CRT_dropPrivileges(void); - -void CRT_restorePrivileges(void); - -#else /* HAVE_SETUID_ENABLED */ - -/* Turn setuid operations into NOPs */ static inline void CRT_dropPrivileges(void) { } static inline void CRT_restorePrivileges(void) { } -#endif /* HAVE_SETUID_ENABLED */ - void CRT_init(const Settings* settings, bool allowUnicode); void CRT_done(void); diff --git a/README b/README index 09ec9347..a9cb1dfb 100644 --- a/README +++ b/README @@ -58,9 +58,6 @@ By default `make install` will install into `/usr/local`, for changing the path enable hwloc support for CPU affinity; disables Linux affinity dependency: *libhwloc* default: *no* - * `--enable-setuid`: - enable setuid support for privilege dropping - default: *no* * `--enable-static`: build a static htop binary; hwloc and delay accounting are not supported default: *no* diff --git a/configure.ac b/configure.ac index b2e3fefb..ee7e8ee5 100644 --- a/configure.ac +++ b/configure.ac @@ -316,16 +316,6 @@ case "$enable_hwloc" in ;; esac - -AC_ARG_ENABLE([setuid], - [AS_HELP_STRING([--enable-setuid], - [enable setuid support for privilege dropping @<:@default=no@:>@])], - [], - [enable_setuid=no]) -if test "x$enable_setuid" = xyes; then - AC_DEFINE([HAVE_SETUID_ENABLED], [1], [Define if setuid support should be enabled.]) -fi - # ---------------------------------------------------------------------- @@ -628,7 +618,6 @@ AC_MSG_RESULT([ (Linux) capabilities: $enable_capabilities unicode: $enable_unicode hwloc: $enable_hwloc - setuid: $enable_setuid debug: $enable_debug static: $enable_static ]) From 82157f598e09790b408a4e519a25d3affba95240 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 16 Feb 2021 19:44:59 +0100 Subject: [PATCH 2/3] Refactor to remove no-op calls This removes the call-sites of the removed setuid feature --- CRT.h | 3 --- EnvScreen.c | 2 -- Process.c | 8 ++------ Settings.c | 18 ++++-------------- TraceScreen.c | 2 -- 5 files changed, 6 insertions(+), 27 deletions(-) diff --git a/CRT.h b/CRT.h index ddd00169..91c2b89d 100644 --- a/CRT.h +++ b/CRT.h @@ -160,9 +160,6 @@ extern int CRT_scrollWheelVAmount; extern ColorScheme CRT_colorScheme; -static inline void CRT_dropPrivileges(void) { } -static inline void CRT_restorePrivileges(void) { } - void CRT_init(const Settings* settings, bool allowUnicode); void CRT_done(void); diff --git a/EnvScreen.c b/EnvScreen.c index 4d7de7d9..bd8a2b03 100644 --- a/EnvScreen.c +++ b/EnvScreen.c @@ -34,9 +34,7 @@ static void EnvScreen_scan(InfoScreen* this) { Panel_prune(panel); - CRT_dropPrivileges(); char* env = Platform_getProcessEnv(this->process->pid); - CRT_restorePrivileges(); if (env) { for (const char* p = env; *p; p = strrchr(p, 0) + 1) InfoScreen_addLine(this, p); diff --git a/Process.c b/Process.c index 9ee7a0f8..3fbcb634 100644 --- a/Process.c +++ b/Process.c @@ -479,10 +479,9 @@ bool Process_isTomb(const Process* this) { } bool Process_setPriority(Process* this, int priority) { - CRT_dropPrivileges(); int old_prio = getpriority(PRIO_PROCESS, this->pid); int err = setpriority(PRIO_PROCESS, this->pid, priority); - CRT_restorePrivileges(); + if (err == 0 && old_prio != getpriority(PRIO_PROCESS, this->pid)) { this->nice = priority; } @@ -494,10 +493,7 @@ bool Process_changePriorityBy(Process* this, Arg delta) { } bool Process_sendSignal(Process* this, Arg sgn) { - CRT_dropPrivileges(); - bool ok = (kill(this->pid, sgn.i) == 0); - CRT_restorePrivileges(); - return ok; + return kill(this->pid, sgn.i) == 0; } int Process_pidCompare(const void* v1, const void* v2) { diff --git a/Settings.c b/Settings.c index a6ca7935..3c93bcd2 100644 --- a/Settings.c +++ b/Settings.c @@ -125,10 +125,7 @@ static void readFields(ProcessField* fields, uint32_t* flags, const char* line) } static bool Settings_read(Settings* this, const char* fileName, int initialCpuCount) { - FILE* fd; - CRT_dropPrivileges(); - fd = fopen(fileName, "r"); - CRT_restorePrivileges(); + FILE* fd = fopen(fileName, "r"); if (!fd) return false; @@ -284,15 +281,10 @@ static void writeMeterModes(Settings* this, FILE* fd, int column) { } bool Settings_write(Settings* this) { - FILE* fd; - - CRT_dropPrivileges(); - fd = fopen(this->filename, "w"); - CRT_restorePrivileges(); - - if (fd == NULL) { + FILE* fd = fopen(this->filename, "w"); + if (fd == NULL) return false; - } + fprintf(fd, "# Beware! This file is rewritten by htop when settings are changed in the interface.\n"); fprintf(fd, "# The parser is also very primitive, and not human-friendly.\n"); writeFields(fd, this->fields, "fields"); @@ -410,7 +402,6 @@ Settings* Settings_new(int initialCpuCount) { htopDir = String_cat(home, "/.config/htop"); } legacyDotfile = String_cat(home, "/.htoprc"); - CRT_dropPrivileges(); (void) mkdir(configDir, 0700); (void) mkdir(htopDir, 0700); free(htopDir); @@ -421,7 +412,6 @@ Settings* Settings_new(int initialCpuCount) { free(legacyDotfile); legacyDotfile = NULL; } - CRT_restorePrivileges(); } this->colorScheme = 0; this->enableMouse = true; diff --git a/TraceScreen.c b/TraceScreen.c index 3e3971d7..c21e4802 100644 --- a/TraceScreen.c +++ b/TraceScreen.c @@ -87,8 +87,6 @@ bool TraceScreen_forkTracer(TraceScreen* this) { dup2(fdpair[1], STDERR_FILENO); close(fdpair[1]); - CRT_dropPrivileges(); - char buffer[32] = {0}; xSnprintf(buffer, sizeof(buffer), "%d", this->super.process->pid); execlp("strace", "strace", "-T", "-tt", "-s", "512", "-p", buffer, NULL); From 067cd6deb87a7a30e85d06cef78c9b5be2bcf973 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Wed, 17 Feb 2021 17:14:06 +0100 Subject: [PATCH 3/3] Include note in changelog regarding removal of the setuid feature --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index 4efaaecf..9ee33723 100644 --- a/ChangeLog +++ b/ChangeLog @@ -8,6 +8,8 @@ What's new in version 3.0.6 (not released yet) in your htoprc file. Solution: Press I (to invert sort order). This changed setting will be saved by htop on exit as long as it can write to your htoprc file. +* The compile-time option to cater specifically for running htop as + setuid has been removed. What's new in version 3.0.5