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

Closes-Bug: #1628360
Change-Id: I5d66fccd9ffb07df0c2e4435ec3da767b3b61117

oslo_privsep/daemon.py

@@ -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)
 

oslo_privsep/tests/test_daemon.py

@@ -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)

releasenotes/notes/setgid-should-be-called-before-setuid-fcf01083df9d5369.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    Fixed the failing setgid call when overriding both uid and gid to non root