from ${URL}: bareos-dir, bareos-fd, and bareos-sd in bareos-core in Bareos 16.2.6 and earlier create a PID file after dropping privileges to a non-root account, which might allow local users to kill arbitrary processes by leveraging access to this non-root account for PID file modification before a root script executes a "kill `cat /pathname`" command. @maintainer(s), after bump, please call for stabilization if needed, thank you. @mjo, thanks for reporting. Daj Uan (jmbailey) Gentoo Security Padawan
Upstream has acknowledged this but there's no fix available yet, and I couldn't figure out a workaround. Best to just sit on the upstream report for a while...
@security, excuse the nose...setting severity correctly.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=0f65fc8909308003cdfe8d933b06b5a054c7715e commit 0f65fc8909308003cdfe8d933b06b5a054c7715e Author: David Seifert <soap@gentoo.org> AuthorDate: 2021-08-05 08:21:27 +0000 Commit: David Seifert <soap@gentoo.org> CommitDate: 2021-08-05 08:21:27 +0000 profiles: last-rite app-backup/bareos Bug: https://bugs.gentoo.org/631598 Bug: https://bugs.gentoo.org/690024 Bug: https://bugs.gentoo.org/735960 Bug: https://bugs.gentoo.org/749038 Bug: https://bugs.gentoo.org/761415 Bug: https://bugs.gentoo.org/761667 Bug: https://bugs.gentoo.org/778557 Bug: https://bugs.gentoo.org/786789 Bug: https://bugs.gentoo.org/799179 Signed-off-by: David Seifert <soap@gentoo.org> profiles/package.mask | 8 ++++++++ 1 file changed, 8 insertions(+)
This bug has been fixed a long time ago in a version that is not in the tree anymore. PID-Files are now being created as root: server ~ # ls -l /run/bareos/ insgesamt 12 -rw-r----- 1 root bareos 5 24. Jul 13:01 bareos-dir.9101.pid -rw-r----- 1 root bareos 6 20. Aug 14:13 bareos-fd.9102.pid -rw-r----- 1 root bareos 5 24. Jul 13:01 bareos-sd.9103.pid @securit team: please close!
(In reply to Marc Schiffbauer from comment #4) > @securit team: please close! Upstream hasn't closed the issue; I think we should assume we're vulnerable (in some configuration or another) if the issue is still open. Maybe ping them so we officially know a fixed version?
(In reply to John Helmert III from comment #5) > (In reply to Marc Schiffbauer from comment #4) > > @securit team: please close! > > Upstream hasn't closed the issue; I think we should assume we're vulnerable > (in some configuration or another) if the issue is still open. Maybe ping > them so we officially know a fixed version? I pinged them today. But as you can see in my paste above, all PID-Files are being created as root in all versions that are in the tree, so this issue does not exist anymore. There is no configuration where you can change the behavior how PID-files are being created by the daemons.
Newest version available (20.0.2) has the issue (again?). So yes, indeed: Lets wait for upstream to confirm.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=cee394d24645d97a6904df90fd0ab960de4367ef commit cee394d24645d97a6904df90fd0ab960de4367ef Author: Marc Schiffbauer <mschiff@gentoo.org> AuthorDate: 2021-09-06 11:58:41 +0000 Commit: Marc Schiffbauer <mschiff@gentoo.org> CommitDate: 2021-09-06 11:59:24 +0000 app-backup/bareos: add workaround for #631598 Bug: https://bugs.gentoo.org/631598 Package-Manager: Portage-3.0.20, Repoman-3.0.3 Signed-off-by: Marc Schiffbauer <mschiff@gentoo.org> ...{bareos-18.2.10-r1.ebuild => bareos-18.2.10-r2.ebuild} | 0 ...{bareos-19.2.10-r1.ebuild => bareos-19.2.10-r2.ebuild} | 0 .../{bareos-20.0.2-r1.ebuild => bareos-20.0.2-r2.ebuild} | 0 app-backup/bareos/files/bareos-dir.initd | 15 ++++++++++++--- app-backup/bareos/files/bareos-sd.initd | 15 ++++++++++++--- 5 files changed, 24 insertions(+), 6 deletions(-)
Upstream says this is fixed for bareos-21, which is in tree. The fix they've made makes pidfiles optional, controlled entirely by the command line. I'm not sure if openrc is able to handle daemons that don't make pidfiles, making that fix unsuitable for us. So.. ping Marc.
(In reply to John Helmert III from comment #9) > Upstream says this is fixed for bareos-21, which is in tree. The fix they've > made makes pidfiles optional, controlled entirely by the command line. I'm > not sure if openrc is able to handle daemons that don't make pidfiles, > making that fix unsuitable for us. > > So.. ping Marc. (start-stop-daemon is able to create its own PID file if the daemon cannot with --make-pidfile) I think what's relevant for us is that the bareos daemons no longer create the PID files after dropping privileges. That's what made the daemon-created PID file hard to use safely. The *-21.initd files look safe to me, since the privileges on /run/bareos have dropped from 770 to 750, and the PID files themselves are now root-owned. Specifically, the bareos user/group can't write to them any more. The initd scripts might be simplified a bit through the use of pidfile and command_args, but the security issue is I think resolved. Further reading in this regard: https://github.com/OpenRC/openrc/blob/master/service-script-guide.md#dont-write-your-own-startstop-functions
I guess it's time to cleanup then.
I've tried to cleanup but something looks suspicious: FILESDIR contains both bareos-dir.service and bareos-dir-21.service but all ebuilds are using the former. Perhaps 21* was supposed to use bareos-dir-21.service?
(In reply to Michał Górny from comment #12) > I've tried to cleanup but something looks suspicious: > > FILESDIR contains both bareos-dir.service and bareos-dir-21.service but all > ebuilds are using the former. Perhaps 21* was supposed to use > bareos-dir-21.service? I guess this is a bug, but I don't think it should block cleanup
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=18ec096ffdca3b05aef98bed01308a55044efa90 commit 18ec096ffdca3b05aef98bed01308a55044efa90 Author: John Helmert III <ajak@gentoo.org> AuthorDate: 2022-11-24 15:04:00 +0000 Commit: John Helmert III <ajak@gentoo.org> CommitDate: 2022-11-24 15:04:00 +0000 app-backup/bareos: drop 19.2.12, 20.0.6 Bug: https://bugs.gentoo.org/631598 Signed-off-by: John Helmert III <ajak@gentoo.org> app-backup/bareos/Manifest | 2 - app-backup/bareos/bareos-19.2.12.ebuild | 382 ------------------------------- app-backup/bareos/bareos-20.0.6.ebuild | 394 -------------------------------- app-backup/bareos/metadata.xml | 1 - 4 files changed, 779 deletions(-)
We had workarounds in place for all supported versions, so not need to drop the 19 and 20 branch ebuilds as they were not affected. And as I like to support all versions in Gentoo that are being supported upstream I would like to revert the final cleanup as all affected versions have already been cleaned up before.
(In reply to Michał Górny from comment #12) > I've tried to cleanup but something looks suspicious: > > FILESDIR contains both bareos-dir.service and bareos-dir-21.service but all > ebuilds are using the former. Perhaps 21* was supposed to use > bareos-dir-21.service? Thanks for spotting, yes this is a bug and I will fix it now
(In reply to Marc Schiffbauer from comment #15) > We had workarounds in place for all supported versions, so not need to drop > the 19 and 20 branch ebuilds as they were not affected. > > And as I like to support all versions in Gentoo that are being supported > upstream I would like to revert the final cleanup as all affected versions > have already been cleaned up before. mjo, could you share your perspective on whether these were effectively fixed in <21?
(In reply to John Helmert III from comment #17) > (In reply to Marc Schiffbauer from comment #15) > > We had workarounds in place for all supported versions, so not need to drop > > the 19 and 20 branch ebuilds as they were not affected. > > > > And as I like to support all versions in Gentoo that are being supported > > upstream I would like to revert the final cleanup as all affected versions > > have already been cleaned up before. > > mjo, could you share your perspective on whether these were effectively > fixed in <21? Yes, please see comments 6 and 4 above. After this issue has been discovered I changed all the init and unit files to not rely on the pid-file functionaility of the daemons anymore. systemd works fine without any pid files and OpenRC stuff has been changed to use start-stop-daemons pid-file functionality instead. These are the relevant commits: commit f9aa2491329e3b770bfa24a8139c8fde245f1867 Author: Marc Schiffbauer <mschiff@gentoo.org> Date: Sun Feb 6 21:23:25 2022 -1000 app-backup/bareos: fix pid file creation Closes: https://bugs.gentoo.org/832805 Signed-off-by: Marc Schiffbauer <mschiff@gentoo.org> commit 98caaa05e0a1ec72de16ada0e6c2c0a9f9307ed9 Author: Marc Schiffbauer <mschiff@gentoo.org> Date: Wed Sep 8 13:18:52 2021 -1000 app-backup/bareos: systemd workaround for #631598 When using systemd. sd and dir services should not depend on the PID files, so we use bareos-sd and bareos-dir as foreground services for which systemd does not need PID files Signed-off-by: Marc Schiffbauer <mschiff@gentoo.org>
(In reply to Marc Schiffbauer from comment #18) > > These are the relevant commits: > > commit f9aa2491329e3b770bfa24a8139c8fde245f1867 > Author: Marc Schiffbauer <mschiff@gentoo.org> > Date: Sun Feb 6 21:23:25 2022 -1000 > > app-backup/bareos: fix pid file creation > This did fix the issue, but only in the *-21 scripts if I'm reading correctly. The 19.x and 20.x ebuilds that ajak dropped in 18ec096f were still referring to the old (non-21) versions. I think the old (non-21) scripts are still vulnerable, because they run e.g. checkpath -d -m 0770 -o root:bareos /run/bareos ... chown root:bareos /run/bareos/bareos-dir.9101.pid which still lets the bareos user (or anyone else in the bareos group) control the PID file because they can write to the parent directory. (If root owns a file in a directory that I can write to, I can delete his file and replace it with my own.) The *-21 scripts, on the other hand, look safe.
(In reply to Michael Orlitzky from comment #19) > (In reply to Marc Schiffbauer from comment #18) > > > > These are the relevant commits: > > > > commit f9aa2491329e3b770bfa24a8139c8fde245f1867 > > Author: Marc Schiffbauer <mschiff@gentoo.org> > > Date: Sun Feb 6 21:23:25 2022 -1000 > > > > app-backup/bareos: fix pid file creation > > > > This did fix the issue, but only in the *-21 scripts if I'm reading > correctly. The 19.x and 20.x ebuilds that ajak dropped in 18ec096f were > still referring to the old (non-21) versions. > > I think the old (non-21) scripts are still vulnerable, because they run e.g. > > checkpath -d -m 0770 -o root:bareos /run/bareos > ... > chown root:bareos /run/bareos/bareos-dir.9101.pid > > which still lets the bareos user (or anyone else in the bareos group) > control the PID file because they can write to the parent directory. (If > root owns a file in a directory that I can write to, I can delete his file > and replace it with my own.) > > The *-21 scripts, on the other hand, look safe. Yes, this also needs to be checkpath -d -m 0750, like in the 21 scripts. Then version 19 and 20 are safe as well. So please lets restore them, change it to 0750 and we are done.
Let's wait for those to actually be fixed before closing this bug then.
(In reply to Marc Schiffbauer from comment #20) > > Yes, this also needs to be checkpath -d -m 0750, like in the 21 scripts. > Then version 19 and 20 are safe as well. > > So please lets restore them, change it to 0750 and we are done. IIRC the trouble was that, without mode g+w, the daemon (which is running as bareos:bareos at that point) is unable to create the PID file. It should be relatively safe to use checkpath to fix the permissions after the PID file has been created, though, so long as you run checkpath (instead of chown) on both /run/bareos and the PID file. There's still a tiny race condition that way, but given the low severity of the issue and the fact that v21 will fix it completely, I don't think it's worth beating to death.
(In reply to Michael Orlitzky from comment #22) > (In reply to Marc Schiffbauer from comment #20) > > > > Yes, this also needs to be checkpath -d -m 0750, like in the 21 scripts. > > Then version 19 and 20 are safe as well. > > > > So please lets restore them, change it to 0750 and we are done. > > IIRC the trouble was that, without mode g+w, the daemon (which is running as > bareos:bareos at that point) is unable to create the PID file. > > It should be relatively safe to use checkpath to fix the permissions after > the PID file has been created, though, so long as you run checkpath (instead > of chown) on both /run/bareos and the PID file. > > There's still a tiny race condition that way, but given the low severity of > the issue and the fact that v21 will fix it completely, I don't think it's > worth beating to death. Yes, you are absolutely right. I was just about to write something similar. I also now remembered that this was the problem, that you cannot tell the damoens to not write a pidfile. Instead of changing the owner of the pid dir I would set the sticky bit, so that the bareos user cannot delete root owned files anymore. That way the the daemons (sd and dir) could still initially write their pidfile here. Otherwise one or the other would fail to start because they share the same pid directory. (The fd daemon ins not affected as it runs as root anyway) What do you think?
(In reply to Marc Schiffbauer from comment #23) > > Instead of changing the owner of the pid dir I would set the sticky bit, so > that the bareos user cannot delete root owned files anymore. > > ... > > share the same pid directory. > > What do you think? You mean instead of changing the mode, right? I think it should work, if the daemon refuses to start upon encountering an existing PID file. Basically all I'd worry about is that the bareos user could create fake PID files while the daemons are stopped. If the start() function ignores a pre-existing PID file and pretends everything is OK, then a subsequent stop() might still try to kill the wrong PID.
Update: I re-added a bareos-20 ebuild (20.0.8) with improved openrc init scripts as suggested by Michael. bareos-19 had other issues with current python versions so I decided not to re-add that version again. @Michael: Maybe you want to review again. I think this bug can be finally closed then.
(In reply to Marc Schiffbauer from comment #25) > Update: > > I re-added a bareos-20 ebuild (20.0.8) with improved openrc init scripts as > suggested by Michael. > > bareos-19 had other issues with current python versions so I decided not to > re-add that version again. > > @Michael: Maybe you want to review again. I think this bug can be finally > closed then. I think that's as good as 20.x is going to get, and I'm happy with calling it fixed and moving on to worry about other things. Thanks.
