Discussion summary: Mock security

[Michael E Brown]

Current mock contains one SUID program, mock-helper, that can be run
only by people in the 'mock' group. This helper script performs many
actions as root that cannot be done by the main mock python executable.
There is one question we need to answer in regards to the current
mock-helper security model. 
	-- Should we allow untrusted users access to the 'mock' group?

If we answer 'yes' to this question, that implies that we must make
mock-helper bulletproof for users in the 'mock' group who wish to use
mock-helper to leverage access to 'root' on the box.

After looking closely at the mock-helper source, I have identified
several problematic areas, listed below. I do not believe, given the
current state of mock-helper, that we should endorse the idea of
allowing untrusted users access to the 'mock' group. We should very
prominently label mock as giving, essentially, root access to each user
you allow to run it. I believe the wiki, the help text of "mock -h", the
mock README, and the mock man page should all be updated with this
information.

Proposed warning text:
====================================================================
Mock contains a Set-UID helper program that can only be run by users in
the 'mock' group. Adding users to the 'mock' group is equivalent to
giving them full root access to the OS. Do not add untrusted users to
the 'mock' group. Mock is not designed to safely be run by untrusted
users.
====================================================================

Problem area: do_mknod()
	passes unchecked user data to the mknod command. User can use
the "-m" to set insecure permissions on, for example, a hard disk device
node.

Proposed Solution:
	Do not pass user-supplied input to mknod. Make an array of
allowed devices, along with major/minor and permissions. Mock-helper
mknod should only be passed the device name, and it should look up the
major/minor/perms from the array.


Problem area: do_chroot()
	passes unchecked user data to chroot command. User can easily
get a shell, and, for example, create device nodes.

Proposed Solution:
	Do not pass user-supplied data to chroot. Make an array of
allowed commands, mock.py should just pass which command it wants to run
from the list. Any required user input should be filtered for
[A-Za-z0-9,+\-.], or similarly strict regexp. No extra options should be
allowed to the command (for example, no input starting with '-')


Problem area: do_rm()
	does not check all user input. Only checks argv[2] and argv[3],
yet passes all args received to rm. User can add new options, as long as
they come after argv[3]. User could remove system files by passing as
argv[4] or later.

Proposed solution:
	Add check to allow only one argument. Force '-rf' argument.
Perform strict validation on the one  argument passed.


Problem area: do_mount()
	does not check all user input. User can pass extra args that
will be passed to mount unchecked.

Proposed solution:
	array of allowed mounts.


Problem area: do_yum()
	does not check all user input. For example, user could possibly
pass an extra --installroot / later in the command.

Proposed solution:
	Check all user input. Disallow extra arguments.


Problem area: do_unpack()
	does not check that tarball is from secure directory. Could
contain insecure /dev/ files (see do_mknod())


--
Michael Brown