Code review comment for lp:~broder/upstart/drop-privileges

* 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`").