Sanitize xenstore keys for metadata injection

Xenstore only allows certain characters in key names.  Change disallowed
characters as well as '/' to '_'.

Fixes bug 1029773

Change-Id: I04055bfbe662f3f3e9d90336d03670aa5468e780

2 files changed, 31 insertions, 5 deletions

diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index 61a4c8149..47a6bfcc6 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -2256,7 +2256,10 @@ class XenAPIInjectMetadataTestCase(stubs.XenAPITestBase):
         # to xenstore
         instance = dict(metadata=[FakeMetaItem("a", 1),
                                   FakeMetaItem("b", 2),
-                                  FakeMetaItem("c", 3)],
+                                  FakeMetaItem("c", 3),
+                                  # Check xenstore key sanitizing
+                                  FakeMetaItem("hi.there", 4),
+                                  FakeMetaItem("hi!t.e/e", 5)],
                         system_metadata=[FakeMetaItem("sys_a", 1),
                                          FakeMetaItem("sys_b", 2),
                                          FakeMetaItem("sys_c", 3)])
@@ -2267,12 +2270,15 @@ class XenAPIInjectMetadataTestCase(stubs.XenAPITestBase):
                     'vm-data/user-metadata/a': '1',
                     'vm-data/user-metadata/b': '2',
                     'vm-data/user-metadata/c': '3',
+                    'vm-data/user-metadata/hi_there': '4',
+                    'vm-data/user-metadata/hi_t_e_e': '5',
                     },
                 'ephem': {},
                 })
 
     def test_change_instance_metadata_add(self):
-        diff = dict(d=['+', 4])
+        # Test XenStore key sanitizing here, too.
+        diff = {'test.key': ['+', 4]}
         self.xenstore = {
             'persist': {
                 'vm-data/user-metadata/a': '1',
@@ -2293,13 +2299,13 @@ class XenAPIInjectMetadataTestCase(stubs.XenAPITestBase):
                     'vm-data/user-metadata/a': '1',
                     'vm-data/user-metadata/b': '2',
                     'vm-data/user-metadata/c': '3',
-                    'vm-data/user-metadata/d': '4',
+                    'vm-data/user-metadata/test_key': '4',
                     },
                 'ephem': {
                     'vm-data/user-metadata/a': '1',
                     'vm-data/user-metadata/b': '2',
                     'vm-data/user-metadata/c': '3',
-                    'vm-data/user-metadata/d': '4',
+                    'vm-data/user-metadata/test_key': '4',
                     },
                 })
 
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index 1113be28f..0c1894ae7 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -841,11 +841,30 @@ class VMOps(object):
         vm_ref = self._get_vm_opaque_ref(instance)
         agent.inject_file(self._session, instance, vm_ref, path, contents)
 
+    @staticmethod
+    def _sanitize_xenstore_key(key):
+        """
+        Xenstore only allows the following characters as keys:
+
+        ABCDEFGHIJKLMNOPQRSTUVWXYZ
+        abcdefghijklmnopqrstuvwxyz
+        0123456789-/_@
+
+        So convert the others to _
+
+        Also convert / to _, because that is somewhat like a path
+        separator.
+        """
+        allowed_chars = ("ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+                         "abcdefghijklmnopqrstuvwxyz"
+                         "0123456789-_@")
+        return ''.join([x in allowed_chars and x or '_' for x in key])
+
     def inject_instance_metadata(self, instance, vm_ref):
         """Inject instance metadata into xenstore."""
         def store_meta(topdir, data_list):
             for item in data_list:
-                key = item.key
+                key = self._sanitize_xenstore_key(item.key)
                 value = item.value or ''
                 self._add_to_param_xenstore(vm_ref, '%s/%s' % (topdir, key),
                                             jsonutils.dumps(value))
@@ -857,6 +876,7 @@ class VMOps(object):
         """Apply changes to instance metadata to xenstore."""
         vm_ref = self._get_vm_opaque_ref(instance)
         for key, change in diff.items():
+            key = self._sanitize_xenstore_key(key)
             location = 'vm-data/user-metadata/%s' % key
             if change[0] == '-':
                 self._remove_from_param_xenstore(vm_ref, location)