From 9f2b40f2cecc116906e77d69797c0c6877bd5b4d Mon Sep 17 00:00:00 2001 From: aojeagarcia Date: Wed, 20 Jun 2018 18:53:36 +0200 Subject: [PATCH] Dropping radvd process privileges radvd needs to run as root, but has the capability to drop privileges on linux hosts. Currently, radvd process is not using this feature and this can be considered a serious risk. In addition, some distributions like SUSE, radvd process runs as a non privileged user by default, causing radvd failure to daemonize because it can't write the pid in the corresponding neutron folder and break the IPv6 functionality. This patch allows radvd process to run with the same user used by neutron. In order to allow this, it changes the radvd config file permissions to 444 because radvd doesn't allow that this file can be writeable by self/group. The readonly mode is not a problem updating the file because of the way the neutron_lib replace_file function handles the files operations. Closes-Bug: #1777922 Change-Id: Ic5d976ba71a966a537d1f31888f82997a7ccb0de Signed-off-by: aojeagarcia --- neutron/agent/linux/ra.py | 9 ++++++++- neutron/tests/unit/agent/l3/test_agent.py | 6 +++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/neutron/agent/linux/ra.py b/neutron/agent/linux/ra.py index 9443ed2febd..0231413cd8c 100644 --- a/neutron/agent/linux/ra.py +++ b/neutron/agent/linux/ra.py @@ -13,6 +13,9 @@ # License for the specific language governing permissions and limitations # under the License. +import os +import pwd + from itertools import chain as iter_chain import jinja2 @@ -125,7 +128,8 @@ class DaemonMonitor(object): contents = buf.getvalue() LOG.debug("radvd config = %s", contents) - file_utils.replace_file(radvd_conf, contents) + # radvd conf file can't be writeable by self/group + file_utils.replace_file(radvd_conf, contents, file_mode=0o444) return radvd_conf def _get_radvd_process_manager(self, callback=None): @@ -139,6 +143,8 @@ 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 # 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 @@ -147,6 +153,7 @@ class DaemonMonitor(object): radvd_cmd = [RADVD_SERVICE_CMD, '-C', '%s' % radvd_conf, '-p', '%s' % pid_file, + '-u', '%s' % radvd_user, '-m', 'syslog'] return radvd_cmd diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index d7f0a6009ce..cc3123fa37c 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -58,6 +58,7 @@ from neutron.conf.agent.l3 import ha as ha_conf from neutron.conf import common as base_config from neutron.tests import base from neutron.tests.common import l3_test_common +from neutron.tests.unit.agent.linux.test_utils import FakeUser _uuid = uuidutils.generate_uuid HOSTNAME = 'myhost' @@ -2791,7 +2792,9 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): self.assertFalse(ri.remove_floating_ip.called) - def test_spawn_radvd(self): + @mock.patch('os.geteuid', return_value='490') + @mock.patch('pwd.getpwuid', return_value=FakeUser('neutron')) + def test_spawn_radvd(self, geteuid, getpwuid): router = l3_test_common.prepare_router_data(ip_version=6) conffile = '/fake/radvd.conf' @@ -2829,6 +2832,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 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) def test_generate_radvd_mtu_conf(self):