From 6ec7ba869573731cfa1c25dac3737526a9e34c8c Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 19 Sep 2019 17:12:59 +0000 Subject: [PATCH] Add radvd_user config option In some deployments, the "neutron" user does not have the permissions to modify the kernel interfaces. In those cases the radvd user should be defined. This patch introduces a new config option: "radvd_user". This config option is the username passed to radvd, used to drop root privileges and change user ID to username and group ID to the primary group of username. If no user specified (by default is an empty string), the user executing the L3 agent will be passed. If "root" specified, because radvd is spawned as root, no "username" parameter will be passed. Change-Id: Ie9a6fbf04d453a3c1c0bddf9ecaa3d4d6467e8ff Closes-Bug: #1844688 (cherry picked from commit 6a5a75d5a6d4af08310774cef1b091d2ce2551d4) --- neutron/agent/linux/ra.py | 11 ++++-- neutron/conf/agent/l3/config.py | 8 ++++ neutron/tests/unit/agent/l3/test_agent.py | 38 +++++++++++-------- ...d_user-config-option-24730a6d686fee18.yaml | 11 ++++++ 4 files changed, 50 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/radvd_user-config-option-24730a6d686fee18.yaml diff --git a/neutron/agent/linux/ra.py b/neutron/agent/linux/ra.py index 550cb450f4b..eb2166d001b 100644 --- a/neutron/agent/linux/ra.py +++ b/neutron/agent/linux/ra.py @@ -150,8 +150,12 @@ class DaemonMonitor(object): def _spawn_radvd(self, radvd_conf): def callback(pid_file): - # drop radvd daemon privileges and run as the neutron user - radvd_user = pwd.getpwuid(os.geteuid()).pw_name + if not self._agent_conf.radvd_user: + radvd_user = pwd.getpwuid(os.geteuid()).pw_name + elif self._agent_conf.radvd_user == 'root': + radvd_user = None + else: + radvd_user = self._agent_conf.radvd_user # we need to use -m syslog and f.e. not -m stderr (the default) # or -m stderr_syslog so that radvd 2.0+ will close stderr and # exit after daemonization; otherwise, the current thread will @@ -160,8 +164,9 @@ class DaemonMonitor(object): radvd_cmd = [RADVD_SERVICE_CMD, '-C', '%s' % radvd_conf, '-p', '%s' % pid_file, - '-u', '%s' % radvd_user, '-m', 'syslog'] + if radvd_user: + radvd_cmd += ['-u', '%s' % radvd_user] return radvd_cmd pm = self._get_radvd_process_manager(callback) diff --git a/neutron/conf/agent/l3/config.py b/neutron/conf/agent/l3/config.py index 8f52ff3a72e..f9cc09cd3fa 100644 --- a/neutron/conf/agent/l3/config.py +++ b/neutron/conf/agent/l3/config.py @@ -92,6 +92,14 @@ OPTS = [ help=_('Iptables mangle mark used to mark ingress from ' 'external network. This mark will be masked with ' '0xffff so that only the lower 16 bits will be used.')), + cfg.StrOpt('radvd_user', + default='', + help=_('The username passed to radvd, used to drop root ' + 'privileges and change user ID to username and group ID ' + 'to the primary group of username. If no user specified ' + '(by default), the user executing the L3 agent will be ' + 'passed. If "root" specified, because radvd is spawned ' + 'as root, no "username" parameter will be passed.')), ] diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index ed1a296c1dc..38b5219c215 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -17,6 +17,8 @@ import copy import functools from itertools import chain as iter_chain from itertools import combinations as iter_combinations +import os +import pwd import eventlet import fixtures @@ -3086,9 +3088,9 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): self.assertFalse(ri.remove_floating_ip.called) - @mock.patch('os.geteuid', return_value='490') - @mock.patch('pwd.getpwuid', return_value=FakeUser('neutron')) - def test_spawn_radvd(self, geteuid, getpwuid): + @mock.patch.object(os, 'geteuid', return_value=mock.ANY) + @mock.patch.object(pwd, 'getpwuid') + def test_spawn_radvd(self, mock_getpwuid, *args): router = l3_test_common.prepare_router_data( ip_version=lib_constants.IP_VERSION_6) @@ -3116,19 +3118,25 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): agent.process_monitor, l3_test_common.FakeDev, self.conf) - radvd.enable(router['_interfaces']) - cmd = execute.call_args[0][0] - - self.assertIn('radvd', cmd) - - _join = lambda *args: ' '.join(args) - - cmd = _join(*cmd) - self.assertIn(_join('-C', conffile), cmd) - self.assertIn(_join('-p', pidfile), cmd) - self.assertIn(_join('-u', 'neutron'), cmd) - self.assertIn(_join('-m', 'syslog'), cmd) + test_users = [('', 'stack', '-u stack'), + ('neutron', mock.ANY, '-u neutron'), + ('root', mock.ANY, None)] + for radvd_user, os_user, to_check in test_users: + self.conf.set_override('radvd_user', radvd_user) + mock_getpwuid.return_value = FakeUser(os_user) + radvd.enable(router['_interfaces']) + cmd = execute.call_args[0][0] + _join = lambda *args: ' '.join(args) + cmd = _join(*cmd) + self.assertIn('radvd', cmd) + self.assertIn(_join('-C', conffile), cmd) + self.assertIn(_join('-p', pidfile), cmd) + self.assertIn(_join('-m', 'syslog'), cmd) + if to_check: + self.assertIn(to_check, cmd) + else: + self.assertNotIn('-u', cmd) def test_generate_radvd_mtu_conf(self): router = l3_test_common.prepare_router_data() diff --git a/releasenotes/notes/radvd_user-config-option-24730a6d686fee18.yaml b/releasenotes/notes/radvd_user-config-option-24730a6d686fee18.yaml new file mode 100644 index 00000000000..f0d447bcccb --- /dev/null +++ b/releasenotes/notes/radvd_user-config-option-24730a6d686fee18.yaml @@ -0,0 +1,11 @@ +--- +other: + - | + A new config option, ``radvd_user``, was added to l3_agent.ini for the L3 + agent. This option defines the username passed to radvd, used to drop + "root" privileges and change user ID to username and group ID to the + primary group of the user. If no user specified (by default), the user + executing the L3 agent will be passed. If "root" specified, because radvd + is spawned as root, no "username" parameter will be passed. + (For more information see bug `1844688 + `_.)