upstart 1.4: setuid/setguid apply to ALL scripts

Bug #911207 reported by James Page
48
This bug affects 10 people
Affects Status Importance Assigned to Milestone
upstart
Confirmed
Low
Unassigned
upstart (Ubuntu)
Confirmed
Low
Unassigned

Bug Description

Upstart 1.4 on Ubuntu Precise from https://launchpad.net/~jamesodhunt/+archive/upstart-job-logging

-----------------------

My understanding of the setuid/setguid stanza's in upstart 1.4 is that they should emulate what start-stop-daemon/daemon and suchlike do with regards to dropping privileges.

At the moment the stanza's apply to all script blocks (not just the main exec one) which makes it hard to setup /var/run directories etc.. which normally need to be created by root, not the owner of the application.

James Page (james-page)
description: updated
Revision history for this message
James Hunt (jamesodhunt) wrote :

Most stanzas currently operate on all script/exec sections (with the notable exception of 'respawn').

A fix would be for job_process_run() to pass its ProcessType parameter to job_process_spawn() so that for example:

    if (class->setuid) { ... }

would become:

    if (class->setuid && process == PROCESS_MAIN) { ... }

However, if we did make setuid+setgid only apply to the main script/exec section:

1) it could be confusing to users expecting that stanza to "apply" to all script/exec sections.
2) it would be potentially dangerous if users create files in a pre-start for example thinking they will be
    owned by the setuid user when in fact they'd be owned by root. It would also be slightly annoying
    since users would need to remember to chown any files created in a pre-start section for example
    to make sure their ownership matched the user specified in the setuid stanza.

That said, we can of course document the behaviour to guard against misunderstandings since maybe the only common scenario where setuid/setgid support is required is for the main script/exec section?

We could modify the setuid/setgid syntax to take an optional list of section names to apply the setuid/setgid to:

    setuid USERNAME [<section>]
    setgid GROUPNAME [<section>]

Example:

    setuid james pre-start post-start
    setgid james pre-stop

... but there is no "name" for the main section since it is either "exec" or "script". We could accept both but that's not very elegant in these scenarios:

    setuid james pre-start exec
    script
      echo hello
    end script

    setuid james pre-start script
    exec echo hello

We should also think ahead: what if we introduce 'mkdir', 'chown', and 'chmod' stanzas? Who would they run as if setuid is specified?

Revision history for this message
James Hunt (jamesodhunt) wrote :

Suggestion from Clint is to enhance pre- and post- syntax such that the setuid and setgid stanzas can also appear in a pre-post section. If they do, these values are used rather than the "globally" specified setuid/setgid value. For example:

________________________

# this is the "global" value for setuid. If not overriden, all job processes will run as user 'foo'
setuid foo

pre-start setuid bar script

    echo this runs as user 'bar'

end script

post-stop setuid baz exec echo this runs as user 'baz'

exec echo I run as user 'foo'
________________________

This is a very elegant solution to the problem. For now however, it is possible to work around the limitation by creating a separate .conf file to handle the pre/post conditions whilst running as root.

Changed in upstart (Ubuntu):
importance: Undecided → Low
Changed in upstart:
importance: Undecided → Low
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in upstart (Ubuntu):
status: New → Confirmed
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Seems like this should also be "Triaged", since a solution is known and accepted.

Changed in upstart:
status: New → Confirmed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.