Bug 1307094 - Passwords disappear from domain XML passed to virDomainRestoreFlags or virDomainSaveImageDefineXML
Passwords disappear from domain XML passed to virDomainRestoreFlags or virDom...
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libvirt (Show other bugs)
6.7
Unspecified Unspecified
urgent Severity urgent
: rc
: ---
Assigned To: Jiri Denemark
Virtualization Bugs
: ZStream
Depends On: 1254164
Blocks: 1305530 1310747
  Show dependency treegraph
 
Reported: 2016-02-12 12:10 EST by Jiri Denemark
Modified: 2016-05-10 15:26 EDT (History)
18 users (show)

See Also:
Fixed In Version: libvirt-0.10.2-59.el6
Doc Type: Bug Fix
Doc Text:
Prior to this update, the libvirt service in some cases removed the password for the SPICE client from the domain XML file after modifying the file and restoring the domain. As a consequence, anyone was able to connect to the SPICE client without password authentication. With this update, the code that updates XML configuration of a saved domain uses correct internal options to avoid removing passwords. As a result, users can change the XML file of a saved domain without the risk of losing set-up passwords.
Story Points: ---
Clone Of: 1254164
: 1310747 (view as bug list)
Environment:
Last Closed: 2016-05-10 15:26:13 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
The debug log for libvirtd crash (64.12 KB, text/plain)
2016-03-04 04:39 EST, zhenfeng wang
no flags Details

  None (edit)
Description Jiri Denemark 2016-02-12 12:10:09 EST
+++ This bug was initially created as a clone of Bug #1254164 +++

Description of problem:
on a VM with SPICE configured, libvirt 1.2.8-16.el7_1.3 on RHEL 7.1 sends 'disable-ticketing' to QEMU after a save/restore cycle.
This is not what libvirt 0.10.2-46.el6_6.6 used to do, and may open breaches into VMs.

Version-Release number of selected component (if applicable):
libvirt-0.10.2-54.el6_7.3.x86_64

How reproducible:
100%

Steps to Reproduce:
1. define a domain with spice password set, e.g.,
   <graphics type='spice' autoport='yes' passwd='**' passwdValidTo='1970-01-01T00:00:01'/>
2. start the domain
3. virsh save DOM /tmp/DOM.save
4. virsh save-image-dumpxml --security-info /tmp/DOM.save >/tmp/DOM.xml
5. check the password is present in /tmp/DOM.xml
6. change the spice password using virsh save-image-edit /tmp/DOM.save
7. virsh save-image-dumpxml --security-info /tmp/DOM.save
8. virsh restore --xml /tmp/DOM.xml /tmp/DOM.save
9. virsh dumpxml --security-info DOM

Actual results:

The XMLs produced in step 7 and 9 do not contain any spice password.

Expected results:

Step 7 should show the changed password in the XML and step 9 should show the original spice password.
Additional info:

Fixed upstream with the three commits:

commit f76df311e8896957b92d0e0c5251ee3e6754d9ac
Author: Luyao Huang <lhuang@redhat.com>
Date:   Tue Jan 20 17:04:41 2015 +0800

    qemu: fix cannot set graphic passwd via qemuDomainSaveImageDefineXML
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1183890
    
    When we try to update a xml to a image file, we will clear the
    graphics passwd settings, because we do not pass VIR_DOMAIN_XML_SECURE
    to qemuDomainDefCopy, qemuDomainDefFormatBuf won't format the passwd.
    
    Add VIR_DOMAIN_XML_SECURE flag when we call qemuDomainDefCopy
    in qemuDomainSaveImageUpdateDef.
    
    Signed-off-by: Luyao Huang <lhuang@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bc0a48c..1d3bee6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5623,7 +5623,8 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,

     if (!(newdef_migr = qemuDomainDefCopy(driver,
                                           newdef,
-                                          VIR_DOMAIN_XML_MIGRATABLE)))
+                                          VIR_DOMAIN_XML_MIGRATABLE |
+                                          VIR_DOMAIN_XML_SECURE)))
         goto cleanup;

     if (!virDomainDefCheckABIStability(def, newdef_migr)) {



commit cf2d4c603c37a58c7c6cacc87f0e78b45903ae7c
Author: Michal Privoznik <mprivozn@redhat.com>
Date:   Thu Feb 19 13:54:53 2015 +0100

    qemu: Use correct flags for ABI stability check in SaveImageUpdateDef
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1183869
    
    Soo. you've successfully started yourself a domain. And since you want
    to use it on your host exclusively you are confident enough to
    passthrough the host CPU model, like this:
    
      <cpu mode='host-passthrough'/>
    
    Then, after a while, you want to save the domain into a file (e.g.
    virsh save dom dom.save). And here comes the trouble. The file consist
    of two parts: Libvirt header (containing domain XML among other
    things), and qemu migration data. Now, the domain XML in the header is
    formatted using special flags (VIR_DOMAIN_XML_SECURE |
    VIR_DOMAIN_XML_UPDATE_CPU | VIR_DOMAIN_XML_INACTIVE |
    VIR_DOMAIN_XML_MIGRATABLE).
    
    Then, on your way back from the bar, you think of changing something
    in the XML in the saved file (we have a command for it after all), say
    listen address for graphics console. So you successfully type in the
    command:
    
      virsh save-image-edit dom.save
    
    Change all the bits, and exit the editor. But instead of success
    you're left with sad error message:
    
      error: unsupported configuration: Target CPU model <null> does not
      match source Pentium Pro
    
    Sigh. Digging into the code you see lines, where we check for ABI
    stability. The new XML you've produced is compared with the old one
    from the saved file to see if qemu ABI will break or not. Wait, what?
    We are using different flags to parse the XML you've provided so we
    were just lucky it worked in some cases? Yep, that's right.
    
    Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bec05d4..4057abd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5734,8 +5734,8 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,

     if (!(newdef_migr = qemuDomainDefCopy(driver,
                                           newdef,
-                                          VIR_DOMAIN_XML_MIGRATABLE |
-                                          VIR_DOMAIN_XML_SECURE)))
+                                          QEMU_DOMAIN_FORMAT_LIVE_FLAGS ||
+                                          VIR_DOMAIN_XML_MIGRATABLE)))
         goto cleanup;

     if (!virDomainDefCheckABIStability(def, newdef_migr)) {



commit c75f42f3318463b0555edac333c8f20d54b4d5dc
Author: Ján Tomko <jtomko@redhat.com>
Date:   Fri Feb 27 11:55:40 2015 +0100

    Really fix XML formatting flags in SaveImageUpdateDef
    
    Commit cf2d4c6 used a logical or instead of bitwise or,
    effectively passing 1, that is VIR_DOMAIN_XML_INACTIVE.
    
    This was caught by a warning when building with clang.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1183869

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e9aa0b5..e282464 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5734,7 +5734,7 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,

     if (!(newdef_migr = qemuDomainDefCopy(driver,
                                           newdef,
-                                          QEMU_DOMAIN_FORMAT_LIVE_FLAGS ||
+                                          QEMU_DOMAIN_FORMAT_LIVE_FLAGS |
                                           VIR_DOMAIN_XML_MIGRATABLE)))
         goto cleanup;
Comment 4 zhenfeng wang 2016-02-17 22:29:34 EST
Could reproduce the bug with libvirt-0.10.2-56.el6.x86_64 with the comment0's steps
The following steps were my verify steps
Scenario1
1.Prepare a running guest with vnc&spice configured 
#virsh dumpxml aa --security-info
--
    <graphics type='spice' port='5900' autoport='yes' listen='127.0.0.1' passwd='123456'>
      <listen type='address' address='127.0.0.1'/>
    </graphics>
    <graphics type='vnc' port='5901' autoport='yes' listen='127.0.0.1' passwd='111111'>
      <listen type='address' address='127.0.0.1'/>
    </graphics>

2.save the guest
#virsh save aa aa.save

3. check the password is present in the save file
#virsh save-image-dumpxml aa.save --security-info
--
    <graphics type='spice' autoport='yes' passwd='123456'/>
    <graphics type='vnc' port='-1' autoport='yes' passwd='111111'/>

4.change the password for spice or vnc,then save the edit
#virsh save-image-edit aa.save
--
   <graphics type='spice' autoport='yes' passwd='222222'/>
    <graphics type='vnc' port='-1' autoport='yes' passwd='111111'/>
wq:

5.Check the password is presend in the save file
#virsh save-image-dumpxml aa.save --security-info
    <graphics type='spice' autoport='yes' passwd='222222'/>
    <graphics type='vnc' port='-1' autoport='yes' passwd='111111'/>

6.Restore the guest and check the password still present in the guest's xml
#virsh restore aa.save
#virsh dumpxml aa --security-info
    <graphics type='spice' port='5900' autoport='yes' listen='127.0.0.1' passwd='222222'>
      <listen type='address' address='127.0.0.1'/>
    </graphics>
    <graphics type='vnc' port='5901' autoport='yes' listen='127.0.0.1' passwd='111111'>
      <listen type='address' address='127.0.0.1'/>

7.Connect the guest with remote-viewer, the new password works
# remote-viewer spice://127.0.0.1:5900
# remote-viewer vnc://127.0.0.1:5901

8.Restart libvirtd service, could still get the expect result

Scenario2
1.Prepare the migration env with share storage in the nfs server
2.Prepare a running guest on the source host
#virsh dumpxml aa --security-info
--
    <graphics type='spice' port='5900' autoport='yes' listen='127.0.0.1' passwd='123456'>
      <listen type='address' address='127.0.0.1'/>
    </graphics>
    <graphics type='vnc' port='5901' autoport='yes' listen='127.0.0.1' passwd='111111'>
      <listen type='address' address='127.0.0.1'/>
    </graphics>

3.Dump the guest's xml and modify the spice or vnc's password in the guest's xml
#virsh dumpxml aa --security-info >aa.xml
#vim aa.xml

4.Migrate the guest with modified guest's xml, guest could migrate successfully
#virsh migrate --live aa --xml aa.xml qemu+ssh://$dest_ip/system --verbose

5.After migration finished, check the guest's password changed and present
#virsh dumpxml aa --security-info
#virsh dumpxml aa --migratable

6.Connect the guest with remote-viewer, the new password works
# remote-viewer spice://127.0.0.1:5900
# remote-viewer vnc://127.0.0.1:5901

According to upper steps, mark this bug verified
Comment 5 zhenfeng wang 2016-02-17 22:31:47 EST
Forget to offer the verify libvirt version, list it here
libvirt-0.10.2-57.el6.x86_64
Comment 8 zhenfeng wang 2016-03-04 04:36:45 EST
Hi Jiri
Found 1 issue that libvirtd will crash while restore a guest which with host-passthrough cpu model in guest's xml, please help check, thanks

version:
libvirt-0.10.2-57.el6.x86_64

steps
1.Prepare a running guest with host-passthrough cpu model
#virsh dumpxml aa
--
  <cpu mode='host-passthrough'/>

2.Save the guest
#virsh save aa aa.save

3.change any content in the save file
#virsh save-image-edit aa.save
--
 <on_crash>restart</on_crash>

to

 <on_crash>coredump-restart</on_crash>

wq:


4.Restore the guest from the save file, guest crashed
# virsh restore /root/aa.save 
error: Failed to restore domain from /root/aa.save
error: End of file while reading data: Input/output error
error: One or more references were leaked after disconnect from the hypervisor
error: Failed to reconnect to the hypervisor

5.debug log and coredump info will upload the attachment later
Comment 9 zhenfeng wang 2016-03-04 04:39 EST
Created attachment 1133131 [details]
The debug log for libvirtd crash
Comment 10 Jiri Denemark 2016-03-04 04:42:07 EST
OK, file a separate bug for it since it has nothing to do with this bug.
Comment 11 Jiri Denemark 2016-03-04 05:14:53 EST
So after discussing this on IRC, the crash is apparently caused by the patch for this bug.
Comment 14 Fangge Jin 2016-03-09 21:19:15 EST
Verify the issue in comment 8 with build libvirt-0.10.2-59.el6.x86_64.  - PASSED

Steps:
1. Start a guest with the following setting:
  <cpu mode='host-passthrough'>

2. Save guest:
# virsh save rhel6.6 /tmp/rhel6.6.save 

Domain rhel6.6 saved to /tmp/rhel6.6.save


3. Edit saved image(just add a blank in xml)
# virsh save-image-edit /tmp/rhel6.6.save 
State file /tmp/rhel6.6.save edited.

4. Restore guest
# virsh restore /tmp/rhel6.6.save 
Domain restored from /tmp/rhel6.6.save

5. Do step 1~4 again with cpu mode='host-model'.

6. Do step 1-4 again with cpu mode='custom'
Comment 16 errata-xmlrpc 2016-05-10 15:26:13 EDT
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2016-0738.html

Note You need to log in before you can comment on or make changes to this bug.