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 <jlibosva@redhat.com>
Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>

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']))