Bug 953519 - After changing config_opts['chrootgid'], the process still runs with mock group privileges
Summary: After changing config_opts['chrootgid'], the process still runs with mock gro...
Status: CLOSED ERRATA
Alias: None
Product: Fedora EPEL
Classification: Fedora
Component: mock
Version: el6
Hardware: Unspecified
OS: Unspecified
urgent
urgent
Target Milestone: ---
Assignee: Clark Williams
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords: Reopened
Depends On:
Blocks: 948138
TreeView+ depends on / blocked
 
Reported: 2013-04-18 11:10 UTC by Michal Toman
Modified: 2015-03-23 00:41 UTC (History)
8 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2013-11-10 06:37:04 UTC


Attachments (Terms of Use)
patch to set the group definde in chrootgid (1.78 KB, patch)
2013-09-30 20:11 UTC, Matthew Gyurgyik
no flags Details | Diff
Modified version of Matthew Gyurgyik's patch (2.11 KB, patch)
2013-10-23 20:41 UTC, Clark Williams
no flags Details | Diff

Description Michal Toman 2013-04-18 11:10:58 UTC
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.

Comment 1 Clark Williams 2013-04-30 19:03:48 UTC
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.

Comment 5 Clark Williams 2013-07-22 17:57:18 UTC
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.

Comment 8 Fedora Update System 2013-08-21 22:55:27 UTC
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

Comment 9 Fedora Update System 2013-08-21 22:55:52 UTC
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

Comment 10 Fedora Update System 2013-08-21 22:56:11 UTC
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

Comment 11 Fedora Update System 2013-08-22 05:48:52 UTC
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).

Comment 12 Fedora Update System 2013-08-24 22:25:51 UTC
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.

Comment 13 Michal Toman 2013-08-26 14:00:06 UTC
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.

Comment 14 Matthew Gyurgyik 2013-09-30 20:11:10 UTC
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?

Comment 15 Clark Williams 2013-10-23 20:32:28 UTC
(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.

Comment 16 Clark Williams 2013-10-23 20:41:25 UTC
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.

Comment 17 Matthew Gyurgyik 2013-10-25 12:51:58 UTC
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.

Comment 18 Fedora Update System 2013-10-30 14:58:16 UTC
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

Comment 19 Fedora Update System 2013-10-30 14:59:53 UTC
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

Comment 20 Fedora Update System 2013-10-30 15:02:24 UTC
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

Comment 21 Fedora Update System 2013-10-30 15:04:30 UTC
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

Comment 22 Fedora Update System 2013-10-30 17:12:25 UTC
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).

Comment 23 Fedora Update System 2013-11-05 05:32:41 UTC
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

Comment 24 Fedora Update System 2013-11-05 05:34:15 UTC
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

Comment 25 Fedora Update System 2013-11-05 05:35:37 UTC
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

Comment 26 Fedora Update System 2013-11-05 05:36:52 UTC
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

Comment 27 Michal Toman 2013-11-05 13:06:24 UTC
Works now, thank you

Comment 28 Fedora Update System 2013-11-10 06:37:04 UTC
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.

Comment 29 Fedora Update System 2014-02-06 02:09:40 UTC
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

Comment 30 Fedora Update System 2014-02-06 02:11:22 UTC
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

Comment 31 Fedora Update System 2014-02-06 02:12:47 UTC
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

Comment 32 Fedora Update System 2014-02-08 05:03:33 UTC
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.

Comment 33 Fedora Update System 2014-03-25 20:25:45 UTC
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

Comment 34 Fedora Update System 2014-03-25 20:28:07 UTC
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

Comment 35 Fedora Update System 2014-03-25 20:29:58 UTC
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

Comment 36 Fedora Update System 2014-03-27 17:48:34 UTC
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

Comment 37 Fedora Update System 2014-03-27 17:50:29 UTC
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

Comment 38 Fedora Update System 2014-03-27 17:52:23 UTC
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

Comment 39 Fedora Update System 2014-03-31 19:05:21 UTC
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

Comment 40 Fedora Update System 2014-03-31 19:07:32 UTC
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

Comment 41 Fedora Update System 2014-03-31 19:09:29 UTC
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

Comment 42 Fedora Update System 2014-04-09 13:19:41 UTC
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.

Comment 43 Fedora Update System 2014-04-18 15:38:29 UTC
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.

Comment 44 Fedora Update System 2014-04-19 09:20:46 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.