From d2d3c9cba4a647724f75c036a1985a10c966da35 Mon Sep 17 00:00:00 2001 From: Thierry Carrez Date: Mon, 23 Jul 2012 16:21:28 +0200 Subject: [PATCH] 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 | 33 +++++++++++++------- cinder/rootwrap/__init__.py | 0 cinder/rootwrap/filters.py | 19 +++++++----- cinder/rootwrap/volume.py | 45 ---------------------------- cinder/rootwrap/wrapper.py | 42 ++++++++++++++++---------- cinder/tests/test_nova_rootwrap.py | 38 +++++++++++------------ etc/cinder/rootwrap.conf | 7 +++++ etc/cinder/rootwrap.d/volume.filters | 27 +++++++++++++++++ 8 files changed, 112 insertions(+), 99 deletions(-) mode change 100755 => 100644 cinder/rootwrap/__init__.py mode change 100755 => 100644 cinder/rootwrap/filters.py delete mode 100755 cinder/rootwrap/volume.py mode change 100755 => 100644 cinder/rootwrap/wrapper.py create mode 100644 etc/cinder/rootwrap.conf create mode 100644 etc/cinder/rootwrap.d/volume.filters diff --git a/bin/cinder-rootwrap b/bin/cinder-rootwrap index 537324c6c..71984d96e 100755 --- a/bin/cinder-rootwrap +++ b/bin/cinder-rootwrap @@ -18,21 +18,21 @@ """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 @@ -40,16 +40,27 @@ 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), diff --git a/cinder/rootwrap/__init__.py b/cinder/rootwrap/__init__.py old mode 100755 new mode 100644 diff --git a/cinder/rootwrap/filters.py b/cinder/rootwrap/filters.py old mode 100755 new mode 100644 index a51ecae3d..3509602f2 --- a/cinder/rootwrap/filters.py +++ b/cinder/rootwrap/filters.py @@ -92,28 +92,33 @@ class DnsmasqFilter(CommandFilter): 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])) @@ -121,8 +126,8 @@ class KillFilter(CommandFilter): # 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 diff --git a/cinder/rootwrap/volume.py b/cinder/rootwrap/volume.py deleted file mode 100755 index 8d96f4600..000000000 --- a/cinder/rootwrap/volume.py +++ /dev/null @@ -1,45 +0,0 @@ -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - -# Copyright (c) 2011 OpenStack, LLC. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - - -from cinder.rootwrap import filters - -filterlist = [ - # cinder/volume/iscsi.py: iscsi_helper '--op' ... - filters.CommandFilter("/usr/sbin/ietadm", "root"), - filters.CommandFilter("/usr/sbin/tgtadm", "root"), - - # cinder/volume/driver.py: 'vgs', '--noheadings', '-o', 'name' - filters.CommandFilter("/sbin/vgs", "root"), - - # cinder/volume/driver.py: 'lvcreate', '-L', sizestr, '-n', volume_name,.. - # cinder/volume/driver.py: 'lvcreate', '-L', ... - filters.CommandFilter("/sbin/lvcreate", "root"), - - # cinder/volume/driver.py: 'dd', 'if=%s' % srcstr, 'of=%s' % deststr,... - filters.CommandFilter("/bin/dd", "root"), - - # cinder/volume/driver.py: 'lvremove', '-f', "%s/%s" % ... - filters.CommandFilter("/sbin/lvremove", "root"), - - # cinder/volume/driver.py: 'lvdisplay','--noheading','-C','-o','Attr',.. - filters.CommandFilter("/sbin/lvdisplay", "root"), - - # cinder/volume/driver.py: 'iscsiadm', '-m', 'discovery', '-t',... - # cinder/volume/driver.py: 'iscsiadm', '-m', 'node', '-T', ... - filters.CommandFilter("/sbin/iscsiadm", "root"), - ] diff --git a/cinder/rootwrap/wrapper.py b/cinder/rootwrap/wrapper.py old mode 100755 new mode 100644 index 683224e31..4211a4926 --- a/cinder/rootwrap/wrapper.py +++ b/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): diff --git a/cinder/tests/test_nova_rootwrap.py b/cinder/tests/test_nova_rootwrap.py index 42fd5ee5f..47f176a2f 100644 --- a/cinder/tests/test_nova_rootwrap.py +++ b/cinder/tests/test_nova_rootwrap.py @@ -67,35 +67,33 @@ class RootwrapTestCase(test.TestCase): "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 @@ class RootwrapTestCase(test.TestCase): 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) diff --git a/etc/cinder/rootwrap.conf b/etc/cinder/rootwrap.conf new file mode 100644 index 000000000..f2bafe891 --- /dev/null +++ b/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 diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters new file mode 100644 index 000000000..94a621b98 --- /dev/null +++ b/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 -- 2.45.2