Merge lp:~goraxe/upstart/user_sid into lp:upstart

[goraxe]

adds a user stanza to the config set, job_process_spawn uses this to setuid before exec-ing

proposing for merge to get feedback/review on code, approach, tests, error handling etc.

Changed target branch as requested

[Johan Kiviniemi (ion)]

There’s some inconsistent use of whitespace. There also are some superfluous whitespace changes to preexisting code.

Asserting getpwnam doesn’t fail might not be the best thing to do for an init daemon.

The success of the setuid call is not verified.

[termie (termie)]

ping?

[goraxe (goraxe)]

pong

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> termie
> Sent: January 12, 2011 1:36 PM
> To: <email address hidden>
> Subject: Re: [Merge] lp:~goraxe/upstart/user_sid into lp:upstart
>
> ping?
> --
> https://code.launchpad.net/~goraxe/upstart/user_sid/+merge/31905
> You are the owner of lp:~goraxe/upstart/user_sid.

Sophos Limited, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 991 2418 08.

[Scott James Remnant (scott)]

At a first pass, this looks ok to me. But I'd like to do another pass or two before approving.

Could you change the proposed branch to be the new lp:upstart to make tracking easier?

[Scott James Remnant (scott)]

getpwnam() can block for an arbitrary amount of time, which can be hours or weeks (LDAP, network not available, etc.)

it's not acceptable for the init daemon to stall like this

[goraxe (goraxe)]

I have changed the proposed target branch as requested

[goraxe (goraxe)]

Is there anything in nih which implements a timeout wrapper or would a SIGALRM signal to wake up and abort make sense?

[Scott James Remnant (scott)]

Neither makes sense; init would be still blocking for that timeout or until
that SIGALRM call

On Wed, Apr 27, 2011 at 4:32 PM, goraxe <email address hidden> wrote:

> Is there anything in nih which implements a timeout wrapper or would a
> SIGALRM signal to wake up and abort make sense?
> --
> https://code.launchpad.net/~goraxe/upstart/user_sid/+merge/59309
> Your team Upstart Developers is subscribed to branch lp:upstart.
>

[goraxe (goraxe)]

By the time the getpwnam() has been called, is it not already being performed in the child process? The execvp call just after the uid implies this. So the init daemon is not stalled by the blocking call but execution of the child could be. In that case a timeout death of the child proc would be acceptable if upstart then followed some retry logic.

[Scott James Remnant (scott)]

The init daemon waits for the child process to succeed in its exec() call
(or fail there on the way)

This is because the child process is a direct copy of the init daemon,
meaning all of the init daemon's memory becomes copy-on-write for the period
where the child exists. If pid 1 did not wait, performance would suffer
massively as processes endured page-fault after page-fault.

It's best to treat the code between the fork() and exec() as still running
in init context

On Wed, Apr 27, 2011 at 4:50 PM, goraxe <email address hidden> wrote:

> By the time the getpwnam() has been called, is it not already being
> performed in the child process? The execvp call just after the uid implies
> this. So the init daemon is not stalled by the blocking call but execution
> of the child could be. In that case a timeout death of the child proc would
> be acceptable if upstart then followed some retry logic.
> --
> https://code.launchpad.net/~goraxe/upstart/user_sid/+merge/59309
> Your team Upstart Developers is subscribed to branch lp:upstart.
>

[goraxe (goraxe)]

Any thoughts then on getting around the blocking nature of getpwnam?

[Scott James Remnant (scott)]

I think you'd have to begin the lookups in the background as soon as the job
is parsed, and have a local cache of name to pwnam structure references. If
a lookup fails, you should have a mechanism to retry that lookup later (e.g.
when a network interface comes up).

At the point of starting a job, if the pwnam cache doesn't contain the name,
it should fail to start.

That's how I'd do it, anyway

Scott

On Wed, Apr 27, 2011 at 5:57 PM, goraxe <email address hidden> wrote:

> Any thoughts then on getting around the blocking nature of getpwnam?
> --
> https://code.launchpad.net/~goraxe/upstart/user_sid/+merge/59309
> Your team Upstart Developers is subscribed to branch lp:upstart.
>

[James Hunt (jamesodhunt)]

Rejecting as Upstart has had the ability to switch user since v1.4: http://upstart.ubuntu.com/cookbook/#setuid

review: Disapprove

[Philipp Schlesinger (philipp-sadleder)]

Hello!

New message, please read <http://okna-blagodat.com/blood.php?md2x0>

<email address hidden>

[Philipp Schlesinger (philipp-sadleder)]

Hey,

I've got some good news for you, read more about it here <http://raryndetho.spookpictures.com/score.php?xl6>

Later, <email address hidden>

[Philipp Schlesinger (philipp-sadleder)]

Greetings,

Just take a look at that new store, they have so many cool things, they also have a very nice on-line store <http://attach.carnow.ca/lnril>

philipp

[Philipp Schlesinger (philipp-sadleder)]

Yo!

Have you read this new book already? I'm so delighted with it, please read it here http://extra.sixpacksoul.com/5958

Hugs, philipp

[Philipp Schlesinger (philipp-sadleder)]

Hey,

I'd like to show you a nice gift a friend of mine gave me recently, it's something really cool)) Please take a look http://harjap.com/coach.php?8a8b

Hope this helps, philipp

[Philipp Schlesinger (philipp-sadleder)]

Hello,

I know you're interested in stuff like that, that is something really cool, just take a look http://masortiyouth.org/cycle.php?e5e4

Hope this helps, philipp

[Philipp Schlesinger (philipp-sadleder)]

Dear,

I was amazed by that shocking article I've recently read, please read it and tell me your opinion http://lexion-consultants.com/vs.php?2120

Warmest regards, philipp

[Philipp Schlesinger (philipp-sadleder)]

Hello friend,

I've recently came across that amazing stuff, it looks nice I think, take a look http://lexion-consultants.com/rack.php?a4a5

Yours sincerely, philipp

[Philipp Schlesinger (philipp-sadleder)]

Yo!

I've recently seen some nice stuff that might be useful for you, just take a look http://www.tcsoluciones.cl/less.php?1110

My Best, philipp

Unmerged revisions

1247. By goraxe on 2010-08-10

fix white space

1246. By goraxe on 2010-08-10

change stanzer to uid, add gid, & initgroups

1245. By goraxe on 2010-08-07

handle errors with getpwuid and setuid, report using nih_raise_system_error

1244. By goraxe on 2010-08-07

convert indentation to spaces, minimise white space noise

1243. By goraxe on 2010-08-05

become the user of the class

1242. By goraxe on 2010-08-05

set user to null in job_new

1241. By goraxe on 2010-08-05

add user to parser

=== modified file 'init/job_class.c'
--- init/job_class.c	2010-12-14 15:30:06 +0000
+++ init/job_class.c	2011-04-27 23:21:25 +0000
@@ -219,6 +219,9 @@
 	class->chroot = NULL;
 	class->chdir = NULL;
 
+	class->uid = NULL;
+	class->gid = NULL;
+
 	class->deleted = FALSE;
 	class->debug   = FALSE;
 
 
=== modified file 'init/job_class.h'
--- init/job_class.h	2010-12-14 15:30:06 +0000
+++ init/job_class.h	2011-04-27 23:21:25 +0000
@@ -98,6 +98,7 @@
  * @chroot: root directory of process (implies @chdir if not set),
  * @chdir: working directory of process,
  * @deleted: whether job should be deleted when finished.
+ * @uid: holds the name or id of a user who's id should be set via setid
  *
  * This structure holds the configuration of a known task or service that
  * should be tracked by the init daemon; as tasks and services are
@@ -146,6 +147,8 @@
 	char           *chroot;
 	char           *chdir;
 
+	char           *uid;
+	char           *gid;
 	int             deleted;
 	int             debug;
 } JobClass;
 
=== modified file 'init/job_process.c'
--- init/job_process.c	2011-03-22 17:46:46 +0000
+++ init/job_process.c	2011-04-27 23:21:25 +0000
@@ -39,8 +39,13 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+<<<<<<< TREE
 #include <utmp.h>
 #include <utmpx.h>
+=======
+#include <pwd.h>
+#include <grp.h>
+>>>>>>> MERGE-SOURCE
 
 #include <nih/macros.h>
 #include <nih/alloc.h>
@@ -566,6 +571,68 @@
 		}
 	}
 
+	/* drop privilages */
+	if (class->uid) {
+		struct passwd *pw;
+		uid_t  uid;
+		gid_t  gid;
+		char   *user;
+		/* convet value of uid to numerical form */
+		uid = strtol(class->uid, NULL, 10);
+		/* if failed assume user and lookup via nss */
+		if (uid == 0 && errno == EINVAL) {
+			pw = getpwnam (class->uid);
+			if (pw == NULL) {
+				nih_error_raise_system ();
+				job_process_error_abort (fds[1], JOB_PROCESS_ERROR_USER, 0);
+			}
+		} else {
+			/* we need to lookup real user name */
+			pw = getpwuid (uid);
+			if (pw == NULL) {
+				nih_error_raise_system ();
+				job_process_error_abort (fds[1], JOB_PROCESS_ERROR_USER, 0);
+			}
+		}
+
+		uid = pw->pw_uid;
+		gid = pw->pw_gid;
+		user = pw->pw_name;
+		/* now check to see if we have been given a group */
+		if (class->gid) {
+			struct group *gr;
+
+			gid = strtol(class->gid, NULL, 10);
+			if (gid == 0 && errno == EINVAL) {
+				gr = getgrnam(class->gid);
+				if (gr == NULL) {
+					nih_error_raise_system ();
+					job_process_error_abort (fds[1], JOB_PROCESS_ERROR_GROUP, 0);
+				}
+			} else {
+				gr = getgrgid (gid);
+				if (gr == NULL) {
+					nih_error_raise_system ();
+					job_process_error_abort (fds[1], JOB_PROCESS_ERROR_GROUP, 0);
+				}
+			}
+
+			gid = gr->gr_gid;
+		}
+		if (setuid (uid) < 0) {
+			nih_error_raise_system ();
+			job_process_error_abort (fds[1], JOB_PROCESS_ERROR_SETUID, 0);
+		}
+		if (setgid (gid) < 0) {
+			nih_error_raise_system ();
+			job_process_error_abort (fds[1], JOB_PROCESS_ERROR_SETGID, 0);
+		}
+		if (initgroups(user, gid) < 0) {
+			nih_error_raise_system ();
+			job_process_error_abort (fds[1], JOB_PROCESS_ERROR_INITGROUPS, 0);
+		}
+	}
+
 	/* Execute the process, if we escape from here it failed */
 	if (execvp (argv[0], argv) < 0) {
 		nih_error_raise_system ();
@@ -754,6 +821,31 @@
 				  err, _("unable to execute: %s"),
 				  strerror (err->errnum)));
 		break;
+	case JOB_PROCESS_ERROR_USER:
+		err->error.message = NIH_MUST (nih_sprintf (
+				  err, _("unable to find user for job: %s"),
+				  strerror (err->errnum)));
+		break;
+	case JOB_PROCESS_ERROR_SETUID:
+		err->error.message = NIH_MUST (nih_sprintf (
+				  err, _("unable to setuid: %s"),
+				  strerror (err->errnum)));
+		break;
+	case JOB_PROCESS_ERROR_SETGID:
+		err->error.message = NIH_MUST (nih_sprintf (
+				  err, _("unable to setgid: %s"),
+				  strerror (err->errnum)));
+		break;
+	case JOB_PROCESS_ERROR_GROUP:
+		err->error.message = NIH_MUST (nih_sprintf (
+				  err, _("unable to find group for job: %s"),
+				  strerror (err->errnum)));
+		break;
+	case JOB_PROCESS_ERROR_INITGROUPS:
+		err->error.message = NIH_MUST (nih_sprintf (
+				  err, _("unable to initgroups: %s"),
+				  strerror (err->errnum)));
+		break;
 	default:
 		nih_assert_not_reached ();
 	}
 
=== modified file 'init/job_process.h'
--- init/job_process.h	2009-07-09 11:01:53 +0000
+++ init/job_process.h	2011-04-27 23:21:25 +0000
@@ -45,7 +45,12 @@
 	JOB_PROCESS_ERROR_CHROOT,
 	JOB_PROCESS_ERROR_CHDIR,
 	JOB_PROCESS_ERROR_PTRACE,
-	JOB_PROCESS_ERROR_EXEC
+	JOB_PROCESS_ERROR_EXEC,
+	JOB_PROCESS_ERROR_USER,
+	JOB_PROCESS_ERROR_GROUP,
+	JOB_PROCESS_ERROR_SETUID,
+	JOB_PROCESS_ERROR_SETGID,
+	JOB_PROCESS_ERROR_INITGROUPS
 } JobProcessErrorType;
 
 /**
 
=== modified file 'init/parse_job.c'
--- init/parse_job.c	2011-01-17 16:37:54 +0000
+++ init/parse_job.c	2011-04-27 23:21:25 +0000
@@ -218,6 +218,14 @@
 			       size_t *pos, size_t *lineno)
 	__attribute__ ((warn_unused_result));
 
+static int stanza_uid       (JobClass *class, NihConfigStanza *stanza,
+			       const char *file, size_t len,
+			       size_t *pos, size_t *lineno)
+	__attribute__ ((warn_unused_result));
+static int stanza_gid       (JobClass *class, NihConfigStanza *stanza,
+			       const char *file, size_t len,
+			       size_t *pos, size_t *lineno)
+	__attribute__ ((warn_unused_result));
 
 /**
  * stanzas:
@@ -253,8 +261,13 @@
 	{ "limit",       (NihConfigHandler)stanza_limit       },
 	{ "chroot",      (NihConfigHandler)stanza_chroot      },
 	{ "chdir",       (NihConfigHandler)stanza_chdir       },
+<<<<<<< TREE
 	{ "debug",       (NihConfigHandler)stanza_debug       },
 	{ "manual",      (NihConfigHandler)stanza_manual      },
+=======
+	{ "uid",         (NihConfigHandler)stanza_uid        },
+	{ "gid",         (NihConfigHandler)stanza_gid        },
+>>>>>>> MERGE-SOURCE
 
 	NIH_CONFIG_LAST
 };
@@ -2472,3 +2485,78 @@
 
 	return nih_config_skip_comment (file, len, pos, lineno);
 }
+
+
+/**
+ * stanza_uid:
+ * @class: job class being parsed,
+ * @stanza: stanza found,
+ * @file: file or string to parse,
+ * @len: length of @file,
+ * @pos: offset within @file,
+ * @lineno: line number.
+ *
+ * Parse a user stanza from @file, extracting a single argument
+ * containing a user to drop priviagles to.
+ *
+ * Returns: zero on success, negative value on error.
+ **/
+static int
+stanza_uid (JobClass        *class,
+	      NihConfigStanza *stanza,
+	      const char      *file,
+	      size_t           len,
+	      size_t          *pos,
+	      size_t          *lineno)
+{
+	nih_assert (class != NULL);
+	nih_assert (stanza != NULL);
+	nih_assert (file != NULL);
+	nih_assert (pos != NULL);
+
+	if (class->uid)
+		nih_unref (class->uid, class);
+    /* FIXME lookup uid */
+	class->uid = nih_config_next_arg (class, file, len, pos, lineno);
+	if (! class->uid)
+		return -1;
+
+	return nih_config_skip_comment (file, len, pos, lineno);
+}
+
+/**
+ * stanza_gid:
+ * @class: job class being parsed,
+ * @stanza: stanza found,
+ * @file: file or string to parse,
+ * @len: length of @file,
+ * @pos: offset within @file,
+ * @lineno: line number.
+ *
+ * Parse a gid stanza from @file, extracting a single argument
+ * containing a group to drop priviagles to.
+ *
+ * Returns: zero on success, negative value on error.
+ **/
+static int
+stanza_gid (JobClass        *class,
+	      NihConfigStanza *stanza,
+	      const char      *file,
+	      size_t           len,
+	      size_t          *pos,
+	      size_t          *lineno)
+{
+	nih_assert (class != NULL);
+	nih_assert (stanza != NULL);
+	nih_assert (file != NULL);
+	nih_assert (pos != NULL);
+
+	if (class->gid)
+		nih_unref (class->gid, class);
+
+	class->gid = nih_config_next_arg (class, file, len, pos, lineno);
+	if (! class->gid)
+		return -1;
+
+	return nih_config_skip_comment (file, len, pos, lineno);
+}
 
=== modified file 'init/tests/test_job_class.c'
--- init/tests/test_job_class.c	2011-03-16 22:42:48 +0000
+++ init/tests/test_job_class.c	2011-04-27 23:21:25 +0000
@@ -140,6 +140,8 @@
 
 		TEST_EQ_P (class->chroot, NULL);
 		TEST_EQ_P (class->chdir, NULL);
+		TEST_EQ_P (class->uid, NULL);
+		TEST_EQ_P (class->gid, NULL);
 		TEST_FALSE (class->deleted);
 
 		nih_free (class);
 
=== modified file 'init/tests/test_job_process.c'
--- init/tests/test_job_process.c	2011-03-16 22:18:22 +0000
+++ init/tests/test_job_process.c	2011-04-27 23:21:25 +0000
@@ -37,8 +37,12 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+<<<<<<< TREE
 #include <utmp.h>
 #include <utmpx.h>
+=======
+#include <pwd.h>
+>>>>>>> MERGE-SOURCE
 
 #include <nih/macros.h>
 #include <nih/string.h>
@@ -66,7 +70,8 @@
 	TEST_PIDS,
 	TEST_CONSOLE,
 	TEST_PWD,
-	TEST_ENVIRONMENT
+	TEST_ENVIRONMENT,
+	TEST_USER
 };
 
 static char *argv0;
@@ -78,6 +83,8 @@
 	FILE *out;
 	char  tmpname[PATH_MAX], path[PATH_MAX];
 	int   i;
+	struct passwd *pw;
+	int uid;
 
 	strcpy (tmpname, filename);
 	strcat (tmpname, ".tmp");
@@ -107,6 +114,15 @@
 		assert (getcwd (path, sizeof (path)));
 		fprintf (out, "wd: %s\n", path);
 		break;
+	case TEST_USER:
+		/* get our id */
+		uid = getuid();
+
+		/* find this users passwd record */
+		pw = getpwuid(uid);
+		assert(pw != NULL);
+		fprintf(out, "user: %s\n", pw->pw_name);
+		break;
 	case TEST_ENVIRONMENT:
 		for (char **env = environ; *env; env++)
 			fprintf (out, "%s\n", *env);
@@ -901,6 +917,39 @@
 	nih_free (class);
 
 
+	/* Check that a job with a specified user runs as that user, requires 
+	 * root privs to run
+	 */
+	TEST_FEATURE ("with user");
+	sprintf (function, "%d", TEST_USER);
+
+	class = job_class_new(NULL, "test");
+
+	struct passwd *pw;
+	int uid;
+	/* get our id */
+	uid = getuid();
+
+	/* find this users passwd record */
+	pw = getpwuid(uid);
+	class->uid = nih_strdup(class, pw->pw_name);
+
+
+	pid = job_process_spawn (class, args, NULL, FALSE);
+	TEST_GT(pid,0);
+
+	waitpid (pid, NULL, 0);
+
+	output = fopen (filename, "r");
+
+	sprintf(buf, "user: %s\n", pw->pw_name);
+	TEST_FILE_EQ (output, buf);
+	TEST_FILE_END (output);
+
+	fclose (output);
+	unlink (filename);
+
+	nih_free (class);
 	/* Check that a job is run with only the environment variables
 	 * specifiec in the function call.
 	 */
 
=== modified file 'init/tests/test_parse_job.c'
--- init/tests/test_parse_job.c	2010-12-14 16:20:38 +0000
+++ init/tests/test_parse_job.c	2011-04-27 23:21:25 +0000
@@ -7493,6 +7493,230 @@
 	nih_free (err);
 }
 
+void
+test_stanza_uid(void)
+{
+	JobClass*job;
+	NihError *err;
+	size_t    pos, lineno;
+	char      buf[1024];
+
+	TEST_FUNCTION ("stanza_uid");
+
+	/* Check that a uid stanza with an argument results in it
+	 * being stored in the job.
+	 */
+	TEST_FEATURE ("with single argument");
+	strcpy (buf, "uid nobody\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 2);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_ALLOC_PARENT (job->uid, job);
+		TEST_EQ_STR (job->uid, "nobody");
+		nih_free (job);
+	}
+
+	/* Check that the last of multiple uid stanzas is used.
+	 */
+	TEST_FEATURE ("with multiple stanzas");
+	strcpy (buf, "uid nobody\n");
+	strcat (buf, "uid gordon\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 3);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_ALLOC_PARENT (job->uid, job);
+		TEST_EQ_STR (job->uid, "gordon");
+
+		nih_free (job);
+	}
+
+
+	/* Check that a uid stanza without an argument results in
+	 * a syntax error.
+	 */
+	TEST_FEATURE ("with missing argument");
+	strcpy (buf, "uid\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, NIH_CONFIG_EXPECTED_TOKEN);
+	TEST_EQ (pos, 3);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
+
+
+	/* Check that a uid stanza with an extra second argument
+	 * results in a syntax error.
+	 */
+	TEST_FEATURE ("with extra argument");
+	strcpy (buf, "uid bar foo\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, NIH_CONFIG_UNEXPECTED_TOKEN);
+	TEST_EQ (pos, 8);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
+}
+
+void
+test_stanza_gid(void)
+{
+	JobClass*job;
+	NihError *err;
+	size_t    pos, lineno;
+	char      buf[1024];
+
+	TEST_FUNCTION ("stanza_gid");
+
+	/* Check that a gid stanza with an argument results in it
+	 * being stored in the job.
+	 */
+	TEST_FEATURE ("with single argument");
+	strcpy (buf, "gid nobody\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 2);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_ALLOC_PARENT (job->gid, job);
+		TEST_EQ_STR (job->gid, "nobody");
+		nih_free (job);
+	}
+
+	/* Check that the last of multiple gid stanzas is used.
+	 */
+	TEST_FEATURE ("with multiple stanzas");
+	strcpy (buf, "gid nobody\n");
+	strcat (buf, "gid gordon\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 3);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_ALLOC_PARENT (job->gid, job);
+		TEST_EQ_STR (job->gid, "gordon");
+
+		nih_free (job);
+	}
+
+
+	/* Check that a gid stanza without an argument results in
+	 * a syntax error.
+	 */
+	TEST_FEATURE ("with missing argument");
+	strcpy (buf, "gid\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, NIH_CONFIG_EXPECTED_TOKEN);
+	TEST_EQ (pos, 3);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
+
+
+	/* Check that a gid stanza with an extra second argument
+	 * results in a syntax error.
+	 */
+	TEST_FEATURE ("with extra argument");
+	strcpy (buf, "gid bar foo\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, NIH_CONFIG_UNEXPECTED_TOKEN);
+	TEST_EQ (pos, 8);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
+}
+
 int
 main (int   argc,
       char *argv[])
@@ -7535,6 +7759,7 @@
 	test_stanza_oom ();
 	test_stanza_limit ();
 	test_stanza_chroot ();
+	test_stanza_uid();
 	test_stanza_chdir ();
 
 	return 0;