From ed125c0c1cd6168cbf529c94ef81173dedce2726 Mon Sep 17 00:00:00 2001 From: Daniel Alvarez Date: Thu, 26 Apr 2018 18:33:21 +0200 Subject: [PATCH] Make IpNetnsExecFilter more strict to detect aliases Currently, this filter only takes into account 'ip netns exec' as input but this command accepts different aliases like 'ip net e' or 'ip netn ex', etcetera. This is a security issue since bypassing this filter basically allows anyone to execute arbitary commands because IpFilter will get hit and there's not going to be any further checks against CommandFilters. Change-Id: I2f6e55de4e60f2d3a6166c2fefbc31e9afc6c26f Closes-Bug: 1765734 Co-Authored-By: Jakub Libosvar Signed-off-by: Daniel Alvarez --- oslo_rootwrap/filters.py | 11 ++++++++--- oslo_rootwrap/tests/test_rootwrap.py | 11 +++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/oslo_rootwrap/filters.py b/oslo_rootwrap/filters.py index 1950f02..c4c7abc 100644 --- a/oslo_rootwrap/filters.py +++ b/oslo_rootwrap/filters.py @@ -18,6 +18,10 @@ import re import shutil import sys +NETNS_VARS = ('net', 'netn', 'netns') +EXEC_VARS = ('e', 'ex', 'exe', 'exec') + + if sys.platform != 'win32': # NOTE(claudiub): pwd is a Linux-specific library, and currently there is # no Windows support for oslo.rootwrap. @@ -274,8 +278,8 @@ class IpFilter(CommandFilter): if userargs[0] == 'ip': # Avoid the 'netns exec' command here for a, b in zip(userargs[1:], userargs[2:]): - if a == 'netns': - return (b != 'exec') + if a in NETNS_VARS: + return b not in EXEC_VARS else: return True @@ -373,7 +377,8 @@ class IpNetnsExecFilter(ChainingFilter): if self.run_as != "root" or len(userargs) < 4: return False - return (userargs[:3] == ['ip', 'netns', 'exec']) + return (userargs[0] == 'ip' and userargs[1] in NETNS_VARS + and userargs[2] in EXEC_VARS) def exec_args(self, userargs): args = userargs[4:] diff --git a/oslo_rootwrap/tests/test_rootwrap.py b/oslo_rootwrap/tests/test_rootwrap.py index bca5cf9..ad923a5 100644 --- a/oslo_rootwrap/tests/test_rootwrap.py +++ b/oslo_rootwrap/tests/test_rootwrap.py @@ -328,6 +328,8 @@ class RootwrapTestCase(testtools.TestCase): self.assertFalse(f.match(['ip', 'netns', 'exec'])) self.assertFalse(f.match(['ip', '-s', 'netns', 'exec'])) self.assertFalse(f.match(['ip', '-l', '42', 'netns', 'exec'])) + self.assertFalse(f.match(['ip', 'net', 'exec', 'foo'])) + self.assertFalse(f.match(['ip', 'netns', 'e', 'foo'])) def _test_IpFilter_netns_helper(self, action): f = filters.IpFilter(self._ip, 'root') @@ -346,10 +348,19 @@ class RootwrapTestCase(testtools.TestCase): f = filters.IpNetnsExecFilter(self._ip, 'root') self.assertTrue( f.match(['ip', 'netns', 'exec', 'foo', 'ip', 'link', 'list'])) + self.assertTrue(f.match(['ip', 'net', 'exec', 'foo', 'bar'])) + self.assertTrue(f.match(['ip', 'netn', 'e', 'foo', 'bar'])) + self.assertTrue(f.match(['ip', 'net', 'e', 'foo', 'bar'])) + self.assertTrue(f.match(['ip', 'net', 'exe', 'foo', 'bar'])) def test_IpNetnsExecFilter_nomatch(self): f = filters.IpNetnsExecFilter(self._ip, 'root') self.assertFalse(f.match(['ip', 'link', 'list'])) + self.assertFalse(f.match(['ip', 'foo', 'bar', 'netns'])) + self.assertFalse(f.match(['ip', '-s', 'netns', 'exec'])) + self.assertFalse(f.match(['ip', '-l', '42', 'netns', 'exec'])) + self.assertFalse(f.match(['ip', 'netns exec', 'foo', 'bar', 'baz'])) + self.assertFalse(f.match([])) # verify that at least a NS is given self.assertFalse(f.match(['ip', 'netns', 'exec']))