openstack/oslo.rootwrap
Code Issues Proposed changes

Make IpNetnsExecFilter more strict to detect aliases

Browse Source 
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>
This commit is contained in:
Daniel Alvarez 2018-04-26 18:33:21 +02:00 committed by Jakub Libosvar
parent 5a37cb77b4
commit ed125c0c1c
2 changed files with 19 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
oslo_rootwrap/filters.py
View File

@ -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:]

11
oslo_rootwrap/tests/test_rootwrap.py
View File

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