From acb534fd6dbecebabdae4ac8770eeedef776fbe6 Mon Sep 17 00:00:00 2001
From: Thierry Carrez <thierry@openstack.org>
Date: Tue, 3 Jul 2012 16:34:58 +0200
Subject: [PATCH] Prevent key/net/md injection writing to host fs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix bug 1015531, CVE-2012-3361

Checks that the final normalized path that is about to be written
to is always within the mounted guest filesystem.

This is a Diablo backport of the part of Russell Bryant, Pádraig Brady
and Mark McLoughlin's Folsom patch that applies to stable/diablo.

Change-Id: I134c40258ff2c9c225bd6092decd9c10e4e22273
---
 nova/tests/test_virt.py |   20 +++++++++++++++++++
 nova/virt/disk/base.py  |   49 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py
index 388f075..c33d7b3 100644
--- a/nova/tests/test_virt.py
+++ b/nova/tests/test_virt.py
@@ -15,8 +15,10 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from nova import exception
 from nova import flags
 from nova import test
+from nova.virt import disk
 from nova.virt import driver
 
 FLAGS = flags.FLAGS
@@ -81,3 +83,21 @@ class TestVirtDriver(test.TestCase):
                                                 'swap_size': 0}))
         self.assertTrue(driver.swap_is_usable({'device_name': '/dev/sdb',
                                                 'swap_size': 1}))
+
+
+class TestVirtDisk(test.TestCase):
+    def test_check_safe_path(self):
+        ret = disk._join_and_check_path_within_fs('/foo', 'etc',
+                                                      'something.conf')
+        self.assertEquals(ret, '/foo/etc/something.conf')
+
+    def test_check_unsafe_path(self):
+        self.assertRaises(exception.Invalid,
+                          disk._join_and_check_path_within_fs,
+                          '/foo', 'etc/../../../something.conf')
+
+    def test_inject_files_with_bad_path(self):
+        self.assertRaises(exception.Invalid,
+                          disk._inject_file_into_fs,
+                          '/tmp', '/etc/../../../../etc/passwd',
+                         'hax')
diff --git a/nova/virt/disk/base.py b/nova/virt/disk/base.py
index 9ae0bd1..f708741 100644
--- a/nova/virt/disk/base.py
+++ b/nova/virt/disk/base.py
@@ -250,12 +250,39 @@ def inject_data_into_fs(fs, key, net, metadata, execute):
         _inject_metadata_into_fs(metadata, fs, execute=execute)
 
 
+def _join_and_check_path_within_fs(fs, *args):
+    '''os.path.join() with safety check for injected file paths.
+
+    Join the supplied path components and make sure that the
+    resulting path we are injecting into is within the
+    mounted guest fs.  Trying to be clever and specifying a
+    path with '..' in it will hit this safeguard.
+    '''
+    absolute_path = os.path.realpath(os.path.join(fs, *args))
+    if not absolute_path.startswith(os.path.realpath(fs) + '/'):
+        raise exception.Invalid()
+    return absolute_path
+
+
+def _inject_file_into_fs(fs, path, contents, append=False):
+    absolute_path = _join_and_check_path_within_fs(fs, path.lstrip('/'))
+
+    parent_dir = os.path.dirname(absolute_path)
+    utils.execute('mkdir', '-p', parent_dir, run_as_root=True)
+
+    args = []
+    if append:
+        args.append('-a')
+    args.append(absolute_path)
+
+    kwargs = dict(process_input=contents, run_as_root=True)
+
+    utils.execute('tee', *args, **kwargs)
+
+
 def _inject_metadata_into_fs(metadata, fs, execute=None):
-    metadata_path = os.path.join(fs, "meta.js")
     metadata = dict([(m.key, m.value) for m in metadata])
-
-    utils.execute('tee', metadata_path,
-                  process_input=json.dumps(metadata), run_as_root=True)
+    _inject_file_into_fs(fs, 'meta.js', json.dumps(metadata))
 
 
 def _inject_key_into_fs(key, fs, execute=None):
@@ -264,13 +291,12 @@ def _inject_key_into_fs(key, fs, execute=None):
     key is an ssh key string.
     fs is the path to the base of the filesystem into which to inject the key.
     """
-    sshdir = os.path.join(fs, 'root', '.ssh')
+    sshdir = _join_and_check_path_within_fs(fs, 'root', '.ssh')
     utils.execute('mkdir', '-p', sshdir, run_as_root=True)
     utils.execute('chown', 'root', sshdir, run_as_root=True)
     utils.execute('chmod', '700', sshdir, run_as_root=True)
-    keyfile = os.path.join(sshdir, 'authorized_keys')
-    utils.execute('tee', '-a', keyfile,
-                  process_input='\n' + key.strip() + '\n', run_as_root=True)
+    keyfile = os.path.join('root', '.ssh', 'authorized_keys')
+    _inject_file_into_fs(fs, keyfile, '\n' + key.strip() + '\n', append=True)
 
 
 def _inject_net_into_fs(net, fs, execute=None):
@@ -278,9 +304,10 @@ def _inject_net_into_fs(net, fs, execute=None):
 
     net is the contents of /etc/network/interfaces.
     """
-    netdir = os.path.join(os.path.join(fs, 'etc'), 'network')
+    netdir = _join_and_check_path_within_fs(fs, 'etc', 'network')
     utils.execute('mkdir', '-p', netdir, run_as_root=True)
     utils.execute('chown', 'root:root', netdir, run_as_root=True)
     utils.execute('chmod', 755, netdir, run_as_root=True)
-    netfile = os.path.join(netdir, 'interfaces')
-    utils.execute('tee', netfile, process_input=net, run_as_root=True)
+
+    netfile = os.path.join('etc', 'network', 'interfaces')
+    _inject_file_into_fs(fs, netfile, net)