Merge lp:~broder/upstart/drop-privileges into lp:upstart

Proposed by Evan Broder on 2011-11-06
Status: Merged
Merged at revision: 1331
Proposed branch: lp:~broder/upstart/drop-privileges
Merge into: lp:upstart
Diff against target: 748 lines (+536/-8)
12 files modified
contrib/vim/syntax/upstart.vim (+1/-1)
init/errors.h (+4/-0)
init/job_class.c (+3/-0)
init/job_class.h (+4/-0)
init/job_process.c (+82/-0)
init/job_process.h (+4/-0)
init/man/init.5 (+26/-0)
init/parse_job.c (+84/-0)
init/tests/test_job_class.c (+4/-0)
init/tests/test_job_process.c (+49/-7)
init/tests/test_parse_job.c (+234/-0)
util/tests/test_user_sessions.sh (+41/-0)
To merge this branch: bzr merge lp:~broder/upstart/drop-privileges
Reviewer Review Type Date Requested Status
James Hunt 2011-11-06 Needs Fixing on 2011-11-11
Review via email: mp+81417@code.launchpad.net
To post a comment you must log in.
James Hunt (jamesodhunt) wrote :

* init/man/init.5:

- Typo 'unspceified'.
- To avoid any confusion, I think it's worth explaining that system jobs which specify "setuid X"
cannot be controlled by user X since they are not user jobs: they are simply jobs running as the
user in question [*].

* init/job_process.c:job_process_spawn():

- That fchown() looks potentially unsafe:

  if job_setgid != -1, but job_setuid == -1, we get:

    fchown (script_fd, -1, job_setgid);

However, something like the following is also too simplistic...

    fchown (script_fd,
      job_setuid == -1 ? 0 : job_setuid,
      job_setgid == -1 ? 0 : job_setgid);

... since it doesn't take account of user jobs (and in my snippet above could allow privilege
escalation).

- Resource Limits/OOM/Priority

The fact that the setgid+setuid calls come *after* the chroot+user session code means we're
effectively allowing non-privileged (system) jobs to elevate their resource limits, OOM score and
priority. It could be argued that these are after all system jobs, so why not allow such behaviour?
But until compelling examples are given, I'd prefer we take the cautious approach and disallowed
this behaviour. Again, to avoid confusion we should document in init.5 that system jobs running as
non-privileged users cannot elevate their resource limits beyond that users limits.

* Testing

Yes, I appreciate the issues testing some of these scenarios. It will admittedly complicated by
adding setuid+setgid support to the already interesting combination of user jobs and chroot support
 What we need is a fully automated set of tests for these features. Effort is being put into this
for the current Ubuntu cycle.

What I would recommend is adding some tests to util/tests/test_user_sessions.sh that...

  - essentially duplicate the tests you've created in test_parse_job.c to ensure that say specifying
multiple values after the setuid stanza fails as expected.

  - ensure 'setuid root' fails as expected
  - ensure 'setdid root' fails as expected
  - ensure 'setgid <group>' works (assuming <group> is the users primary group).
  - ensure 'setgid <group>' works (assuming <group> is a valid supplementary group).

Also, could you add a test to init/tests/test_job_process.c that ensures it is possible to run a job
which specifies the *current* user and group of the user running the tests (essentially specifying
"setuid `id -u`" and "setgid `id -g`").

review: Needs Fixing
lp:~broder/upstart/drop-privileges updated on 2011-12-07
1336. By Evan Broder on 2011-11-11

* init/man/init.5: Correct spelling typo and clarify that system jobs
which drop privilege are still system jobs, not user jobs.

1337. By Evan Broder on 2011-11-11

* contrib/vim/syntax/upstart.vim: Add the new setuid and setgid
  stanzas

1338. By Evan Broder on 2011-11-18

* init/tests/test_job_process.c, util/tests/test_user_sessions.sh:
  Test setuid and setgid stanzas as thoroughly as an unprivileged user
  can (i.e. make sure dropping to yourself works, and escalating to
  anything else doesn't)

1339. By Evan Broder on 2011-12-07

* init/job_process.c: Cast signed constants to uid_t and gid_t before
  comparing them with the same to correct warnings from the
  build. uid_t and gid_t are unsigned, but -1 is still their dummy
  value

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'contrib/vim/syntax/upstart.vim'
2--- contrib/vim/syntax/upstart.vim 2011-06-14 13:53:27 +0000
3+++ contrib/vim/syntax/upstart.vim 2011-12-07 15:52:24 +0000
4@@ -33,7 +33,7 @@
5 " one argument
6 syn keyword upstartStatement description author version instance expect
7 syn keyword upstartStatement pid kill normal console env exit export
8-syn keyword upstartStatement umask nice oom chroot chdir exec
9+syn keyword upstartStatement umask nice oom chroot chdir exec setiud setgid
10
11 " two arguments
12 syn keyword upstartStatement limit
13
14=== modified file 'init/errors.h'
15--- init/errors.h 2011-05-12 20:42:28 +0000
16+++ init/errors.h 2011-12-07 15:52:24 +0000
17@@ -36,6 +36,8 @@
18
19 /* Errors while handling job processes */
20 JOB_PROCESS_ERROR,
21+ JOB_PROCESS_INVALID_SETUID,
22+ JOB_PROCESS_INVALID_SETGID,
23
24 /* Errors while parsing configuration files */
25 PARSE_ILLEGAL_INTERVAL,
26@@ -59,6 +61,8 @@
27 #define ENVIRON_UNKNOWN_PARAM_STR N_("Unknown parameter")
28 #define ENVIRON_EXPECTED_OPERATOR_STR N_("Expected operator")
29 #define ENVIRON_MISMATCHED_BRACES_STR N_("Mismatched braces")
30+#define JOB_PROCESS_INVALID_SETUID_STR N_("Invalid setuid user name does not exist")
31+#define JOB_PROCESS_INVALID_SETGID_STR N_("Invalid setgid group name does not exist")
32 #define PARSE_ILLEGAL_INTERVAL_STR N_("Illegal interval, expected number of seconds")
33 #define PARSE_ILLEGAL_EXIT_STR N_("Illegal exit status, expected integer")
34 #define PARSE_ILLEGAL_SIGNAL_STR N_("Illegal signal status, expected integer")
35
36=== modified file 'init/job_class.c'
37--- init/job_class.c 2011-08-11 20:47:00 +0000
38+++ init/job_class.c 2011-12-07 15:52:24 +0000
39@@ -216,6 +216,9 @@
40 class->chroot = NULL;
41 class->chdir = NULL;
42
43+ class->setuid = NULL;
44+ class->setgid = NULL;
45+
46 class->deleted = FALSE;
47 class->debug = FALSE;
48
49
50=== modified file 'init/job_class.h'
51--- init/job_class.h 2011-08-11 20:47:00 +0000
52+++ init/job_class.h 2011-12-07 15:52:24 +0000
53@@ -156,6 +156,8 @@
54 * @limits: resource limits indexed by resource,
55 * @chroot: root directory of process (implies @chdir if not set),
56 * @chdir: working directory of process,
57+ * @setuid: user name to drop to before starting process,
58+ * @setgid: group name to drop to before starting process,
59 * @deleted: whether job should be deleted when finished.
60 *
61 * This structure holds the configuration of a known task or service that
62@@ -206,6 +208,8 @@
63 struct rlimit *limits[RLIMIT_NLIMITS];
64 char *chroot;
65 char *chdir;
66+ char *setuid;
67+ char *setgid;
68
69 int deleted;
70 int debug;
71
72=== modified file 'init/job_process.c'
73--- init/job_process.c 2011-08-11 21:08:52 +0000
74+++ init/job_process.c 2011-12-07 15:52:24 +0000
75@@ -41,6 +41,7 @@
76 #include <utmp.h>
77 #include <utmpx.h>
78 #include <pwd.h>
79+#include <grp.h>
80
81 #include <nih/macros.h>
82 #include <nih/alloc.h>
83@@ -380,6 +381,8 @@
84 FILE *fd;
85 int user_job = FALSE;
86 nih_local char *user_dir = NULL;
87+ uid_t job_setuid = -1;
88+ gid_t job_setgid = -1;
89
90
91 nih_assert (class != NULL);
92@@ -654,6 +657,67 @@
93 job_process_error_abort (fds[1], JOB_PROCESS_ERROR_CHDIR, 0);
94 }
95
96+ /* Change the user and group of the process to the one
97+ * configured in the job. We must wait until now to lookup the
98+ * UID and GID from the names to accommodate both chroot
99+ * session jobs and jobs with a chroot stanza.
100+ */
101+ if (class->setuid) {
102+ /* Without resetting errno, it's impossible to
103+ * distinguish between a non-existent user and and
104+ * error during lookup */
105+ errno = 0;
106+ struct passwd *pwd = getpwnam (class->setuid);
107+ if (! pwd) {
108+ if (errno != 0) {
109+ nih_error_raise_system ();
110+ job_process_error_abort (fds[1], JOB_PROCESS_ERROR_GETPWNAM, 0);
111+ } else {
112+ nih_error_raise (JOB_PROCESS_INVALID_SETUID,
113+ JOB_PROCESS_INVALID_SETUID_STR);
114+ job_process_error_abort (fds[1], JOB_PROCESS_ERROR_BAD_SETUID, 0);
115+ }
116+ }
117+
118+ job_setuid = pwd->pw_uid;
119+ /* This will be overridden if setgid is also set: */
120+ job_setgid = pwd->pw_gid;
121+ }
122+
123+ if (class->setgid) {
124+ errno = 0;
125+ struct group *grp = getgrnam (class->setgid);
126+ if (! grp) {
127+ if (errno != 0) {
128+ nih_error_raise_system ();
129+ job_process_error_abort (fds[1], JOB_PROCESS_ERROR_GETGRNAM, 0);
130+ } else {
131+ nih_error_raise (JOB_PROCESS_INVALID_SETGID,
132+ JOB_PROCESS_INVALID_SETGID_STR);
133+ job_process_error_abort (fds[1], JOB_PROCESS_ERROR_BAD_SETGID, 0);
134+ }
135+ }
136+
137+ job_setgid = grp->gr_gid;
138+ }
139+
140+ if (script_fd != -1 &&
141+ (job_setuid != (uid_t) -1 || job_setgid != (gid_t) -1) &&
142+ fchown (script_fd, job_setuid, job_setgid) < 0) {
143+ nih_error_raise_system ();
144+ job_process_error_abort (fds[1], JOB_PROCESS_ERROR_CHOWN, 0);
145+ }
146+
147+ if (job_setgid != (gid_t) -1 && setgid (job_setgid) < 0) {
148+ nih_error_raise_system ();
149+ job_process_error_abort (fds[1], JOB_PROCESS_ERROR_SETGID, 0);
150+ }
151+
152+ if (job_setuid != (uid_t)-1 && setuid (job_setuid) < 0) {
153+ nih_error_raise_system ();
154+ job_process_error_abort (fds[1], JOB_PROCESS_ERROR_SETUID, 0);
155+ }
156+
157 /* Reset all the signal handlers back to their default handling so
158 * the child isn't unexpectedly ignoring any, and so we won't
159 * surprisingly handle them before we've exec()d the new process.
160@@ -880,6 +944,24 @@
161 err, _("unable to execute: %s"),
162 strerror (err->errnum)));
163 break;
164+ case JOB_PROCESS_ERROR_GETPWNAM:
165+ err->error.message = NIH_MUST (nih_sprintf (
166+ err, _("unable to getpwnam: %s"),
167+ strerror (err->errnum)));
168+ break;
169+ case JOB_PROCESS_ERROR_GETGRNAM:
170+ err->error.message = NIH_MUST (nih_sprintf (
171+ err, _("unable to getgrnam: %s"),
172+ strerror (err->errnum)));
173+ break;
174+ case JOB_PROCESS_ERROR_BAD_SETUID:
175+ err->error.message = NIH_MUST (nih_sprintf (
176+ err, _("unable to find setuid user")));
177+ break;
178+ case JOB_PROCESS_ERROR_BAD_SETGID:
179+ err->error.message = NIH_MUST (nih_sprintf (
180+ err, _("unable to find setgid group")));
181+ break;
182 case JOB_PROCESS_ERROR_SETUID:
183 err->error.message = NIH_MUST (nih_sprintf (
184 err, _("unable to setuid: %s"),
185
186=== modified file 'init/job_process.h'
187--- init/job_process.h 2011-07-25 14:08:47 +0000
188+++ init/job_process.h 2011-12-07 15:52:24 +0000
189@@ -57,6 +57,10 @@
190 JOB_PROCESS_ERROR_CHDIR,
191 JOB_PROCESS_ERROR_PTRACE,
192 JOB_PROCESS_ERROR_EXEC,
193+ JOB_PROCESS_ERROR_GETPWNAM,
194+ JOB_PROCESS_ERROR_GETGRNAM,
195+ JOB_PROCESS_ERROR_BAD_SETUID,
196+ JOB_PROCESS_ERROR_BAD_SETGID,
197 JOB_PROCESS_ERROR_SETUID,
198 JOB_PROCESS_ERROR_SETGID,
199 JOB_PROCESS_ERROR_CHOWN
200
201=== modified file 'init/man/init.5'
202--- init/man/init.5 2011-07-25 15:01:24 +0000
203+++ init/man/init.5 2011-12-07 15:52:24 +0000
204@@ -659,6 +659,32 @@
205 .B unlimited
206 may be specified for either.
207 .\"
208+.TP
209+.B setuid \fIUSERNAME
210+Changes to the user
211+.I USERNAME
212+before running the job's process.
213+
214+If this stanza is unspecified, the job will run as root in the case of
215+system jobs, and as the user in the case of User Jobs.
216+
217+Note that System jobs using the setuid stanza are still system jobs,
218+and can not be controlled by an unprivileged user, even if the setuid
219+stanza specifies that user.
220+.\"
221+.TP
222+.B setgid \fIGROUPNAME
223+Changes to the group
224+.I GROUPNAME
225+before running the job's process.
226+
227+If this stanza is unspecified, the primary group of the user specified
228+in the
229+.B setuid
230+block is used. If both stanzas are unspecified, the job will run with
231+its group ID set to 0 in the case of system jobs, and as the primary
232+group of the user in the case of User Jobs.
233+.\"
234 .SS Override File Handling
235 Override files allow a jobs environment to be changed without modifying
236 the jobs configuration file. Rules governing override files:
237
238=== modified file 'init/parse_job.c'
239--- init/parse_job.c 2011-06-06 12:52:08 +0000
240+++ init/parse_job.c 2011-12-07 15:52:24 +0000
241@@ -209,6 +209,14 @@
242 const char *file, size_t len,
243 size_t *pos, size_t *lineno)
244 __attribute__ ((warn_unused_result));
245+static int stanza_setuid (JobClass *class, NihConfigStanza *stanza,
246+ const char *file, size_t len,
247+ size_t *pos, size_t *lineno)
248+ __attribute__ ((warn_unused_result));
249+static int stanza_setgid (JobClass *class, NihConfigStanza *stanza,
250+ const char *file, size_t len,
251+ size_t *pos, size_t *lineno)
252+ __attribute__ ((warn_unused_result));
253 static int stanza_debug (JobClass *class, NihConfigStanza *stanza,
254 const char *file, size_t len,
255 size_t *pos, size_t *lineno)
256@@ -253,6 +261,8 @@
257 { "limit", (NihConfigHandler)stanza_limit },
258 { "chroot", (NihConfigHandler)stanza_chroot },
259 { "chdir", (NihConfigHandler)stanza_chdir },
260+ { "setuid", (NihConfigHandler)stanza_setuid },
261+ { "setgid", (NihConfigHandler)stanza_setgid },
262 { "debug", (NihConfigHandler)stanza_debug },
263 { "manual", (NihConfigHandler)stanza_manual },
264
265@@ -2538,3 +2548,77 @@
266
267 return nih_config_skip_comment (file, len, pos, lineno);
268 }
269+
270+/**
271+ * stanza_setuid:
272+ * @class: job class being parsed,
273+ * @stanza: stanza found,
274+ * @file: file or string to parse,
275+ * @len: length of @file,
276+ * @pos: offset within @file,
277+ * @lineno: line number.
278+ *
279+ * Parse a setuid stanza from @file, extracting a single argument
280+ * containing a user name.
281+ *
282+ * Returns: zero on success, negative value on error.
283+ **/
284+static int
285+stanza_setuid (JobClass *class,
286+ NihConfigStanza *stanza,
287+ const char *file,
288+ size_t len,
289+ size_t *pos,
290+ size_t *lineno)
291+{
292+ nih_assert (class != NULL);
293+ nih_assert (stanza != NULL);
294+ nih_assert (file != NULL);
295+ nih_assert (pos != NULL);
296+
297+ if (class->setuid)
298+ nih_unref (class->setuid, class);
299+
300+ class->setuid = nih_config_next_arg (class, file, len, pos, lineno);
301+ if (! class->setuid)
302+ return -1;
303+
304+ return nih_config_skip_comment (file, len, pos, lineno);
305+}
306+
307+/**
308+ * stanza_setgid:
309+ * @class: job class being parsed,
310+ * @stanza: stanza found,
311+ * @file: file or string to parse,
312+ * @len: length of @file,
313+ * @pos: offset within @file,
314+ * @lineno: line number.
315+ *
316+ * Parse a setgid stanza from @file, extracting a single argument
317+ * containing a group name.
318+ *
319+ * Returns: zero on success, negative value on error.
320+ **/
321+static int
322+stanza_setgid (JobClass *class,
323+ NihConfigStanza *stanza,
324+ const char *file,
325+ size_t len,
326+ size_t *pos,
327+ size_t *lineno)
328+{
329+ nih_assert (class != NULL);
330+ nih_assert (stanza != NULL);
331+ nih_assert (file != NULL);
332+ nih_assert (pos != NULL);
333+
334+ if (class->setgid)
335+ nih_unref (class->setgid, class);
336+
337+ class->setgid = nih_config_next_arg (class, file, len, pos, lineno);
338+ if (! class->setgid)
339+ return -1;
340+
341+ return nih_config_skip_comment (file, len, pos, lineno);
342+}
343
344=== modified file 'init/tests/test_job_class.c'
345--- init/tests/test_job_class.c 2011-06-06 17:05:11 +0000
346+++ init/tests/test_job_class.c 2011-12-07 15:52:24 +0000
347@@ -139,6 +139,10 @@
348
349 TEST_EQ_P (class->chroot, NULL);
350 TEST_EQ_P (class->chdir, NULL);
351+
352+ TEST_EQ_P (class->setuid, NULL);
353+ TEST_EQ_P (class->setgid, NULL);
354+
355 TEST_FALSE (class->deleted);
356
357 nih_free (class);
358
359=== modified file 'init/tests/test_job_process.c'
360--- init/tests/test_job_process.c 2011-06-06 17:05:11 +0000
361+++ init/tests/test_job_process.c 2011-12-07 15:52:24 +0000
362@@ -38,6 +38,8 @@
363 #include <unistd.h>
364 #include <utmp.h>
365 #include <utmpx.h>
366+#include <pwd.h>
367+#include <grp.h>
368
369 #include <nih/macros.h>
370 #include <nih/string.h>
371@@ -124,13 +126,15 @@
372 void
373 test_run (void)
374 {
375- JobClass *class = NULL;
376- Job *job = NULL;
377- FILE *output;
378- struct stat statbuf;
379- char filename[PATH_MAX], buf[80];
380- int ret = -1, status, first;
381- siginfo_t info;
382+ JobClass *class = NULL;
383+ Job *job = NULL;
384+ FILE *output;
385+ struct stat statbuf;
386+ char filename[PATH_MAX], buf[80];
387+ int ret = -1, status, first;
388+ siginfo_t info;
389+ struct passwd *pwd;
390+ struct group *grp;
391
392 TEST_FUNCTION ("job_process_run");
393 job_class_init ();
394@@ -787,6 +791,44 @@
395
396 nih_free (class);
397 }
398+
399+ /* Check that we can succesfully setuid and setgid to
400+ * ourselves. This should always work, privileged or
401+ * otherwise.
402+ */
403+ TEST_FEATURE ("with setuid me");
404+ TEST_ALLOC_FAIL {
405+ TEST_ALLOC_SAFE {
406+ class = job_class_new (NULL, "test", NULL);
407+ class->process[PROCESS_MAIN] = process_new (class);
408+ class->process[PROCESS_MAIN]->command = nih_sprintf (
409+ class->process[PROCESS_MAIN],
410+ "touch %s", filename);
411+
412+ pwd = getpwuid (getuid ());
413+ TEST_NE (pwd, NULL);
414+ class->setuid = nih_strdup (class, pwd->pw_name);
415+
416+ grp = getgrgid (getgid ());
417+ TEST_NE (grp, NULL);
418+ class->setuid = nih_strdup (class, grp->gr_name);
419+
420+ job = job_new (class, "");
421+ job->goal = JOB_START;
422+ job->state = JOB_SPAWNED;
423+ }
424+
425+ ret = job_process_run (job, PROCESS_MAIN);
426+ TEST_EQ (ret, 0);
427+
428+ TEST_NE (job->pid[PROCESS_MAIN], 0);
429+
430+ waitpid (job->pid[PROCESS_MAIN], NULL, 0);
431+ TEST_EQ (stat (filename, &statbuf), 0);
432+
433+ unlink (filename);
434+ nih_free (class);
435+ }
436 }
437
438
439
440=== modified file 'init/tests/test_parse_job.c'
441--- init/tests/test_parse_job.c 2011-06-06 12:52:08 +0000
442+++ init/tests/test_parse_job.c 2011-12-07 15:52:24 +0000
443@@ -7933,6 +7933,238 @@
444 nih_free (err);
445 }
446
447+void
448+test_stanza_setuid (void)
449+{
450+ JobClass*job;
451+ NihError *err;
452+ size_t pos, lineno;
453+ char buf[1024];
454+
455+ TEST_FUNCTION ("stanza_setuid");
456+
457+ /* Check that a setuid stanza with an argument results in it
458+ * being stored in the job.
459+ */
460+ TEST_FEATURE ("with single argument");
461+ strcpy (buf, "setuid www-data\n");
462+
463+ TEST_ALLOC_FAIL {
464+ pos = 0;
465+ lineno = 1;
466+ job = parse_job (NULL, NULL, NULL, "test", buf, strlen (buf),
467+ &pos, &lineno);
468+
469+ if (test_alloc_failed) {
470+ TEST_EQ_P (job, NULL);
471+
472+ err = nih_error_get ();
473+ TEST_EQ (err->number, ENOMEM);
474+ nih_free (err);
475+
476+ continue;
477+ }
478+
479+ TEST_EQ (pos, strlen (buf));
480+ TEST_EQ (lineno, 2);
481+
482+ TEST_ALLOC_SIZE (job, sizeof (JobClass));
483+
484+ TEST_ALLOC_PARENT (job->setuid, job);
485+ TEST_EQ_STR (job->setuid, "www-data");
486+
487+ nih_free (job);
488+ }
489+
490+
491+ /* Check that the last of multiple setuid stanzas is used.
492+ */
493+ TEST_FEATURE ("with multiple stanzas");
494+ strcpy (buf, "setuid www-data\n");
495+ strcat (buf, "setuid pulse\n");
496+
497+ TEST_ALLOC_FAIL {
498+ pos = 0;
499+ lineno = 1;
500+ job = parse_job (NULL, NULL, NULL, "test", buf, strlen (buf),
501+ &pos, &lineno);
502+
503+ if (test_alloc_failed) {
504+ TEST_EQ_P (job, NULL);
505+
506+ err = nih_error_get ();
507+ TEST_EQ (err->number, ENOMEM);
508+ nih_free (err);
509+
510+ continue;
511+ }
512+
513+ TEST_EQ (pos, strlen (buf));
514+ TEST_EQ (lineno, 3);
515+
516+ TEST_ALLOC_SIZE (job, sizeof (JobClass));
517+
518+ TEST_ALLOC_PARENT (job->setuid, job);
519+ TEST_EQ_STR (job->setuid, "pulse");
520+
521+ nih_free (job);
522+ }
523+
524+
525+ /* Check that a setuid stanza without an argument results in
526+ * a syntax error.
527+ */
528+ TEST_FEATURE ("with missing argument");
529+ strcpy (buf, "setuid\n");
530+
531+ pos = 0;
532+ lineno = 1;
533+ job = parse_job (NULL, NULL, NULL, "test", buf, strlen (buf), &pos, &lineno);
534+
535+ TEST_EQ_P (job, NULL);
536+
537+ err = nih_error_get ();
538+ TEST_EQ (err->number, NIH_CONFIG_EXPECTED_TOKEN);
539+ TEST_EQ (pos, 6);
540+ TEST_EQ (lineno, 1);
541+ nih_free (err);
542+
543+
544+ /* Check that a setuid stanza with an extra second argument
545+ * results in a syntax error.
546+ */
547+ TEST_FEATURE ("with extra argument");
548+ strcpy (buf, "setuid www-data foo\n");
549+
550+ pos = 0;
551+ lineno = 1;
552+ job = parse_job (NULL, NULL, NULL, "test", buf, strlen (buf), &pos, &lineno);
553+
554+ TEST_EQ_P (job, NULL);
555+
556+ err = nih_error_get ();
557+ TEST_EQ (err->number, NIH_CONFIG_UNEXPECTED_TOKEN);
558+ TEST_EQ (pos, 16);
559+ TEST_EQ (lineno, 1);
560+ nih_free (err);
561+}
562+
563+void
564+test_stanza_setgid (void)
565+{
566+ JobClass*job;
567+ NihError *err;
568+ size_t pos, lineno;
569+ char buf[1024];
570+
571+ TEST_FUNCTION ("stanza_setgid");
572+
573+ /* Check that a setgid stanza with an argument results in it
574+ * being stored in the job.
575+ */
576+ TEST_FEATURE ("with single argument");
577+ strcpy (buf, "setgid kvm\n");
578+
579+ TEST_ALLOC_FAIL {
580+ pos = 0;
581+ lineno = 1;
582+ job = parse_job (NULL, NULL, NULL, "test", buf, strlen (buf),
583+ &pos, &lineno);
584+
585+ if (test_alloc_failed) {
586+ TEST_EQ_P (job, NULL);
587+
588+ err = nih_error_get ();
589+ TEST_EQ (err->number, ENOMEM);
590+ nih_free (err);
591+
592+ continue;
593+ }
594+
595+ TEST_EQ (pos, strlen (buf));
596+ TEST_EQ (lineno, 2);
597+
598+ TEST_ALLOC_SIZE (job, sizeof (JobClass));
599+
600+ TEST_ALLOC_PARENT (job->setgid, job);
601+ TEST_EQ_STR (job->setgid, "kvm");
602+
603+ nih_free (job);
604+ }
605+
606+
607+ /* Check that the last of multiple setgid stanzas is used.
608+ */
609+ TEST_FEATURE ("with multiple stanzas");
610+ strcpy (buf, "setgid kvm\n");
611+ strcat (buf, "setgid fuse\n");
612+
613+ TEST_ALLOC_FAIL {
614+ pos = 0;
615+ lineno = 1;
616+ job = parse_job (NULL, NULL, NULL, "test", buf, strlen (buf),
617+ &pos, &lineno);
618+
619+ if (test_alloc_failed) {
620+ TEST_EQ_P (job, NULL);
621+
622+ err = nih_error_get ();
623+ TEST_EQ (err->number, ENOMEM);
624+ nih_free (err);
625+
626+ continue;
627+ }
628+
629+ TEST_EQ (pos, strlen (buf));
630+ TEST_EQ (lineno, 3);
631+
632+ TEST_ALLOC_SIZE (job, sizeof (JobClass));
633+
634+ TEST_ALLOC_PARENT (job->setgid, job);
635+ TEST_EQ_STR (job->setgid, "fuse");
636+
637+ nih_free (job);
638+ }
639+
640+
641+ /* Check that a setgid stanza without an argument results in
642+ * a syntax error.
643+ */
644+ TEST_FEATURE ("with missing argument");
645+ strcpy (buf, "setgid\n");
646+
647+ pos = 0;
648+ lineno = 1;
649+ job = parse_job (NULL, NULL, NULL, "test", buf, strlen (buf), &pos, &lineno);
650+
651+ TEST_EQ_P (job, NULL);
652+
653+ err = nih_error_get ();
654+ TEST_EQ (err->number, NIH_CONFIG_EXPECTED_TOKEN);
655+ TEST_EQ (pos, 6);
656+ TEST_EQ (lineno, 1);
657+ nih_free (err);
658+
659+
660+ /* Check that a setgid stanza with an extra second argument
661+ * results in a syntax error.
662+ */
663+ TEST_FEATURE ("with extra argument");
664+ strcpy (buf, "setgid kvm foo\n");
665+
666+ pos = 0;
667+ lineno = 1;
668+ job = parse_job (NULL, NULL, NULL, "test", buf, strlen (buf), &pos, &lineno);
669+
670+ TEST_EQ_P (job, NULL);
671+
672+ err = nih_error_get ();
673+ TEST_EQ (err->number, NIH_CONFIG_UNEXPECTED_TOKEN);
674+ TEST_EQ (pos, 11);
675+ TEST_EQ (lineno, 1);
676+ nih_free (err);
677+}
678+
679 int
680 main (int argc,
681 char *argv[])
682@@ -7979,6 +8211,8 @@
683 test_stanza_limit ();
684 test_stanza_chroot ();
685 test_stanza_chdir ();
686+ test_stanza_setuid ();
687+ test_stanza_setgid ();
688
689 return 0;
690 }
691
692=== modified file 'util/tests/test_user_sessions.sh'
693--- util/tests/test_user_sessions.sh 2011-07-25 14:49:17 +0000
694+++ util/tests/test_user_sessions.sh 2011-12-07 15:52:24 +0000
695@@ -470,6 +470,45 @@
696 rm -f "$job_file"
697 }
698
699+test_user_job_setuid_setgid()
700+{
701+ group="user job with setuid and setgid me"
702+ job_name="setuid_setgid_me_test"
703+ script="\
704+setuid $(id -un)
705+setgid $(id -gn)
706+exec true"
707+ test_user_job "$group" "$job_name" "$script" no ""
708+
709+ TEST_GROUP "user job with setuid and setgid root"
710+ script="\
711+setuid root
712+setgid root
713+exec true"
714+
715+ job_name="setuid_setgid_root_test"
716+ job_file="${test_dir}/${job_name}.conf"
717+ job="${test_dir_suffix}/${job_name}"
718+
719+ echo "$script" > $job_file
720+
721+ ensure_job_known "$job" "$job_name"
722+
723+ TEST_FEATURE "ensure job fails to start as root"
724+ cmd="start ${job}"
725+ output=$(eval "$cmd")
726+ rc=$?
727+ TEST_EQ "$cmd" $rc 1
728+
729+ TEST_FEATURE "ensure 'start' indicates job failure"
730+ error=$(echo "$output"|grep failed)
731+ TEST_NE "error" "$error" ""
732+
733+ TEST_FEATURE "ensure 'initctl' does not list job"
734+ initctl list|grep -q "^$job stop/waiting" || \
735+ TEST_FAILED "job $job_name not listed as stopped"
736+}
737+
738 test_user_jobs()
739 {
740 test_user_job_binary
741@@ -480,6 +519,8 @@
742 test_user_job_single_line_script_task
743 test_user_job_multi_line_script_task
744
745+ test_user_job_setuid_setgid
746+
747 test_user_emit_events
748 }
749

Subscribers

People subscribed via source and target branches