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
Michael_E_Brown@Dell.com writes:
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.
Other solution:
* do not use mknod(2) but bind-mount a read-only (e.g. ramfs) filesystem into dev/; pseudo C-code:
| root_fd=open("/"); | chroot(ROOT) | chdir("/dev"); | new_fd=open("."); | fchdir(root_fd); | chroot("."); | fchdir(new_fd); | mount(DEV_IN_TRUSTED_DIR, ".", "none", MS_BIND);
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 '-')
can be always exploited; e.g. by placing an own version of an allowed command into the chroot
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.
Better solution:
* implement 'rm -rf' in C and do it after a chroot(2)
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.
Better solution:
* do not use /bin/mount but mount(2) in a way like in the example above
Problem area: do_unpack() does not check that tarball is from secure directory. Could contain insecure /dev/ files (see do_mknod())
Better solution:
* open tarball in a secure way and redirect the opened fd into tar's stdin
Enrico
Michael_E_Brown@Dell.com wrote:
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.
I think this makes sense to do as the short-term "so we can get mock 0.6 out" as there are a lot of cool and important stuff that people are clamoring for.
Then, post 0.6, focusing a bit on fixing the areas of security concern would seem to make sense.
Jeremy
On Tue, 2006-06-06 at 15:50 -0500, Michael_E_Brown@Dell.com wrote:
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. ====================================================================
... stuff deleted ...
If you implemented all your proposed solutions, do you think that warning could be removed? Do you think that mock would be secure at that point?
If the answer is no, how much effort do we want to spend here if at some point we have to trust the users in the "mock" group anyway?
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 '-')
I'm afraid you might be missing the point of the "mock chroot" command - which is to do arbitrary things on/in the chroot. By definition there is no predefined list of allowed commands.
In our case, we're using mock to cross-build RPMs. Here's how my co- worker Clark Williams williams@redhat.com described it a few months ago when he added the chroot command:
On Thu, 2006-03-02 at 10:09 -0600, Clark Williams wrote:
When cross-building a root filesystem, you need to bootstrap something we call a "sysroot", which is essentially a software development environment for the target system (headers and libraries mostly). The way we do that is we create an empty "root" and as we build RPMS (starting with GLIBC), we install the -devel packages into that target root (the sysroot). Our cross toolchains know how to access the sysroot, so when you compile hello.c, it gets the correct headers and libraries.
To be able to install rpms into the sysroot (which lives in the mock chroot) I've added a chroot command to my local copy of mock (patch below).
In our case the "mock chroot" command is really just a convenience, but it is a nice big fat one. It keeps all the mount logic and chroot logic in one place instead of us re-inventing the wheel. I'm hoping others are also using mock in weird and wonderful ways.
I'm not sure I can think of a great solution to the problem of tightening security of mock without totally removing the "mock chroot" command, which I'm certainly not in favor of.
A "hack" solution would be to have a second group, named something like "mockgurus", whose members would only be the only ones allowed to execute the "mock chroot" command. Of course we're really back in the same security boat here, but perhaps we've shrunk the boat a bit.
David Smith wrote:
On Tue, 2006-06-06 at 15:50 -0500, Michael_E_Brown@Dell.com wrote:
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.
... stuff deleted ...
If you implemented all your proposed solutions, do you think that warning could be removed? Do you think that mock would be secure at that point?
It sounds like Michael did a pretty full investigation. And things at least initially weren't bad off in this respect.
If the answer is no, how much effort do we want to spend here if at some point we have to trust the users in the "mock" group anyway?
We really want it to be pretty secure, though -- otherwise, it's a risk for build farms and a future where we might try to have shell servers available for all arches for people to test and debug builds.
Problem area: do_chroot() passes unchecked user data to chroot command. User can easily get a shell, and, for example, create device nodes.
[snip]
In our case, we're using mock to cross-build RPMs. Here's how my co- worker Clark Williams williams@redhat.com described it a few months ago when he added the chroot command:
What if, instead, it chrooted + changed to the mockbuild user? Would that break your usage case?
Jeremy
On Wed, 2006-06-07 at 11:13 -0400, Jeremy Katz wrote:
David Smith wrote:
On Tue, 2006-06-06 at 15:50 -0500, Michael_E_Brown@Dell.com wrote:
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.
... stuff deleted ...
If you implemented all your proposed solutions, do you think that warning could be removed? Do you think that mock would be secure at that point?
It sounds like Michael did a pretty full investigation. And things at least initially weren't bad off in this respect.
If the answer is no, how much effort do we want to spend here if at some point we have to trust the users in the "mock" group anyway?
We really want it to be pretty secure, though -- otherwise, it's a risk for build farms and a future where we might try to have shell servers available for all arches for people to test and debug builds.
Problem area: do_chroot() passes unchecked user data to chroot command. User can easily get a shell, and, for example, create device nodes.
[snip]
In our case, we're using mock to cross-build RPMs. Here's how my co- worker Clark Williams williams@redhat.com described it a few months ago when he added the chroot command:
What if, instead, it chrooted + changed to the mockbuild user? Would that break your usage case?
Jeremy
Hmm, it might. Actually we currently do the equivalent of
# mock chroot "/sbin/runuser - mockbuild -c "{command}""
on most of the commands we run (but not all). I'll see if we can't run as the mockbuild user all the time and let you know (probably tomorrow).
On Wed, 2006-06-07 at 10:20 -0500, David Smith wrote:
On Wed, 2006-06-07 at 11:13 -0400, Jeremy Katz wrote:
... stuff deleted ...
What if, instead, it chrooted + changed to the mockbuild user? Would that break your usage case?
Jeremy
Hmm, it might. Actually we currently do the equivalent of
# mock chroot "/sbin/runuser - mockbuild -c "{command}""
on most of the commands we run (but not all). I'll see if we can't run as the mockbuild user all the time and let you know (probably tomorrow).
I'm not sure if the results of my investigations are still needed at this point (with all the other ideas floating around), but here they are anyway.
In general running as mockbuild worked, except for a couple of cases:
1) Annoying problem. We copy a few files into the chroot, then chown them to the mockbuild user. Originally it was done like this:
# sudo cp foo /var/lib/mock/{chroot_name}/root/{whatever} # mock chroot chown mockbuild.mockbuild /{whatever}
Obviously the mockbuild user can't run chown, so I tried to just do it with sudo.
# sudo cp foo /var/lib/mock/{chroot_name}/root/{whatever} # sudo chown mockbuild.mockbuild /var/lib/mock/{chroot_name}/root/{whatever}
but of course the problem is that I don't know the mockbuild UID/GID in the host OS. I just hardcoded them for now and went on.
2) Real problem. I could do most other things as the mockbuild user, but one of the things we do is build up what we call a sysroot inside the chroot. The sysroot has its own RPM database (since it is a different architecture than the chroot). Unfortunately, when writing to the rpm database you have to be root. I don't see any good way around this one.
On Wed, 2006-06-07 at 11:13 -0400, Jeremy Katz wrote:
David Smith wrote:
On Tue, 2006-06-06 at 15:50 -0500, Michael_E_Brown@Dell.com wrote:
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.
... stuff deleted ...
If you implemented all your proposed solutions, do you think that warning could be removed? Do you think that mock would be secure at that point?
It sounds like Michael did a pretty full investigation. And things at least initially weren't bad off in this respect.
I don't want to lead anybody astray here. I am not a security expert, so there may well be other things here that I am missing. I just pointed out the ones that were obvious to me.
If the answer is no, how much effort do we want to spend here if at some point we have to trust the users in the "mock" group anyway?
Given the current state of mock-helper, I would say a different design is called for, one where 'untrusted' user cannot pass any data down to a trusted process, or as a last resort, all data that is passed is subject to rigorous validation.
We really want it to be pretty secure, though -- otherwise, it's a risk for build farms and a future where we might try to have shell servers available for all arches for people to test and debug builds.
That is the thinking behind my audit.
Problem area: do_chroot() passes unchecked user data to chroot command. User can easily get a shell, and, for example, create device nodes.
[snip]
In our case, we're using mock to cross-build RPMs. Here's how my co- worker Clark Williams williams@redhat.com described it a few months ago when he added the chroot command:
What if, instead, it chrooted + changed to the mockbuild user? Would that break your usage case?
Since the mockbuild user in the chroot == current user id non-chroot, this should preclude some problems, the problem is that many operations will not be able to be done by current user id due to permissions.
The two suggestions I have seen to fix the architecture here are: A) A daemon that runs as root. It accepts minimal 'commands' from untrusted user and uses those commands to build the chroot.
B) Same idea as above, but as a SUID program instead of daemon.
the problem with (B) is that it will be a C program, where (A) can be a python executable. I think we all know which one is easier to maintain.
The problem with the implementation of this solution is that it involves a pretty hefty rewrite of large portions of code, and then re-testing. It is only worth this effort if somebody steps forward and says they actually need this level of security. -- Michael
Hi,
In our case, we're using mock to cross-build RPMs. Here's how my co- worker Clark Williams williams@redhat.com described it a few months ago when he added the chroot command:
On Thu, 2006-03-02 at 10:09 -0600, Clark Williams wrote:
When cross-building a root filesystem, you need to bootstrap something we call a "sysroot", which is essentially a software development environment for the target system (headers and libraries mostly). The way we do that is we create an empty "root" and as we build RPMS (starting with GLIBC), we install the -devel packages into that target root (the sysroot). Our cross toolchains know how to access the sysroot, so when you compile hello.c, it gets the correct headers and libraries.
To be able to install rpms into the sysroot (which lives in the mock chroot) I've added a chroot command to my local copy of mock (patch below).
FYI, mach (which was the blueprint from which mock was forked) had commands like this since the beginning for these reasons. mock removed some functionality to basically only implement the build-one-package case.
Maybe you want to take a look at mach instead if these are the things you are doing - mach still has a wider focus than mock and possibly may suit you better.
See http://thomas.apestaart.org/projects/mach/
Thomas
On Fri, 2006-06-09 at 13:54 +0200, Thomas Vander Stichele wrote:
Hi,
In our case, we're using mock to cross-build RPMs. Here's how my co- worker Clark Williams williams@redhat.com described it a few months ago when he added the chroot command:
On Thu, 2006-03-02 at 10:09 -0600, Clark Williams wrote:
When cross-building a root filesystem, you need to bootstrap something we call a "sysroot", which is essentially a software development environment for the target system (headers and libraries mostly). The way we do that is we create an empty "root" and as we build RPMS (starting with GLIBC), we install the -devel packages into that target root (the sysroot). Our cross toolchains know how to access the sysroot, so when you compile hello.c, it gets the correct headers and libraries.
To be able to install rpms into the sysroot (which lives in the mock chroot) I've added a chroot command to my local copy of mock (patch below).
FYI, mach (which was the blueprint from which mock was forked) had commands like this since the beginning for these reasons. mock removed some functionality to basically only implement the build-one-package case.
Maybe you want to take a look at mach instead if these are the things you are doing - mach still has a wider focus than mock and possibly may suit you better.
See http://thomas.apestaart.org/projects/mach/
Thomas
I've used mach in the past (in fact you and I traded a few emails on it back in 2003).
We started with mock this time because it seemed (perhaps incorrectly) that more development effort was focused toward mock and because we had thoughts of using plague.
Thanks for the reminder.
Michael_E_Brown@Dell.com wrote:
-- Should we allow untrusted users access to the 'mock' group?
This has been a concern of mine as well. However, I think the solution is not to harden mockhelper, but to change the role of mockhelper.
At the moment, mock runs as a mortal user and uses mockhelper to execute a limited number of shell commands as root. What I'd like to do is have mock-helper (possibly renamed) run mock.py (and only mock.py) as root, letting mock.py take actions directly without having to filter back through mockhelper.
Consider that mock.py is in a much better position to make decisions about whether operations are sane or not. Giving users access to run mock.py as root could be much safer than giving them access to run a number of more general purpose tools.
On Wed, 2006-06-07 at 19:52 -0400, Mike McLean wrote:
Michael_E_Brown@Dell.com wrote:
-- Should we allow untrusted users access to the 'mock' group?
This has been a concern of mine as well. However, I think the solution is not to harden mockhelper, but to change the role of mockhelper.
At the moment, mock runs as a mortal user and uses mockhelper to execute a limited number of shell commands as root. What I'd like to do is have mock-helper (possibly renamed) run mock.py (and only mock.py) as root, letting mock.py take actions directly without having to filter back through mockhelper.
Ok, so this is the coolest proposed solution I have seen to this problem. I like it a lot. -- Michael
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Michael E Brown wrote:
On Wed, 2006-06-07 at 19:52 -0400, Mike McLean wrote:
Michael_E_Brown@Dell.com wrote:
-- Should we allow untrusted users access to the 'mock' group?This has been a concern of mine as well. However, I think the solution is not to harden mockhelper, but to change the role of mockhelper.
At the moment, mock runs as a mortal user and uses mockhelper to execute a limited number of shell commands as root. What I'd like to do is have mock-helper (possibly renamed) run mock.py (and only mock.py) as root, letting mock.py take actions directly without having to filter back through mockhelper.
Ok, so this is the coolest proposed solution I have seen to this problem. I like it a lot.
How would we tell that the mock.py being run as root is the mock.py we all know and love (and not one defiled by some black hat)?
Clark "not a security guru" Williams
Clark Williams wrote:
Michael E Brown wrote:
On Wed, 2006-06-07 at 19:52 -0400, Mike McLean wrote:
At the moment, mock runs as a mortal user and uses mockhelper to execute a limited number of shell commands as root. What I'd like to do is have mock-helper (possibly renamed) run mock.py (and only mock.py) as root, letting mock.py take actions directly without having to filter back through mockhelper.
Ok, so this is the coolest proposed solution I have seen to this problem. I like it a lot.
How would we tell that the mock.py being run as root is the mock.py we all know and love (and not one defiled by some black hat)?
So mockhelper would continue to perform env sanitation, and I imagine it will have a hard-coded path for mock.py. I suppose if we're really paranoid we could store the sha1sum of mock.py at compile time and check it at runtime, but I think restricting to running mock.py from the standard location is sufficient.
-----Original Message----- From: fedora-buildsys-list-bounces@redhat.com [mailto:fedora-buildsys-list-bounces@redhat.com] On Behalf Of Mike McLean Sent: Wednesday, June 07, 2006 9:04 PM To: Discussion of Fedora build system Subject: Re: Discussion summary: Mock security
Clark Williams wrote:
How would we tell that the mock.py being run as root is the
mock.py we
all know and love (and not one defiled by some black hat)?
So mockhelper would continue to perform env sanitation, and I imagine it will have a hard-coded path for mock.py. I suppose if we're really paranoid we could store the sha1sum of mock.py at compile time and check it at runtime, but I think restricting to running mock.py from the standard location is sufficient.
Hardcoded /usr/bin/mock.py is sufficient. If a black hat has somehow manged to subvert /usr/bin/mock.py, game over already.
There are no show-stoppers that I see, but there are a few changes to be made for security.
The --configdir option will need to be rethought a bit. Considering that the config files are executable code, we will have to change the config to separate 'trusted' from 'untrusted' configs where we do not execute code from untrusted sources.
The --resultdir and --statedir options will have to be changed to drop privileges before creating/writing to them, to disallow somebody from stomping system paths or using symlink attacks. -- Michael
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Mike McLean wrote:
Clark Williams wrote:
Michael E Brown wrote:
On Wed, 2006-06-07 at 19:52 -0400, Mike McLean wrote:
At the moment, mock runs as a mortal user and uses mockhelper to execute a limited number of shell commands as root. What I'd like to do is have mock-helper (possibly renamed) run mock.py (and only mock.py) as root, letting mock.py take actions directly without having to filter back through mockhelper.
Ok, so this is the coolest proposed solution I have seen to this problem. I like it a lot.
How would we tell that the mock.py being run as root is the mock.py we all know and love (and not one defiled by some black hat)?
So mockhelper would continue to perform env sanitation, and I imagine it will have a hard-coded path for mock.py. I suppose if we're really paranoid we could store the sha1sum of mock.py at compile time and check it at runtime, but I think restricting to running mock.py from the standard location is sufficient.
You're probably right. I do kinda like the SHA1 idea, but then I'm paranoid...
So we're saying that we turn this on it's head and let mock-helper become mock-launcher? It sanitizes the environment, does it's setuid thing to root and then execs mock with an arg vector. Nice and simple. As you and Michael already pointed out, a hard-coded path to mock will probably be sufficient for security. We could actually name this new executable "mock", put it in /usr/bin and have it launch mock.py from a private location (not sure what the LFS directive is there).
This is too straightforward. What are we missing?
Clark
buildsys@lists.fedoraproject.org
-
Clark Williams -
David Smith -
Enrico Scholz -
Jeremy Katz -
Michael E Brown -
Michael E Brown
-
Mike McLean -
Thomas Vander Stichele