Azure: Redact on-disk user password in /var/lib/ovf-env.xml

The fabric provides the user password in plain text via the CDROM,
and cloud-init has previously wrote the ovf-env.xml in /var/lib/waagent
with the password in plain text. This change redacts the password.

LP: #1311827

3 files changed, 92 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index ff06e989..8d59d024 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -42,6 +42,7 @@
  - Centos: detect/expect use of systemd on centos 7. [Brian Rak]
  - Azure: remove dependency on walinux-agent [Daniel Watkins]
  - EC2: know about eu-central-1 availability-zone (LP: #1456684)
+ - Azure: remove password from on-disk ovf-env.xml (LP: #1443311) [Ben Howard]
 0.7.6:
  - open 0.7.6
  - Enable vendordata on CloudSigma datasource (LP: #1303986)
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index f2388c63..d0a882ca 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -23,6 +23,8 @@ import fnmatch
 import os
 import os.path
 import time
+import xml.etree.ElementTree as ET
+
 from xml.dom import minidom
 
 from cloudinit import log as logging
@@ -68,6 +70,10 @@ BUILTIN_CLOUD_CONFIG = {
 DS_CFG_PATH = ['datasource', DS_NAME]
 DEF_EPHEMERAL_LABEL = 'Temporary Storage'
 
+# The redacted password fails to meet password complexity requirements
+# so we can safely use this to mask/redact the password in the ovf-env.xml
+DEF_PASSWD_REDACTION = 'REDACTED'
+
 
 def get_hostname(hostname_command='hostname'):
     return util.subp(hostname_command, capture=True)[0].strip()
@@ -414,14 +420,30 @@ def wait_for_files(flist, maxwait=60, naplen=.5):
 
 
 def write_files(datadir, files, dirmode=None):
+
+    def _redact_password(cnt, fname):
+        """Azure provides the UserPassword in plain text. So we redact it"""
+        try:
+            root = ET.fromstring(cnt)
+            for elem in root.iter():
+                if ('UserPassword' in elem.tag and
+                   elem.text != DEF_PASSWD_REDACTION):
+                    elem.text = DEF_PASSWD_REDACTION
+            return ET.tostring(root)
+        except Exception as e:
+            LOG.critical("failed to redact userpassword in {}".format(fname))
+            return cnt
+
     if not datadir:
         return
     if not files:
         files = {}
     util.ensure_dir(datadir, dirmode)
     for (name, content) in files.items():
-        util.write_file(filename=os.path.join(datadir, name),
-                        content=content, mode=0o600)
+        fname = os.path.join(datadir, name)
+        if 'ovf-env.xml' in name:
+            content = _redact_password(content, fname)
+        util.write_file(filename=fname, content=content, mode=0o600)
 
 
 def invoke_agent(cmd):
@@ -576,7 +598,7 @@ def read_azure_ovf(contents):
     defuser = {}
     if username:
         defuser['name'] = username
-    if password:
+    if password and DEF_PASSWD_REDACTION != password:
         defuser['passwd'] = encrypt_pass(password)
         defuser['lock_passwd'] = False
 
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 4c4b8eec..33b971f6 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -18,6 +18,7 @@ import stat
 import yaml
 import shutil
 import tempfile
+import xml.etree.ElementTree as ET
 
 
 def construct_valid_ovf_env(data=None, pubkeys=None, userdata=None):
@@ -144,6 +145,39 @@ class TestAzureDataSource(TestCase):
 
         return dsrc
 
+    def xml_equals(self, oxml, nxml):
+        """Compare two sets of XML to make sure they are equal"""
+
+        def create_tag_index(xml):
+            et = ET.fromstring(xml)
+            ret = {}
+            for x in et.iter():
+                ret[x.tag] = x
+            return ret
+
+        def tags_exists(x, y):
+            for tag in x.keys():
+                self.assertIn(tag, y)
+            for tag in y.keys():
+                self.assertIn(tag, x)
+
+        def tags_equal(x, y):
+            for x_tag, x_val in x.items():
+                y_val = y.get(x_val.tag)
+                self.assertEquals(x_val.text, y_val.text)
+
+        old_cnt = create_tag_index(oxml)
+        new_cnt = create_tag_index(nxml)
+        tags_exists(old_cnt, new_cnt)
+        tags_equal(old_cnt, new_cnt)
+
+    def xml_notequals(self, oxml, nxml):
+        try:
+            self.xml_equals(oxml, nxml)
+        except AssertionError as e:
+            return
+        raise AssertionError("XML is the same")
+
     def test_basic_seed_dir(self):
         odata = {'HostName': "myhost", 'UserName': "myuser"}
         data = {'ovfcontent': construct_valid_ovf_env(data=odata),
@@ -322,6 +356,31 @@ class TestAzureDataSource(TestCase):
 
         self.assertEqual(userdata.encode('us-ascii'), dsrc.userdata_raw)
 
+    def test_password_redacted_in_ovf(self):
+        odata = {'HostName': "myhost", 'UserName': "myuser",
+                 'UserPassword': "mypass"}
+        data = {'ovfcontent': construct_valid_ovf_env(data=odata)}
+        dsrc = self._get_ds(data)
+        ret = dsrc.get_data()
+
+        self.assertTrue(ret)
+        ovf_env_path = os.path.join(self.waagent_d, 'ovf-env.xml')
+
+        # The XML should not be same since the user password is redacted
+        on_disk_ovf = load_file(ovf_env_path)
+        self.xml_notequals(data['ovfcontent'], on_disk_ovf)
+
+        # Make sure that the redacted password on disk is not used by CI
+        self.assertNotEquals(dsrc.cfg.get('password'),
+                             DataSourceAzure.DEF_PASSWD_REDACTION)
+
+        # Make sure that the password was really encrypted
+        et = ET.fromstring(on_disk_ovf)
+        for elem in et.iter():
+            if 'UserPassword' in elem.tag:
+                self.assertEquals(DataSourceAzure.DEF_PASSWD_REDACTION,
+                                  elem.text)
+
     def test_ovf_env_arrives_in_waagent_dir(self):
         xml = construct_valid_ovf_env(data={}, userdata="FOODATA")
         dsrc = self._get_ds({'ovfcontent': xml})
@@ -331,7 +390,7 @@ class TestAzureDataSource(TestCase):
         # we expect that the ovf-env.xml file is copied there.
         ovf_env_path = os.path.join(self.waagent_d, 'ovf-env.xml')
         self.assertTrue(os.path.exists(ovf_env_path))
-        self.assertEqual(xml, load_file(ovf_env_path))
+        self.xml_equals(xml, load_file(ovf_env_path))
 
     def test_ovf_can_include_unicode(self):
         xml = construct_valid_ovf_env(data={})
@@ -380,12 +439,12 @@ class TestAzureDataSource(TestCase):
         self.assertEqual(dsrc.userdata_raw, b"NEW_USERDATA")
         self.assertTrue(os.path.exists(
             os.path.join(self.waagent_d, 'otherfile')))
-        self.assertFalse(
-            os.path.exists(os.path.join(self.waagent_d, 'SharedConfig.xml')))
-        self.assertTrue(
-            os.path.exists(os.path.join(self.waagent_d, 'ovf-env.xml')))
-        self.assertEqual(new_ovfenv,
-            load_file(os.path.join(self.waagent_d, 'ovf-env.xml')))
+        self.assertFalse(os.path.exists(
+                        os.path.join(self.waagent_d, 'SharedConfig.xml')))
+        self.assertTrue(os.path.exists(
+                        os.path.join(self.waagent_d, 'ovf-env.xml')))
+        new_xml = load_file(os.path.join(self.waagent_d, 'ovf-env.xml'))
+        self.xml_equals(new_ovfenv, new_xml)
 
     def test_exception_fetching_fabric_data_doesnt_propagate(self):
         ds = self._get_ds({'ovfcontent': construct_valid_ovf_env()})