> On Sept. 1, 2016, 12:17 a.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, line 239
> > <https://reviews.apache.org/r/51564/diff/3/?file=1489394#file1489394line239>
> >
> > This changes seems to come with a severe security risk. As an normal
> > user, I can now gain root on any agent:
> >
> > * Prepare a docker/appc container with a manually crafted user with UID
> > 0 but with my role name.
> > * Launch the container with said role name.
> > * The sandbox code will bail out early here and don't proceed to create
> > an unpriviledged user
> > * Setuid will switch from root to my prepare custom user with root
> > permissions
> > * Game over
> >
> > Unless someone can correct me here, that would be a -1 from my end.
>
> Joshua Cohen wrote:
> I'm not sure about step 4 above. Are you referring to the [setuid in
> process.py](https://github.com/apache/aurora/blob/master/src/main/python/apache/thermos/core/process.py#L369-L380)?
> If so, that setuid shouldn't be switching to root, it will be switching to
> the user matching the role name on the host system, the uid set in your
> docker/appc image wouldn't have any impact on that. Am I missing something?
>
> John Sirois wrote:
> Joshua mentioned this in Slack/IRC, but I do think we need to ensure the
> uid/uname and gid/gname pairs in the chroot match those of the host system
> when we hit an exists condition in either direction.
>
> Given:
> Job author only specifies a role name, in this example `jsirois`
>
> Scenarios:
> 1. host (uid=1000, uname=jsirois) chroot (uid=500, uname=jsirois)
> 2. host (uid=1000, uname=jsirois) chroot (uid=1000, uname=fred)
> 3. host (uid=1000, uname=jsirois) chroot (uid=1000, uname=jsirois)
>
> A Job author can have task code that references the role name, for
> example it might shell out a call to `id -g jsirois` where the role name is
> `jsirois` to find the primary group id for the current role. It seems then
> that we must ensure the chroot has the role name available, and fwict,
> besides the special case of uid 0, we don't really care what the uid is. If
> it matches that's fine, but since the chroot environment will share nothing
> with the host, ids need not match (IIUC).
>
> So it seems to me scenarios 1 and 3 are OK - the sandbox can move along.
> Scenario 2 though should fail (we currently pass).
>
> John Sirois wrote:
> > Joshua mentioned this in Slack/IRC, but I do think we need to ensure
> the uid/uname and gid/gname pairs in the chroot match those of the host
> system when we hit an exists condition in either direction.
>
> Obviously I changed my thinking as I composed the
> Given/Scenarios/Conclusion below that statement!