Skip to content

Commit 0884e06

Browse files
ernst-bablickernst-bablick
committed
BF: CS-2333: sge_peopen/sge_peopen_r drop UID without GID (missing setgid) — server JSV runs with gid=0 in root qmaster
1 parent c47670f commit 0884e06

3 files changed

Lines changed: 254 additions & 0 deletions

File tree

‎source/libs/uti/sge_stdio.cc‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,20 @@ pid_t sge_peopen(const char *shell, int login_shell, const char *command,
216216
sge_exit(1);
217217
}
218218

219+
/* drop the primary group before the uid - initgroups() only sets the
220+
* supplementary groups, so without this setgid() the child would keep
221+
* the parent's (root) gid (CS-2333). Must precede setuid() while
222+
* we are still privileged. */
223+
if (setgid(pw->pw_gid)) {
224+
snprintf(err_str, sizeof(err_str), MSG_SYSTEM_SETGIDFAILED_g, pw->pw_gid);
225+
snprintf(err_str, sizeof(err_str), "\n");
226+
if (write(2, err_str, strlen(err_str)) != (ssize_t)strlen(err_str)) {
227+
/* nothing we can do here - we are anyway about to exit */
228+
}
229+
sge_free(&buffer);
230+
sge_exit(1);
231+
}
232+
219233
if (setuid(pw->pw_uid)) {
220234
snprintf(err_str, sizeof(err_str), MSG_SYSTEM_SWITCHTOUSERFAILED_SS, user, strerror(errno));
221235
snprintf(err_str, sizeof(err_str), "\n");
@@ -527,6 +541,13 @@ pid_t sge_peopen_r(const char *shell, int login_shell, const char *command,
527541
}
528542

529543
if (pw != nullptr) {
544+
/* drop the primary group before the uid - initgroups() (called in the
545+
* parent) only sets the supplementary groups, so without this setgid()
546+
* the child would keep the parent's (root) gid (CS-2333). Must
547+
* precede setuid() while we are still privileged. */
548+
if (setgid(pw->pw_gid)) {
549+
sge_exit(1);
550+
}
530551
int lret = setuid(tuid);
531552
if (lret) {
532553
sge_exit(1);

‎test/libs/uti/CMakeLists.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ target_include_directories(test_uti_os_fds PRIVATE "./")
7272
target_link_libraries(test_uti_os_fds PRIVATE uti commlists ${SGE_LIBS})
7373
add_test(NAME test_uti_os_fds COMMAND test_uti_os_fds)
7474

75+
add_executable(test_uti_peopen test_uti_peopen.cc)
76+
target_include_directories(test_uti_peopen PRIVATE "./")
77+
target_link_libraries(test_uti_peopen PRIVATE uti commlists ${SGE_LIBS})
78+
add_test(NAME test_uti_peopen COMMAND test_uti_peopen)
79+
7580
add_executable(test_uti_profiling test_uti_profiling.cc)
7681
target_include_directories(test_uti_profiling PRIVATE "./")
7782
target_link_libraries(test_uti_profiling PRIVATE uti commlists ${SGE_LIBS})
@@ -132,6 +137,7 @@ if (INSTALL_SGE_TEST)
132137
install(TARGETS test_uti_munge DESTINATION testbin/${SGE_ARCH})
133138
endif()
134139
install(TARGETS test_uti_os_fds DESTINATION testbin/${SGE_ARCH})
140+
install(TARGETS test_uti_peopen DESTINATION testbin/${SGE_ARCH})
135141
install(TARGETS test_uti_profiling DESTINATION testbin/${SGE_ARCH})
136142

137143
install(TARGETS test_uti_sl DESTINATION testbin/${SGE_ARCH})

‎test/libs/uti/test_uti_peopen.cc‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
/*___INFO__MARK_BEGIN_NEW__*/
2+
/***************************************************************************
3+
*
4+
* Copyright 2023-2026 HPC-Gridware GmbH
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*
18+
***************************************************************************/
19+
/*___INFO__MARK_END_NEW__*/
20+
21+
/*
22+
* Regression test for CS-2333:
23+
* sge_peopen()/sge_peopen_r() drop the UID of a target user but never call
24+
* setgid(), so the spawned command keeps the parent's primary GID (gid 0
25+
* when the parent is root). This is reachable in qmaster via the server JSV
26+
* fork (sge_jsv.cc passes a non-NULL user to sge_peopen_r()).
27+
*
28+
* The privileged user-switch path only runs when the test process is root.
29+
* When not root, or when no suitable non-root target user exists, the test
30+
* reports a skip and passes (so it is harmless under CTest in an unprivileged
31+
* build). Run as root (e.g. from the installed testsuite) it spawns "id" as
32+
* the target user and verifies both the uid and the *primary* gid: before the
33+
* fix the gid check fails with gid 0, after the fix it passes.
34+
*/
35+
36+
#include <cstdio>
37+
#include <cstdlib>
38+
#include <cstring>
39+
#include <pwd.h>
40+
#include <unistd.h>
41+
#include <sys/types.h>
42+
43+
#include "uti/sge_component.h"
44+
#include "uti/sge_stdio.h"
45+
#include "uti/sge_rmon_macros.h"
46+
47+
#include <sge_log.h>
48+
49+
static int s_fail = 0;
50+
51+
#define CHECK(id, label, expr) \
52+
do { \
53+
if (!(expr)) { \
54+
printf("FAIL [T%02d] %s\n", (id), (label)); \
55+
++s_fail; \
56+
} else { \
57+
printf("ok [T%02d] %s\n", (id), (label)); \
58+
} \
59+
} while (0)
60+
61+
/** @brief Select a usable non-root target user to switch to.
62+
*
63+
* An explicit user (argv[1] or TEST_PEOPEN_USER) takes precedence; otherwise
64+
* the passwd database is scanned for the first entry with a non-root uid and
65+
* a non-root primary gid.
66+
*
67+
* @param explicit_user requested user name, or nullptr/empty to auto-scan
68+
* @param name_buf buffer receiving the resolved user name
69+
* @param name_len size of name_buf in bytes
70+
* @param uid out: resolved user's uid
71+
* @param gid out: resolved user's primary gid
72+
* @return true if a suitable user was found, false otherwise
73+
*/
74+
static bool
75+
find_target_user(const char *explicit_user, char *name_buf, size_t name_len,
76+
uid_t *uid, gid_t *gid) {
77+
if (explicit_user != nullptr && explicit_user[0] != '\0') {
78+
struct passwd *pw = getpwnam(explicit_user);
79+
if (pw == nullptr) {
80+
printf("requested user '%s' not found\n", explicit_user);
81+
return false;
82+
}
83+
if (pw->pw_uid == 0 || pw->pw_gid == 0) {
84+
printf("requested user '%s' must have non-root uid and primary gid\n", explicit_user);
85+
return false;
86+
}
87+
snprintf(name_buf, name_len, "%s", pw->pw_name);
88+
*uid = pw->pw_uid;
89+
*gid = pw->pw_gid;
90+
return true;
91+
}
92+
93+
setpwent();
94+
struct passwd *pw;
95+
bool found = false;
96+
while ((pw = getpwent()) != nullptr) {
97+
if (pw->pw_uid != 0 && pw->pw_gid != 0) {
98+
snprintf(name_buf, name_len, "%s", pw->pw_name);
99+
*uid = pw->pw_uid;
100+
*gid = pw->pw_gid;
101+
found = true;
102+
break;
103+
}
104+
}
105+
endpwent();
106+
return found;
107+
}
108+
109+
/** @brief Run "id <id_arg>" as @p user through the given peopen variant.
110+
*
111+
* @param use_r true to use sge_peopen_r(), false to use sge_peopen()
112+
* @param user target user the command should run as
113+
* @param id_arg argument passed to id(1), e.g. "-u" or "-g"
114+
* @param result out: numeric value printed by the command
115+
* @return true on successful spawn and read, false otherwise
116+
*/
117+
static bool
118+
run_id(bool use_r, const char *user, const char *id_arg, long *result) {
119+
char command[32];
120+
snprintf(command, sizeof(command), "id %s", id_arg);
121+
122+
FILE *fp_in = nullptr;
123+
FILE *fp_out = nullptr;
124+
FILE *fp_err = nullptr;
125+
126+
pid_t pid;
127+
if (use_r) {
128+
pid = sge_peopen_r("/bin/sh", 0, command, user, nullptr,
129+
&fp_in, &fp_out, &fp_err, false);
130+
} else {
131+
pid = sge_peopen("/bin/sh", 0, command, user, nullptr,
132+
&fp_in, &fp_out, &fp_err, false);
133+
}
134+
if (pid <= 0) {
135+
return false;
136+
}
137+
138+
bool ok = false;
139+
char line[64];
140+
if (fp_out != nullptr && fgets(line, sizeof(line), fp_out) != nullptr) {
141+
*result = strtol(line, nullptr, 10);
142+
ok = true;
143+
}
144+
sge_peclose(pid, fp_in, fp_out, fp_err, nullptr);
145+
return ok;
146+
}
147+
148+
/** @brief Verify one peopen variant drops to the target user's uid and gid.
149+
*
150+
* Spawns id(1) as @p user and checks the child runs with both the target uid
151+
* and the target primary gid. The gid check is the CS-2333 regression:
152+
* without setgid() the child keeps the parent's gid (0 when run as root).
153+
*
154+
* @param id in/out: running check counter, incremented per assertion
155+
* @param use_r true to exercise sge_peopen_r(), false for sge_peopen()
156+
* @param user target user to switch to
157+
* @param want_uid expected uid of the spawned child
158+
* @param want_gid expected primary gid of the spawned child
159+
*/
160+
static void
161+
check_variant(int *id, bool use_r, const char *user, uid_t want_uid, gid_t want_gid) {
162+
const char *tag = use_r ? "PEOPEN_R" : "PEOPEN";
163+
char label[128];
164+
165+
long got_uid = -1;
166+
bool ran_uid = run_id(use_r, user, "-u", &got_uid);
167+
snprintf(label, sizeof(label), "%s: spawn 'id -u' -> ok", tag);
168+
CHECK(*id, label, ran_uid); (*id)++;
169+
snprintf(label, sizeof(label), "%s: child uid -> got %ld, expected %ld",
170+
tag, got_uid, (long) want_uid);
171+
CHECK(*id, label, ran_uid && got_uid == (long) want_uid); (*id)++;
172+
173+
long got_gid = -1;
174+
bool ran_gid = run_id(use_r, user, "-g", &got_gid);
175+
snprintf(label, sizeof(label), "%s: spawn 'id -g' -> ok", tag);
176+
CHECK(*id, label, ran_gid); (*id)++;
177+
snprintf(label, sizeof(label),
178+
"%s: child primary gid -> got %ld, expected %ld (gid 0 == CS-2333)",
179+
tag, got_gid, (long) want_gid);
180+
CHECK(*id, label, ran_gid && got_gid == (long) want_gid); (*id)++;
181+
}
182+
183+
/** @brief Module test entry point for CS-2333 (sge_peopen* GID drop).
184+
*
185+
* Requires root to exercise the user-switch path. Resolves the target user
186+
* from argv[1], TEST_PEOPEN_USER, or an auto-scan, then checks both peopen
187+
* variants. Skips (and passes) when not root or no suitable user exists.
188+
*
189+
* @param argc argument count; argv[1] optionally names the target user
190+
* @param argv argument vector
191+
* @return 0 on success or skip, 1 if any check failed
192+
*/
193+
int main(int argc, char *argv[]) {
194+
DENTER_MAIN(TOP_LAYER, "test_uti_peopen");
195+
int id = 1;
196+
197+
// suppress ERROR-level sge_log output from the spawn helpers
198+
component_set_daemonized(true);
199+
200+
if (geteuid() != 0) {
201+
printf("SKIP: must run as root to exercise the user-switch path (CS-2333)\n");
202+
DRETURN(0);
203+
}
204+
205+
// target user to switch to: argv[1], else TEST_PEOPEN_USER, else auto-scan
206+
const char *requested_user = (argc > 1) ? argv[1] : getenv("TEST_PEOPEN_USER");
207+
208+
char user[256];
209+
uid_t want_uid = 0;
210+
gid_t want_gid = 0;
211+
if (!find_target_user(requested_user, user, sizeof(user), &want_uid, &want_gid)) {
212+
printf("SKIP: no non-root user with non-root primary group found "
213+
"(set TEST_PEOPEN_USER)\n");
214+
DRETURN(0);
215+
}
216+
217+
printf("target user=%s uid=%ld gid=%ld\n", user, (long) want_uid, (long) want_gid);
218+
219+
printf("\n--- PEOPEN_R: qmaster server-JSV fork path ---\n");
220+
check_variant(&id, true, user, want_uid, want_gid);
221+
222+
printf("\n--- PEOPEN: generic user-switch path ---\n");
223+
check_variant(&id, false, user, want_uid, want_gid);
224+
225+
printf("\n%s - %d failure(s)\n", s_fail == 0 ? "PASS" : "FAIL", s_fail);
226+
DRETURN(s_fail == 0 ? 0 : 1);
227+
}

0 commit comments

Comments
 (0)