Bug 953519
| Summary: | After changing config_opts['chrootgid'], the process still runs with mock group privileges | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora EPEL | Reporter: | Michal Toman <mtoman> | ||||||
| Component: | mock | Assignee: | Clark Williams <williams> | ||||||
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
| Severity: | urgent | Docs Contact: | |||||||
| Priority: | urgent | ||||||||
| Version: | el6 | CC: | dwysocha, harshula, kwalker, matthew, mebrown, pknirsch, rvokal, williams | ||||||
| Target Milestone: | --- | Keywords: | Reopened | ||||||
| Target Release: | --- | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | mock-1.1.38-1.el6 | Doc Type: | Bug Fix | ||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2013-11-10 06:37:04 UTC | Type: | Bug | ||||||
| Regression: | --- | Mount Type: | --- | ||||||
| Documentation: | --- | CRM: | |||||||
| Verified Versions: | Category: | --- | |||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Bug Depends On: | |||||||||
| Bug Blocks: | 948138 | ||||||||
| Attachments: |
|
||||||||
I believe there are still places in the code that expect the group name to be 'mock'. We've never really tested the ability to change the mock groupname. This turns out to be harder than I thought, mainly because of the chicken<->egg nature of the early startup of mock.
The way it works now is that we get the gid of the 'mock' group and to a setgroups() call using this value after we determine how we got launched (sudo, user-helper, etc.). Only after we do that do we parse the arguments and read the config file, which is the first place we would see config_opts['chrootgid'].
I *might* see a way to do what you want. After we've parsed the config, we can check the value of config_opts['chrootgid'] against mockgid and if they differ we can add config_opts['chrootgid'] with another setgroups() call.
The only problem is that I can't see a way around you having to modify /etc/pam.d/mock to change all instances of 'mock' to be whatever groupname you're using.
Here's the change to mock.py that I'm considering:
diff --git a/py/mock.py b/py/mock.py
index 939f5ae..a91b030 100755
--- a/py/mock.py
+++ b/py/mock.py
@@ -407,6 +407,10 @@ def main(ret):
(options, args) = command_parse(config_opts)
+ # allow a different mock group to be specified
+ if config_opts['chrootgid'] != mockgid:
+ os.setgroups((mockgid, config_opts['chrootgid']))
+
if options.printrootpath:
options.verbose = 0
Other than running it through my regression suite without changing the test config, I haven't tried this out yet. Still pondering the ramifications.
mock-1.1.33-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/mock-1.1.33-1.fc19 mock-1.1.33-1.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/mock-1.1.33-1.fc18 mock-1.1.33-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mock-1.1.33-1.el6 Package mock-1.1.33-1.el6: * should fix your issue, * was pushed to the Fedora EPEL 6 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=epel-testing mock-1.1.33-1.el6' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-EPEL-2013-11294/mock-1.1.33-1.el6 then log in and leave karma (feedback). mock-1.1.33-1.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report. Does not work, reopening. The check you've added is executed before reading the config file, thus "if config_opts['chrootgid'] != mockgid:" is always false as the default for config_opts['chrootgid'] is mockgid. In addition at the very same place mock has already dropped the privileges and even if the condition was evaluated properly, the following os.setgroups() would fail. Created attachment 805517 [details]
patch to set the group definde in chrootgid
The patch that I attached should fix the issue with minimal code change. It makes use of the dropPrivsTemp function provided by mockbuild/uid.py. Once the config is parsed if chrootgid != mockgid, we'll restorePrivs(), set the group, and then drop privilages again.
However, it looks like there is some legacy code in main(), it might make sense to clean some of it up, but I don't fully grasp all the ways mock can be invoked.
Is there any function besides --help that mock should be able to perform when executed by an unprivilaged user?
Would it be bad to read the mock config while as root?
Looking at the current code (1.1.33) it would appear lines 438-445 (reading user specific config) are unnecessary. Privileges would already be dropped if running as root per line 402-403. If mock is running as a user, is there any benefit to dropping the privileges again, would that even work?
(In reply to Matthew Gyurgyik from comment #14) > Created attachment 805517 [details] > patch to set the group definde in chrootgid > > The patch that I attached should fix the issue with minimal code change. It > makes use of the dropPrivsTemp function provided by mockbuild/uid.py. Once > the config is parsed if chrootgid != mockgid, we'll restorePrivs(), set the > group, and then drop privilages again. This looks promising (more so than my previous attempts). > > However, it looks like there is some legacy code in main(), it might make > sense to clean some of it up, but I don't fully grasp all the ways mock can > be invoked. > > Is there any function besides --help that mock should be able to perform > when executed by an unprivilaged user? > > Would it be bad to read the mock config while as root? Yes, reading the configs is actually executing arbitrary python code. I'd prefer not to ever do that as root. > > Looking at the current code (1.1.33) it would appear lines 438-445 (reading > user specific config) are unnecessary. Privileges would already be dropped > if running as root per line 402-403. If mock is running as a user, is there > any benefit to dropping the privileges again, would that even work? Yes, I applied your patch and modified it to remove the priv drop/raise around reading the user config file, since we're supposed to be running unprivileged at that point. I'll attach my modified patch for your review. Created attachment 815542 [details]
Modified version of Matthew Gyurgyik's patch
Slightly modified version of previous patch, removes priv manipulation around reading user-specified config.
Thanks for reviewing the patch. I agree with your change. I tested the patch on my mock instance and everything seems to have worked as expected. mock-1.1.34-1.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/mock-1.1.34-1.fc18 mock-1.1.34-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mock-1.1.34-1.el6 mock-1.1.34-1.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/mock-1.1.34-1.fc20 mock-1.1.34-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/mock-1.1.34-1.fc19 Package mock-1.1.34-1.fc20: * should fix your issue, * was pushed to the Fedora 20 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing mock-1.1.34-1.fc20' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2013-20329/mock-1.1.34-1.fc20 then log in and leave karma (feedback). mock-1.1.35-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/mock-1.1.35-1.fc19 mock-1.1.35-1.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/mock-1.1.35-1.fc18 mock-1.1.35-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mock-1.1.35-1.el6 mock-1.1.35-1.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/mock-1.1.35-1.fc20 Works now, thank you mock-1.1.35-1.fc20 has been pushed to the Fedora 20 stable repository. If problems still persist, please make note of it in this bug report. mock-1.1.36-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/mock-1.1.36-1.fc19 mock-1.1.36-1.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/mock-1.1.36-1.fc20 mock-1.1.36-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mock-1.1.36-1.el6 mock-1.1.36-1.fc20 has been pushed to the Fedora 20 stable repository. If problems still persist, please make note of it in this bug report. mock-1.1.37-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/mock-1.1.37-1.fc19 mock-1.1.37-1.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/mock-1.1.37-1.fc20 mock-1.1.37-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mock-1.1.37-1.el6 mock-1.1.37-2.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/mock-1.1.37-2.fc20 mock-1.1.37-2.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/mock-1.1.37-2.fc19 mock-1.1.37-2.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mock-1.1.37-2.el6 mock-1.1.38-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/mock-1.1.38-1.fc19 mock-1.1.38-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mock-1.1.38-1.el6 mock-1.1.38-1.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/mock-1.1.38-1.fc20 mock-1.1.38-1.fc20 has been pushed to the Fedora 20 stable repository. If problems still persist, please make note of it in this bug report. mock-1.1.38-1.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report. mock-1.1.38-1.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report. |
Description of problem: I was trying to change the group against which mock authenticates, so I've set config_opts['chrootgid'] = grp.getgrnam("foo")[2] I've also set the permissions on `basedir` and `cache_topdir` from setgid 'mock' to setgid 'foo' and modified PAM accordingly to the comment in /etc/mock/site-defaults.cfg. Although my user is a member of the foo group, executing any mock command gives me the following exception: [mtoman@localhost ~]$ mock init ERROR: Must be member of 'foo' group to run mock! (mock, mtoman) Traceback (most recent call last): File "/usr/sbin/mock", line 928, in <module> main(retParams) File "/usr/sbin/mock", line 673, in main groupcheck(unprivGid, config_opts['chrootgid']) File "/usr/sbin/mock", line 604, in groupcheck (name, ", ".join(members))) RuntimeError: Must be member of 'foo' group to run mock! (mock, mtoman) Version-Release number of selected component (if applicable): mock-1.1.30-1.el6.noarch mock-1.1.30-1.fc18.noarch How reproducible: Always Steps to Reproduce: 1. Set config_opts['chrootgid'] to anything else than mock 2. Set the derived permissions accordingly 3. Run mock Actual results: Authentication fails and message says that my user needs to be a member of 'foo' group even though he is. Expected results: Authentication succeeds and mock runs Additional info: Adding a debug message to groupcheck function in /usr/sbin/mock gives me os.getgroups() = [492] while 492 is the gid of 'mock' group.