Remove env from rootwrap filter

Allowing 'env' as a CommandFilter is similar to
allowing '/bin/bash', which makes all of rootwrap pointless.
Use EnvFilter instead. Change corresponding commands
that use env for setting C locale and adjust rootwrap
filters accordingly.

Several commands that output information that is
parsed as input by cinder change their behavior
based on the environment locale, which is depending
on local system settings. The code is however only
able to parse in C locale, so enforce that one.

Closes-Bug: #1269958

Change-Id: Ie1463e608c80204c7a8906efb95899a66aa733da

diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py
index 0037632b2383b7d3d98d2ff00dfd5252a95c9c09..5ff54630a837d780ee7004d9aa9f11371d832615 100644 (file)
--- a/cinder/brick/local_dev/lvm.py
+++ b/cinder/brick/local_dev/lvm.py
@@ -100,10 +100,9 @@ class LVM(executor.Executor):
 
         """
         exists = False
-        cmd = ['vgs', '--noheadings', '-o', 'name', self.vg_name]
-        (out, err) = self._execute(*cmd,
-                                   root_helper=self._root_helper,
-                                   run_as_root=True)
+        (out, err) = self._execute(
+            'env', 'LC_ALL=C', 'vgs', '--noheadings', '-o', 'name',
+            self.vg_name, root_helper=self._root_helper, run_as_root=True)
 
         if out is not None:
             volume_groups = out.split()
@@ -117,7 +116,7 @@ class LVM(executor.Executor):
         self._execute(*cmd, root_helper=self._root_helper, run_as_root=True)
 
     def _get_vg_uuid(self):
-        (out, err) = self._execute('vgs', '--noheadings',
+        (out, err) = self._execute('env', 'LC_ALL=C', 'vgs', '--noheadings',
                                    '-o uuid', self.vg_name)
         if out is not None:
             return out.split()
@@ -170,7 +169,7 @@ class LVM(executor.Executor):
 
         """
 
-        cmd = ['vgs', '--version']
+        cmd = ['env', 'LC_ALL=C', 'vgs', '--version']
         (out, err) = putils.execute(*cmd,
                                     root_helper=root_helper,
                                     run_as_root=True)
@@ -249,7 +248,8 @@ class LVM(executor.Executor):
         :returns: List of Dictionaries with LV info
 
         """
-        cmd = ['lvs', '--noheadings', '--unit=g', '-o', 'vg_name,name,size']
+        cmd = ['env', 'LC_ALL=C', 'lvs', '--noheadings', '--unit=g',
+               '-o', 'vg_name,name,size']
 
         if no_suffix:
             cmd.append('--nosuffix')
@@ -601,11 +601,10 @@ class LVM(executor.Executor):
                       run_as_root=True)
 
     def lv_has_snapshot(self, name):
-        out, err = self._execute('lvdisplay', '--noheading',
-                                 '-C', '-o', 'Attr',
-                                 '%s/%s' % (self.vg_name, name),
-                                 root_helper=self._root_helper,
-                                 run_as_root=True)
+        out, err = self._execute(
+            'env', 'LC_ALL=C', 'lvdisplay', '--noheading',
+            '-C', '-o', 'Attr', '%s/%s' % (self.vg_name, name),
+            root_helper=self._root_helper, run_as_root=True)
         if out:
             out = out.strip()
             if (out[0] == 'o') or (out[0] == 'O'):

diff --git a/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py
index f7ba541f2247136b37a863dc537d2662430b4652..03fde16d0bec712036e4ed9dedee248999b4ae02 100644 (file)
--- a/cinder/tests/brick/test_brick_lvm.py
+++ b/cinder/tests/brick/test_brick_lvm.py
@@ -64,16 +64,19 @@ class BrickLvmTestCase(test.TestCase):
         cmd_string = ', '.join(cmd)
         data = "\n"
 
-        if 'vgs, --noheadings, --unit=g, -o, name' == cmd_string:
+        if ('env, LC_ALL=C, vgs, --noheadings, --unit=g, -o, name' ==
+                cmd_string):
             data = "  fake-volumes\n"
             data += "  some-other-vg\n"
-        elif 'vgs, --noheadings, -o, name, fake-volumes' == cmd_string:
+        elif ('env, LC_ALL=C, vgs, --noheadings, -o, name, fake-volumes' ==
+                cmd_string):
             data = "  fake-volumes\n"
-        elif 'vgs, --version' in cmd_string:
+        elif 'env, LC_ALL=C, vgs, --version' in cmd_string:
             data = "  LVM version:     2.02.95(2) (2012-03-06)\n"
-        elif 'vgs, --noheadings, -o uuid, fake-volumes' in cmd_string:
+        elif ('env, LC_ALL=C, vgs, --noheadings, -o uuid, fake-volumes' in
+              cmd_string):
             data = "  kVxztV-dKpG-Rz7E-xtKY-jeju-QsYU-SLG6Z1\n"
-        elif 'vgs, --noheadings, --unit=g, ' \
+        elif 'env, LC_ALL=C, vgs, --noheadings, --unit=g, ' \
              '-o, name,size,free,lv_count,uuid, ' \
              '--separator, :, --nosuffix' in cmd_string:
             data = "  fake-volumes:10.00:10.00:0:"\
@@ -84,23 +87,25 @@ class BrickLvmTestCase(test.TestCase):
                     "lWyauW-dKpG-Rz7E-xtKY-jeju-QsYU-SLG7Z2\n"
             data += "  fake-volumes-3:10.00:10.00:0:"\
                     "mXzbuX-dKpG-Rz7E-xtKY-jeju-QsYU-SLG8Z3\n"
-        elif 'lvs, --noheadings, --unit=g, -o, vg_name,name,size'\
-                in cmd_string:
+        elif ('env, LC_ALL=C, lvs, --noheadings, '
+              '--unit=g, -o, vg_name,name,size' in cmd_string):
             data = "  fake-volumes fake-1 1.00g\n"
             data += "  fake-volumes fake-2 1.00g\n"
-        elif 'lvdisplay, --noheading, -C, -o, Attr' in cmd_string:
+        elif ('env, LC_ALL=C, lvdisplay, --noheading, -C, -o, Attr' in
+              cmd_string):
             if 'test-volumes' in cmd_string:
                 data = '  wi-a-'
             else:
                 data = '  owi-a-'
-        elif 'pvs, --noheadings' in cmd_string \
+        elif 'env, LC_ALL=C, pvs, --noheadings' in cmd_string \
                 and 'fake-volumes' in cmd_string:
             data = "  fake-volumes:/dev/sda:10.00g:8.99g\n"
-        elif 'pvs, --noheadings' in cmd_string:
+        elif 'env, LC_ALL=C, pvs, --noheadings' in cmd_string:
             data = "  fake-volumes:/dev/sda:10.00g:8.99g\n"
             data += "  fake-volumes-2:/dev/sdb:10.00g:8.99g\n"
             data += "  fake-volumes-3:/dev/sdc:10.00g:8.99g\n"
-        elif 'lvs, --noheadings, --unit=g, -o, size,data_percent, ' \
+        elif 'env, LC_ALL=C, lvs, --noheadings, --unit=g, -o, ' \
+             'size,data_percent, ' \
              '--separator, :' in cmd_string:
             data = "  9:12\n"
         elif 'lvcreate, -T, -L, ' in cmd_string:

diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters
index 11996250c9b4e1a2da59c8c69e4b581f94c5fb85..2f853ceae3c3d1d8f88dc24c720a665be4314662 100644 (file)
--- a/etc/cinder/rootwrap.d/volume.filters
+++ b/etc/cinder/rootwrap.d/volume.filters
@@ -8,8 +8,11 @@ tgtadm: CommandFilter, tgtadm, root
 tgt-admin: CommandFilter, tgt-admin, root
 cinder-rtstool: CommandFilter, cinder-rtstool, root
 
-# cinder/volume/driver.py: 'vgs', '--noheadings', '-o', 'name'
-vgs: CommandFilter, vgs, root
+# LVM related show commands
+pvs: EnvFilter, env, root, LC_ALL=C, pvs
+vgs: EnvFilter, env, root, LC_ALL=C, vgs
+lvs: EnvFilter, env, root, LC_ALL=C, lvs
+lvdisplay: EnvFilter, env, root, LC_ALL=C, lvdisplay
 
 # cinder/volume/driver.py: 'lvcreate', '-L', sizestr, '-n', volume_name,..
 # cinder/volume/driver.py: 'lvcreate', '-L', ...
@@ -21,9 +24,6 @@ dd: CommandFilter, dd, root
 # cinder/volume/driver.py: 'lvremove', '-f', %s/%s % ...
 lvremove: CommandFilter, lvremove, root
 
-# cinder/volume/driver.py: 'lvdisplay', '--noheading', '-C', '-o', 'Attr',..
-lvdisplay: CommandFilter, lvdisplay, root
-
 # cinder/volume/driver.py: 'lvrename', '%(vg)s', '%(orig)s' '(new)s'...
 lvrename: CommandFilter, lvrename, root
 
@@ -47,8 +47,11 @@ chown: CommandFilter, chown, root
 # cinder/volume/driver.py
 dmsetup: CommandFilter, dmsetup, root
 ln: CommandFilter, ln, root
-qemu-img: CommandFilter, qemu-img, root
-env: CommandFilter, env, root
+
+# cinder/image/image_utils.py
+qemu-img: EnvFilter, env, root, LC_ALL=C, qemu-img
+qemu-img_convert: CommandFilter, qemu-img, root
+
 udevadm: CommandFilter, udevadm, root
 
 # cinder/volume/driver.py: utils.read_file_as_root()
@@ -62,11 +65,9 @@ du: CommandFilter, du, root
 truncate: CommandFilter, truncate, root
 chmod: CommandFilter, chmod, root
 rm: CommandFilter, rm, root
-lvs: CommandFilter, lvs, root
 find: CommandFilter, find, root
 
 # cinder/volume/drivers/glusterfs.py
-mv: CommandFilter, mv, root
 chgrp: CommandFilter, chgrp, root
 
 # cinder/volumes/drivers/hds/hds.py:
@@ -83,6 +84,7 @@ systool: CommandFilter, systool, root
 blockdev: CommandFilter, blockdev, root
 
 # cinder/volume/drivers/gpfs.py
+mv: CommandFilter, mv, root
 mmgetstate: CommandFilter, /usr/lpp/mmfs/bin/mmgetstate, root
 mmclone: CommandFilter, /usr/lpp/mmfs/bin/mmclone, root
 mmlsattr: CommandFilter, /usr/lpp/mmfs/bin/mmlsattr, root