Save admin password to sysmeta in libvirt driver

We have an API for setting the admin password for an already created
instance and we have a metadata API for retrieving the encrypted
password. In the libvirt driver, when a request to set the admin
password is received, it is indeed set in the guest but the instance
system metadata is never updated with the encrypted password, so
attempts to retrieve the password via the metadata service API result
in an empty string returned instead of the encrypted password.

This has been broken in the libvirt driver since the set admin password
password feature was added, as far as I can tell. The xen api driver,
however, handles the same thing correctly and this adds similar logic
to the libvirt driver to fix the problem.

Closes-Bug: #1748544

Change-Id: Icf44c4c94529cb75232abe1f3ecc5a4d3646b0cc

nova/tests/unit/virt/libvirt/test_driver.py

@@ -1337,6 +1337,39 @@ class LibvirtConnTestCase(test.NoDBTestCase,
 
         mock_guest.set_user_password.assert_called_once_with("root", "123")
 
+    @mock.patch('nova.objects.Instance.save')
+    @mock.patch('oslo_serialization.base64.encode_as_text')
+    @mock.patch('nova.api.metadata.password.convert_password')
+    @mock.patch('nova.crypto.ssh_encrypt_text')
+    @mock.patch('nova.utils.get_image_from_system_metadata')
+    @mock.patch.object(host.Host,
+                       'has_min_version', return_value=True)
+    @mock.patch('nova.virt.libvirt.host.Host.get_guest')
+    def test_set_admin_password_saves_sysmeta(self, mock_get_guest,
+                                              ver, mock_image, mock_encrypt,
+                                              mock_convert, mock_encode,
+                                              mock_save):
+        self.flags(virt_type='kvm', group='libvirt')
+        instance = objects.Instance(**self.test_instance)
+        # Password will only be saved in sysmeta if the key_data is present
+        instance.key_data = 'ssh-rsa ABCFEFG'
+        mock_image.return_value = {"properties": {
+            "hw_qemu_guest_agent": "yes"}}
+        mock_guest = mock.Mock(spec=libvirt_guest.Guest)
+        mock_get_guest.return_value = mock_guest
+        mock_convert.return_value = {'password_0': 'converted-password'}
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        drvr.set_admin_password(instance, "123")
+
+        mock_guest.set_user_password.assert_called_once_with("root", "123")
+        mock_encrypt.assert_called_once_with(instance.key_data, '123')
+        mock_encode.assert_called_once_with(mock_encrypt.return_value)
+        mock_convert.assert_called_once_with(None, mock_encode.return_value)
+        self.assertEqual('converted-password',
+                         instance.system_metadata['password_0'])
+        mock_save.assert_called_once_with()
+
     @mock.patch.object(host.Host,
                        'has_min_version', return_value=True)
     @mock.patch('nova.virt.libvirt.host.Host.get_guest')
@@ -1439,8 +1472,11 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         mock_get_guest.return_value = mock_guest
 
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
-        self.assertRaises(exception.NovaException,
-                          drvr.set_admin_password, instance, "123")
+        with mock.patch.object(
+                drvr, '_save_instance_password_if_sshkey_present') as save_p:
+            self.assertRaises(exception.NovaException,
+                              drvr.set_admin_password, instance, "123")
+            save_p.assert_not_called()
 
     @mock.patch('nova.utils.get_image_from_system_metadata')
     @mock.patch.object(host.Host,

nova/virt/libvirt/driver.py

@@ -52,6 +52,7 @@ from os_brick import exception as brick_exception
 from os_brick.initiator import connector
 from oslo_concurrency import processutils
 from oslo_log import log as logging
+from oslo_serialization import base64
 from oslo_serialization import jsonutils
 from oslo_service import loopingcall
 from oslo_utils import encodeutils
@@ -66,6 +67,7 @@ import six
 from six.moves import range
 
 from nova.api.metadata import base as instance_metadata
+from nova.api.metadata import password
 from nova import block_device
 from nova.compute import power_state
 from nova.compute import task_states
@@ -74,6 +76,7 @@ import nova.conf
 from nova.console import serial as serial_console
 from nova.console import type as ctype
 from nova import context as nova_context
+from nova import crypto
 from nova import exception
 from nova.i18n import _
 from nova import image
@@ -1972,6 +1975,17 @@ class LibvirtDriver(driver.ComputeDriver):
         else:
             raise exception.SetAdminPasswdNotSupported()
 
+    # TODO(melwitt): Combine this with the similar xenapi code at some point.
+    def _save_instance_password_if_sshkey_present(self, instance, new_pass):
+        sshkey = instance.key_data if 'key_data' in instance else None
+        if sshkey and sshkey.startswith("ssh-rsa"):
+            enc = crypto.ssh_encrypt_text(sshkey, new_pass)
+            # NOTE(melwitt): The convert_password method doesn't actually do
+            # anything with the context argument, so we can pass None.
+            instance.system_metadata.update(
+                password.convert_password(None, base64.encode_as_text(enc)))
+            instance.save()
+
     def set_admin_password(self, instance, new_pass):
         self._can_set_admin_password(instance.image_meta)
 
@@ -1996,6 +2010,10 @@ class LibvirtDriver(driver.ComputeDriver):
                      '"%(user)s": [Error Code %(error_code)s] %(ex)s')
                    % {'user': user, 'error_code': error_code, 'ex': err_msg})
             raise exception.InternalError(msg)
+        else:
+            # Save the password in sysmeta so it may be retrieved from the
+            # metadata service.
+            self._save_instance_password_if_sshkey_present(instance, new_pass)
 
     def _can_quiesce(self, instance, image_meta):
         if CONF.libvirt.virt_type not in ('kvm', 'qemu'):