Port nova-rootwrap changes to cinder-rootwrap

Port recent nova-rootwrap changes to cinder-rootwrap, including:
* Move filter definition from Python module to configuration files
* Fix tests execution on Fedora (bug 1027814)
* Remove executable bit on cinder/rootwrap files

This certainly needs a matching change to devstack to pass gating.

Change-Id: I963bc7890ba285ae515ea61bbd960bd2523f9061

bin/cinder-rootwrap

@@ -18,38 +18,49 @@
 
 """Root wrapper for Cinder
 
-   Uses modules in cinder.rootwrap containing filters for commands
-   that cinder is allowed to run as another user.
+   Filters which commands cinder is allowed to run as another user.
 
-   To switch to using this, you should:
-   * Set "--root_helper=sudo cinder-rootwrap" in cinder.conf
-   * Allow cinder to run cinder-rootwrap as root in cinder_sudoers:
-     cinder ALL = (root) NOPASSWD: /usr/bin/cinder-rootwrap
-     (all other commands can be removed from this file)
+   To use this, you should set the following in cinder.conf:
+   root_helper=sudo cinder-rootwrap /etc/cinder/rootwrap.conf
+
+   You also need to let the cinder user run cinder-rootwrap as root in sudoers:
+   cinder ALL = (root) NOPASSWD: /usr/bin/cinder-rootwrap
+                                 /etc/cinder/rootwrap.conf *
 
    To make allowed commands node-specific, your packaging should only
-   install cinder/rootwrap/{compute,network,volume}.py respectively on
-   compute, network and volume nodes (i.e. cinder-api nodes should not
+   install volume.filters on volume nodes (i.e. cinder-api nodes should not
    have any of those files installed).
 """
 
+import ConfigParser
 import os
 import subprocess
 import sys
 
 
 RC_UNAUTHORIZED = 99
 RC_NOCOMMAND = 98
+RC_BADCONFIG = 97
 
 if __name__ == '__main__':
     # Split arguments, require at least a command
     execname = sys.argv.pop(0)
-    if len(sys.argv) == 0:
+    if len(sys.argv) < 2:
         print "%s: %s" % (execname, "No command specified")
         sys.exit(RC_NOCOMMAND)
 
+    configfile = sys.argv.pop(0)
     userargs = sys.argv[:]
 
+    # Load configuration
+    config = ConfigParser.RawConfigParser()
+    config.read(configfile)
+    try:
+        filters_path = config.get("DEFAULT", "filters_path").split(",")
+    except ConfigParser.Error:
+        print "%s: Incorrect configuration file: %s" % (execname, configfile)
+        sys.exit(RC_BADCONFIG)
+
     # Add ../ to sys.path to allow running from branch
     possible_topdir = os.path.normpath(os.path.join(os.path.abspath(execname),
                                                     os.pardir, os.pardir))
@@ -59,7 +70,7 @@ if __name__ == '__main__':
     from cinder.rootwrap import wrapper
 
     # Execute command if it matches any of the loaded filters
-    filters = wrapper.load_filters()
+    filters = wrapper.load_filters(filters_path)
     filtermatch = wrapper.match_filter(filters, userargs)
     if filtermatch:
         obj = subprocess.Popen(filtermatch.get_command(userargs),

cinder/rootwrap/filters.py

@@ -92,37 +92,42 @@ def get_environment(self, userargs):
 
 class KillFilter(CommandFilter):
     """Specific filter for the kill calls.
-       1st argument is a list of accepted signals (emptystring means no signal)
-       2nd argument is a list of accepted affected executables.
+       1st argument is the user to run /bin/kill under
+       2nd argument is the location of the affected executable
+       Subsequent arguments list the accepted signals (if any)
 
        This filter relies on /proc to accurately determine affected
        executable, so it will only work on procfs-capable systems (not OSX).
     """
 
+    def __init__(self, *args):
+        super(KillFilter, self).__init__("/bin/kill", *args)
+
     def match(self, userargs):
         if userargs[0] != "kill":
             return False
         args = list(userargs)
         if len(args) == 3:
+            # A specific signal is requested
             signal = args.pop(1)
-            if signal not in self.args[0]:
+            if signal not in self.args[1:]:
                 # Requested signal not in accepted list
                 return False
         else:
             if len(args) != 2:
                 # Incorrect number of arguments
                 return False
-            if '' not in self.args[0]:
-                # No signal, but list doesn't include empty string
+            if len(self.args) > 1:
+                # No signal requested, but filter requires specific signal
                 return False
         try:
             command = os.readlink("/proc/%d/exe" % int(args[1]))
             # NOTE(dprince): /proc/PID/exe may have ' (deleted)' on
             # the end if an executable is updated or deleted
             if command.endswith(" (deleted)"):
                 command = command[:command.rindex(" ")]
-            if command not in self.args[1]:
-                # Affected executable not in accepted list
+            if command != self.args[0]:
+                # Affected executable does not match
                 return False
         except (ValueError, OSError):
             # Incorrect PID

cinder/rootwrap/wrapper.py

@@ -16,26 +16,38 @@
 #    under the License.
 
 
+import ConfigParser
 import os
-import sys
+import string
 
+from cinder.rootwrap import filters
 
-FILTERS_MODULES = ['cinder.rootwrap.volume']
 
+def build_filter(class_name, *args):
+    """Returns a filter object of class class_name"""
+    if not hasattr(filters, class_name):
+        # TODO(ttx): Log the error (whenever cinder-rootwrap has a log file)
+        return None
+    filterclass = getattr(filters, class_name)
+    return filterclass(*args)
 
-def load_filters():
-    """Load filters from modules present in cinder.rootwrap."""
-    filters = []
-    for modulename in FILTERS_MODULES:
-        try:
-            __import__(modulename)
-            module = sys.modules[modulename]
-            filters = filters + module.filterlist
-        except ImportError:
-            # It's OK to have missing filters, since filter modules are
-            # shipped with specific nodes rather than with python-cinder
-            pass
-    return filters
+
+def load_filters(filters_path):
+    """Load filters from a list of directories"""
+    filterlist = []
+    for filterdir in filters_path:
+        if not os.path.isdir(filterdir):
+            continue
+        for filterfile in os.listdir(filterdir):
+            filterconfig = ConfigParser.RawConfigParser()
+            filterconfig.read(os.path.join(filterdir, filterfile))
+            for (name, value) in filterconfig.items("Filters"):
+                filterdefinition = [string.strip(s) for s in value.split(',')]
+                newfilter = build_filter(*filterdefinition)
+                if newfilter is None:
+                    continue
+                filterlist.append(newfilter)
+    return filterlist
 
 
 def match_filter(filters, userargs):

cinder/tests/test_nova_rootwrap.py

@@ -67,35 +67,33 @@ def test_DnsmasqFilter(self):
                   "Test requires /proc filesystem (procfs)")
     def test_KillFilter(self):
         p = subprocess.Popen(["/bin/sleep", "5"])
-        f = filters.KillFilter("/bin/kill", "root",
-                               ["-ALRM"],
-                               ["/bin/sleep"])
-        usercmd = ['kill', '-9', p.pid]
+        f = filters.KillFilter("root", "/bin/sleep", "-9", "-HUP")
+        f2 = filters.KillFilter("root", "/usr/bin/sleep", "-9", "-HUP")
+        usercmd = ['kill', '-ALRM', p.pid]
         # Incorrect signal should fail
-        self.assertFalse(f.match(usercmd))
+        self.assertFalse(f.match(usercmd) or f2.match(usercmd))
         usercmd = ['kill', p.pid]
         # Providing no signal should fail
-        self.assertFalse(f.match(usercmd))
+        self.assertFalse(f.match(usercmd) or f2.match(usercmd))
+        # Providing matching signal should be allowed
+        usercmd = ['kill', '-9', p.pid]
+        self.assertTrue(f.match(usercmd) or f2.match(usercmd))
 
-        f = filters.KillFilter("/bin/kill", "root",
-                               ["-9", ""],
-                               ["/bin/sleep"])
-        usercmd = ['kill', '-9', os.getpid()]
+        f = filters.KillFilter("root", "/bin/sleep")
+        f2 = filters.KillFilter("root", "/usr/bin/sleep")
+        usercmd = ['kill', os.getpid()]
         # Our own PID does not match /bin/sleep, so it should fail
-        self.assertFalse(f.match(usercmd))
-        usercmd = ['kill', '-9', 999999]
+        self.assertFalse(f.match(usercmd) or f2.match(usercmd))
+        usercmd = ['kill', 999999]
         # Nonexistant PID should fail
-        self.assertFalse(f.match(usercmd))
+        self.assertFalse(f.match(usercmd) or f2.match(usercmd))
         usercmd = ['kill', p.pid]
         # Providing no signal should work
-        self.assertTrue(f.match(usercmd))
-        usercmd = ['kill', '-9', p.pid]
-        # Providing -9 signal should work
-        self.assertTrue(f.match(usercmd))
+        self.assertTrue(f.match(usercmd) or f2.match(usercmd))
 
     def test_KillFilter_no_raise(self):
         """Makes sure ValueError from bug 926412 is gone"""
-        f = filters.KillFilter("/bin/kill", "root", [""])
+        f = filters.KillFilter("root", "")
         # Providing anything other than kill should be False
         usercmd = ['notkill', 999999]
         self.assertFalse(f.match(usercmd))
@@ -109,9 +107,7 @@ def test_KillFilter_deleted_exe(self):
         def fake_readlink(blah):
             return '/bin/commandddddd (deleted)'
 
-        f = filters.KillFilter("/bin/kill", "root",
-                               [""],
-                               ["/bin/commandddddd"])
+        f = filters.KillFilter("root", "/bin/commandddddd")
         usercmd = ['kill', 1234]
         # Providing no signal should work
         self.stubs.Set(os, 'readlink', fake_readlink)

etc/cinder/rootwrap.conf

@@ -0,0 +1,7 @@
+# Configuration for cinder-rootwrap
+# This file should be owned by (and only-writeable by) the root user
+
+[DEFAULT]
+# List of directories to load filter definitions from (separated by ',').
+# These directories MUST all be only writeable by root !
+filters_path=/etc/cinder/rootwrap.d,/usr/share/cinder/rootwrap

etc/cinder/rootwrap.d/volume.filters

@@ -0,0 +1,27 @@
+# nova-rootwrap command filters for volume nodes
+# This file should be owned by (and only-writeable by) the root user
+
+[Filters]
+# nova/volume/iscsi.py: iscsi_helper '--op' ...
+ietadm: CommandFilter, /usr/sbin/ietadm, root
+tgtadm: CommandFilter, /usr/sbin/tgtadm, root
+
+# nova/volume/driver.py: 'vgs', '--noheadings', '-o', 'name'
+vgs: CommandFilter, /sbin/vgs, root
+
+# nova/volume/driver.py: 'lvcreate', '-L', sizestr, '-n', volume_name,..
+# nova/volume/driver.py: 'lvcreate', '-L', ...
+lvcreate: CommandFilter, /sbin/lvcreate, root
+
+# nova/volume/driver.py: 'dd', 'if=%s' % srcstr, 'of=%s' % deststr,...
+dd: CommandFilter, /bin/dd, root
+
+# nova/volume/driver.py: 'lvremove', '-f', %s/%s % ...
+lvremove: CommandFilter, /sbin/lvremove, root
+
+# nova/volume/driver.py: 'lvdisplay', '--noheading', '-C', '-o', 'Attr',..
+lvdisplay: CommandFilter, /sbin/lvdisplay, root
+
+# nova/volume/driver.py: 'iscsiadm', '-m', 'discovery', '-t',...
+# nova/volume/driver.py: 'iscsiadm', '-m', 'node', '-T', ...
+iscsiadm: CommandFilter, /sbin/iscsiadm, root