The init script for gimps calls "chown -R" on /var/lib/gimps every time you start it: checkconfig() { ... /bin/chown -R ${USER}:${GROUP} ${GIMPS_DIR} The GIMPS user can place a hard link in that directory to any path he chooses (on the same filesystem), and trick root into giving ownership of that file to him. For example, on my machine, GIMPS is running as nobody:nobody. If I replace /var/lib/gimps/local.txt with a hardlink to /var/lib/portage/world and then start gimps, the "nobody" user gains control of it: $ sudo su nobody -c 'ln /var/lib/portage/world /var/lib/gimps/local.txt' $ sudo /etc/init.d/gimps start * Starting GIMPS ... $ ls /var/lib/portage/world -rw-r--r-- 2 nobody nobody 3.7K 2016-12-21 16:10 /var/lib/portage/world Our gentoo-sources have a patch that prevents this, but vanilla-sources do not.
I'm adding Paolo Pedroni as I'm sort of a proxy maintainer here. Why is that chown in the init script in the first place? Can't we fix the permissions once in the ebuild?
I'm also going into devaway for a week in 5 minutes. Security, please feel free to implement any proposed solution.
(In reply to Thomas Kahle from comment #1) > I'm adding Paolo Pedroni as I'm sort of a proxy maintainer here. > Why is that chown in the init script in the first place? Can't we fix the > permissions once in the ebuild? Beats me. As far as I know it's always been there. I've always thought it was there to allow the user under which we choose to run the program to view and edit files in the folder. If there's a better way, please go ahead and fix it.
Adding bug 540006 to see-also as it is referenced in report re attack mitigation (It was here the protect in gentoo-sources was added) as well as changes done to checkpath to mitigate this. As such one alternative is likely to switch to using checkpath in the init script for this package as well.
(In reply to Kristian Fiskerstrand from comment #4) > Adding bug 540006 to see-also as it is referenced in report re attack > mitigation (It was here the protect in gentoo-sources was added) as well as > changes done to checkpath to mitigate this. As such one alternative is > likely to switch to using checkpath in the init script for this package as > well. So something like: --- gimps.old 2016-12-22 11:40:26.132295240 +0100 +++ gimps.new 2016-12-22 11:42:27.402909801 +0100 @@ -12,7 +12,7 @@ /bin/mkdir "${GIMPS_DIR}" fi - /bin/chown -R ${USER}:${GROUP} ${GIMPS_DIR} + checkpath -d -o ${USER}:${GROUP} ${GIMPS_DIR} if [ ! -e "${GIMPS_DIR}/local.txt" ]; then eerror "GIMPS has not been configured. Please configure it manually before" should work, shouldn't it? I can't test it, though, because I use systemd.
Or even better, to check files in the directory as well: --- gimps.old 2016-12-22 11:40:26.132295240 +0100 +++ gimps.new 2016-12-22 11:42:27.402909801 +0100 @@ -12,7 +12,7 @@ /bin/mkdir "${GIMPS_DIR}" fi - /bin/chown -R ${USER}:${GROUP} ${GIMPS_DIR} + checkpath -d -o ${USER}:${GROUP} ${GIMPS_DIR} + checkpath -f -o ${USER}:${GROUP} ${GIMPS_DIR}/* if [ ! -e "${GIMPS_DIR}/local.txt" ]; then eerror "GIMPS has not been configured. Please configure it manually before" if checkpath allows wildcards...
Checkpath will create the directory if it doesn't exist, so once you've added the checkpath -d -o ${USER}:${GROUP} ${GIMPS_DIR} line, this becomes redundant: if [ ! -e "${GIMPS_DIR}" ]; then einfo "Creating ${GIMPS_DIR}" /bin/mkdir "${GIMPS_DIR}" fi It looks like, * Checkpath will accept wildcards, and * Checkpath won't operate on a hardlink. So while this line, checkpath -f -o ${USER}:${GROUP} ${GIMPS_DIR}/* is probably safe, I have to wonder if it's necessary. Once the GIMPS user/group owns $GIMPS_DIR, the only people who can create files in it are root and the GIMPS user. If root creates a file in there, then you don't want to change its ownership, and if the GIMPS user does, it will already have the correct ownership. I think we could get away with the single call to "checkpath -d"... now all we need is someone who actually runs GIMPS on OpenRC to try it =)
(In reply to Michael Orlitzky from comment #7) > > So while this line, > > checkpath -f -o ${USER}:${GROUP} ${GIMPS_DIR}/* > > is probably safe, Since that wildcard will catch directories, too, the init script may spit out some errors complaining that you're calling "checkpath -f" on a directory. It will also happily create a file named "*" when the directory is empty...
(In reply to Michael Orlitzky from comment #8) > (In reply to Michael Orlitzky from comment #7) > > > > So while this line, > > > > checkpath -f -o ${USER}:${GROUP} ${GIMPS_DIR}/* > > > > is probably safe, > > Since that wildcard will catch directories, too, the init script may spit > out some errors complaining that you're calling "checkpath -f" on a > directory. It will also happily create a file named "*" when the directory > is empty... Ok, scratch that, then. Since tomka is apparently on devaway, will you accept a patch (or a github PR) with all other proposed changes from me?
Yeah, sure. I run OpenRC, so if you can tell me the usual steps that you'd take to configure GIMPS, I can try it out.
Created attachment 457286 [details, diff] 0001-sci-mathematics-gimps-Fix-potential-privilege-escala.patch This is a patch with the intended fix. To check: 1) emerge the modified package (doh!) 2) Before starting the service run (so it will create some files in /var/lib/gimps, among which /var/lib/gimps/local.txt, whose existance is checked in the init script): # /opt/gimps/mprime -m -w/var/lib/gimps 3) Answer 'N' to the "Join Gimps?" question 4) Press enter a few times to accept default values until the stress test starts 5) Stop the stress test with ^C 6) Exit the client, pressing '5' 7) The files in /var/lib/gimps now have the uid and gid of the user under which you run the stress test. Chown them to nobody:nobody or change /etc/conf.d/gimps accordingly (see why the init script happily 'chown'ed the folder and all its contents?) 8) Try to run the init script
(In reply to Paolo Pedroni from comment #11) > Created attachment 457286 [details, diff] [details, diff] > 0001-sci-mathematics-gimps-Fix-potential-privilege-escala.patch > > This is a patch with the intended fix. > Thanks! Can you also do a revision bump of the 28.10 version? The way this usually goes is that the security team will fast-track the stabilization of your new (fixed) revision, and then we can remove v28.9 from the tree. The new stable revision will force everyone to upgrade, ensuring that they pull down the fixed init script. (We're not supposed to modify stable ebuilds, but it's kind of a moot point if we're going to get rid of it ASAP.)
Created attachment 457290 [details, diff] 0002-sci-mathematics-gimps-revbump-for-new-version.patch Revbump for new version.
Thanks again, I just pushed those two commits so that we can get the stabilization process rolling. I don't like adding another step (the chown) to the manual configuration, but some of it seems unavoidable. Am I correct that users have to run mprime at least once to generate their local.txt? If so, it might make sense to document all of that in a special gentoo README file (a la readme.gentoo-r1.eclass) or on the wiki (which could be mentioned in an einfo message). Here's my totally untested idea. 1. Always run the daemon as a new user, "gimps". That's safer than reusing "nobody", and changing $USER/$GROUP in the conf.d file doesn't work anyway, because the file ownership is wrong after you do it. 2. Create /var/lib/gimps as gimps:gimps in the ebuild. 3. After installation, instruct users what to do, if they want to run the daemon: 3a. Give the "gimps" user a shell temporarily. 3b. Run "/opt/gimps/mprime -m -w/var/lib/gimps" once **as the gimps user** to generate local.txt, and use Ctrl-C to kill it. 3c. Make /sbin/nologin the shell for "gimps" again, and lock the account with "passwd -l". 4. You can remove the call to checkpath in the init script, and use "gimps" instead of $USER/$GROUP. I guess $GIMPS_DIR should be fixed at /var/lib/gimps, too. That way you never have to run mprime as root, and root never tries to change ownership or permissions on anything. Users still have to do some manual steps, but everything except the shell juggling and account locking is already necessary.
(In reply to Michael Orlitzky from comment #14) > Thanks again, I just pushed those two commits so that we can get the > stabilization process rolling. Sorry for the late reply, but I was on holiday with nearly no internet access. > > I don't like adding another step (the chown) to the manual configuration, > but some of it seems unavoidable. Am I correct that users have to run mprime > at least once to generate their local.txt? Either that, or importing an old local.txt from another installation. > > If so, it might make sense to document all of that in a special gentoo > README file (a la readme.gentoo-r1.eclass) or on the wiki (which could be > mentioned in an einfo message). I will work on it (if tomka does not beat me to it). > > Here's my totally untested idea. > > 1. Always run the daemon as a new user, "gimps". That's safer than reusing > "nobody", and changing $USER/$GROUP in the conf.d file doesn't work anyway, > because the file ownership is wrong after you do it. > > 2. Create /var/lib/gimps as gimps:gimps in the ebuild. > > 3. After installation, instruct users what to do, if they want to run the > daemon: > > 3a. Give the "gimps" user a shell temporarily. > > 3b. Run "/opt/gimps/mprime -m -w/var/lib/gimps" once **as the gimps user** > to generate local.txt, and use Ctrl-C to kill it. > > 3c. Make /sbin/nologin the shell for "gimps" again, and lock the > account with "passwd -l". > > 4. You can remove the call to checkpath in the init script, and use "gimps" > instead of $USER/$GROUP. I guess $GIMPS_DIR should be fixed at > /var/lib/gimps, too. I'd rather not remove the option of changing user:group for gimps, there are use cases for which it may be necessary. I especially don't like to remove the option of changing $GIMPS_DIR: dual boot users need to change that to put gimps file in a folder readable by Windows. Of course the final decision lies with tomka as the official mantainer.
(In reply to Paolo Pedroni from comment #15) > I will work on it (if tomka does not beat me to it). I'm in the process of retiring as a dev. gimps should be maintained by the sci-mathematics project in the future. Good luck.
Created attachment 458730 [details, diff] 0003-sci-mathematics-gimps-Improve-documentation-for-firs.patch (In reply to Michael Orlitzky from comment #14) > If so, it might make sense to document all of that in a special gentoo > README file (a la readme.gentoo-r1.eclass) or on the wiki (which could be > mentioned in an einfo message). This patch adds a README.gentoo file with instructions to first-time users and links to further upstream documentation (without revbump, since we're still unstable).
(In reply to Thomas Kahle from comment #16) > (In reply to Paolo Pedroni from comment #15) > > I will work on it (if tomka does not beat me to it). > > I'm in the process of retiring as a dev. > > gimps should be maintained by the sci-mathematics project in the future. > Good luck. :-((( It's been nice working with you, tomka. Good luck with your future, then. If the sci-mathematics project agrees I am available to be a proxy-mantainer for this package.
@Paolo: I've added you as a proxy maintainer in metadata.xml, I'm sure the sci-math project will appreciate the help. I also merged the latest patch with two minor fixes: I killed some trailing whitespace, and added an "|| die" to a "cp" call in the ebuild. @Security: This is done, we can proceed with stabilization on amd64 and x86, target =sci-mathematics/gimps-28.10-r1.
(In reply to Michael Orlitzky from comment #19) > @Paolo: I've added you as a proxy maintainer in metadata.xml, I'm sure the > sci-math project will appreciate the help. I also merged the latest patch > with two minor fixes: I killed some trailing whitespace, and added an "|| > die" to a "cp" call in the ebuild. Thanks. > @Security: This is done, we can proceed with stabilization on amd64 and x86, > target =sci-mathematics/gimps-28.10-r1. =sci-mathematics/gimps-28.10-r1 is stable on all archs (see bug #612926). Old versions should be removed. Can someone in sci-mathematics@g.o do it, please? Or should I push a PR on github?
(In reply to Paolo Pedroni from comment #20) > > Old versions should be removed. Can someone in sci-mathematics@g.o do it, > please? Or should I push a PR on github? I took care of it, thanks again.
(In reply to Michael Orlitzky from comment #21) > I took care of it, thanks again. Thanks to you. Security can close this, then?
Security team: the vulnerable versions of this were removed back in April, what's next?
(In reply to Michael Orlitzky from comment #23) > Security team: the vulnerable versions of this were removed back in April, > what's next? As issue is fixed in stable I'm removing the group restriction for this issue. We need to request a CVE and produce a GLSA containing said CVE. @maintainer: In which version was the affected init script added. I take it this is a Gentoo-specific issue and the init script is not included upstream?
(In reply to Kristian Fiskerstrand from comment #24) > @maintainer: In which version was the affected init script added. I take it > this is a Gentoo-specific issue and the init script is not included upstream? The init script was added before I was the mantainer. You are correct this is a Gentoo-specific issue.
New GLSA Request filed.
CVE requested. I'll update the alias as soon as it is confirmed. Gentoo Security Padawan ChrisADR
CVE-2017-14484 assigned. @Security please add it to the database. Gentoo Security Padawan ChrisADR
This issue was resolved and addressed in GLSA 201709-11 at https://security.gentoo.org/glsa/201709-11 by GLSA coordinator Aaron Bauman (b-man).
