setgid should be called before setuid

If you setuid to a non-zero value first(meaning you're no longer root),
then call setgroups, the effective uid of the process
is now no longer root, meaning that the internal setgid call fails

This also removes the duplicated if loop

Change-Id: I5d66fccd9ffb07df0c2e4435ec3da767b3b61117

Closes-Bug: #1628360
Change-Id: Iaf6f561cd6bc9f3eda9629d08845069d64f3dbb5
This commit is contained in:
Max Lamprecht 2023-02-13 10:45:19 +01:00
parent c2b6df05e0
commit 49e04d84c7
2 changed files with 8 additions and 3 deletions

View File

@ -414,13 +414,11 @@ class Daemon(object):
msg = _('Failed to remove supplemental groups')
LOG.critical(msg)
raise FailedToDropPrivileges(msg)
setgid(self.group)
if self.user is not None:
setuid(self.user)
if self.group is not None:
setgid(self.group)
finally:
capabilities.set_keepcaps(False)

View File

@ -166,6 +166,11 @@ class DaemonTest(base.BaseTestCase):
channel = mock.NonCallableMock()
context = get_fake_context()
manager = mock.Mock()
manager.attach_mock(mock_setuid, "setuid")
manager.attach_mock(mock_setgid, "setgid")
expected_calls = [mock.call.setgid(84), mock.call.setuid(42)]
d = daemon.Daemon(channel, context)
d._drop_privs()
@ -173,6 +178,8 @@ class DaemonTest(base.BaseTestCase):
mock_setgid.assert_called_once_with(84)
mock_setgroups.assert_called_once_with([])
assert manager.mock_calls == expected_calls
self.assertCountEqual(
[mock.call(True), mock.call(False)],
mock_keepcaps.mock_calls)