9119d9
From 2720e38743321bdd20f94b6abf7440b7f57f5e00 Mon Sep 17 00:00:00 2001
9119d9
Message-Id: <2720e38743321bdd20f94b6abf7440b7f57f5e00@dist-git>
9119d9
From: Pavel Hrdina <phrdina@redhat.com>
9119d9
Date: Fri, 19 Sep 2014 09:20:15 +0200
9119d9
Subject: [PATCH] Move the FIPS detection from capabilities
9119d9
9119d9
We are not detecting the presence of FIPS from QEMU, but from procfs and
9119d9
that means it's not QEMU capability. It was decided that we will pass
9119d9
this flag to QEMU even if it's not supported by old QEMU binaries.
9119d9
9119d9
This patch also reverts changes done by commit a21cfb0f to
9119d9
qemucapabilitestest and implements a new test case in qemuxml2argvtest.
9119d9
9119d9
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135431
9119d9
9119d9
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
9119d9
(cherry picked from commit da7799d879fd037849f820667b9b610bf94b6262)
9119d9
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
9119d9
---
9119d9
 src/qemu/qemu_capabilities.c                       | 24 ---------------
9119d9
 src/qemu/qemu_command.c                            | 34 ++++++++++++++++++++--
9119d9
 src/qemu/qemu_command.h                            |  6 +++-
9119d9
 src/qemu/qemu_driver.c                             |  3 +-
9119d9
 src/qemu/qemu_process.c                            |  3 +-
9119d9
 tests/qemucapabilitiesdata/caps_1.2.2-1.caps       |  1 -
9119d9
 tests/qemucapabilitiesdata/caps_1.6.0-1.caps       |  1 -
9119d9
 tests/qemucapabilitiestest.c                       | 24 +++++----------
9119d9
 .../qemuxml2argv-fips-enabled.args                 |  6 ++++
9119d9
 .../qemuxml2argvdata/qemuxml2argv-fips-enabled.xml | 25 ++++++++++++++++
9119d9
 tests/qemuxml2argvtest.c                           |  9 +++++-
9119d9
 tests/qemuxmlnstest.c                              |  2 +-
9119d9
 12 files changed, 89 insertions(+), 49 deletions(-)
9119d9
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.args
9119d9
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.xml
9119d9
9119d9
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
9119d9
index 67bfc36..4145c22 100644
9119d9
--- a/src/qemu/qemu_capabilities.c
9119d9
+++ b/src/qemu/qemu_capabilities.c
9119d9
@@ -3215,30 +3215,6 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
9119d9
     config.data.nix.path = monpath;
9119d9
     config.data.nix.listen = false;
9119d9
 
9119d9
-    /* Qemu 1.2 and later have a binary flag -enable-fips that must be
9119d9
-     * used for VNC auth to obey FIPS settings; but the flag only
9119d9
-     * exists on Linux, and with no way to probe for it via QMP.  Our
9119d9
-     * solution: if FIPS mode is required, then unconditionally use
9119d9
-     * the flag, regardless of qemu version, for the following matrix:
9119d9
-     *
9119d9
-     *                          old QEMU            new QEMU
9119d9
-     * FIPS enabled             doesn't start       VNC auth disabled
9119d9
-     * FIPS disabled/missing    VNC auth enabled    VNC auth enabled
9119d9
-     *
9119d9
-     * Setting the flag here instead of in virQEMUCapsInitQMPMonitor
9119d9
-     * or virQEMUCapsInitHelp also allows the testsuite to be
9119d9
-     * independent of FIPS setting.
9119d9
-     */
9119d9
-    if (virFileExists("/proc/sys/crypto/fips_enabled")) {
9119d9
-        char *buf = NULL;
9119d9
-
9119d9
-        if (virFileReadAll("/proc/sys/crypto/fips_enabled", 10, &buf) < 0)
9119d9
-            goto cleanup;
9119d9
-        if (STREQ(buf, "1\n"))
9119d9
-            virQEMUCapsSet(qemuCaps, QEMU_CAPS_ENABLE_FIPS);
9119d9
-        VIR_FREE(buf);
9119d9
-    }
9119d9
-
9119d9
     VIR_DEBUG("Try to get caps via QMP qemuCaps=%p", qemuCaps);
9119d9
 
9119d9
     /*
9119d9
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
9119d9
index fb9c453..82ee58b 100644
9119d9
--- a/src/qemu/qemu_command.c
9119d9
+++ b/src/qemu/qemu_command.c
9119d9
@@ -3292,6 +3292,35 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
9119d9
 }
9119d9
 
9119d9
 
9119d9
+/* Qemu 1.2 and later have a binary flag -enable-fips that must be
9119d9
+ * used for VNC auth to obey FIPS settings; but the flag only
9119d9
+ * exists on Linux, and with no way to probe for it via QMP.  Our
9119d9
+ * solution: if FIPS mode is required, then unconditionally use
9119d9
+ * the flag, regardless of qemu version, for the following matrix:
9119d9
+ *
9119d9
+ *                          old QEMU            new QEMU
9119d9
+ * FIPS enabled             doesn't start       VNC auth disabled
9119d9
+ * FIPS disabled/missing    VNC auth enabled    VNC auth enabled
9119d9
+ */
9119d9
+bool
9119d9
+qemuCheckFips(void)
9119d9
+{
9119d9
+    bool ret = false;
9119d9
+
9119d9
+    if (virFileExists("/proc/sys/crypto/fips_enabled")) {
9119d9
+        char *buf = NULL;
9119d9
+
9119d9
+        if (virFileReadAll("/proc/sys/crypto/fips_enabled", 10, &buf) < 0)
9119d9
+            return ret;
9119d9
+        if (STREQ(buf, "1\n"))
9119d9
+            ret = true;
9119d9
+        VIR_FREE(buf);
9119d9
+    }
9119d9
+
9119d9
+    return ret;
9119d9
+}
9119d9
+
9119d9
+
9119d9
 char *
9119d9
 qemuBuildDriveStr(virConnectPtr conn,
9119d9
                   virDomainDiskDefPtr disk,
9119d9
@@ -7542,7 +7571,8 @@ qemuBuildCommandLine(virConnectPtr conn,
9119d9
                      virDomainSnapshotObjPtr snapshot,
9119d9
                      virNetDevVPortProfileOp vmop,
9119d9
                      qemuBuildCommandLineCallbacksPtr callbacks,
9119d9
-                     bool standalone)
9119d9
+                     bool standalone,
9119d9
+                     bool enableFips)
9119d9
 {
9119d9
     virErrorPtr originalError = NULL;
9119d9
     size_t i, j;
9119d9
@@ -7655,7 +7685,7 @@ qemuBuildCommandLine(virConnectPtr conn,
9119d9
     if (!standalone)
9119d9
         virCommandAddArg(cmd, "-S"); /* freeze CPU */
9119d9
 
9119d9
-    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_FIPS))
9119d9
+    if (enableFips)
9119d9
         virCommandAddArg(cmd, "-enable-fips");
9119d9
 
9119d9
     if (qemuBuildMachineArgStr(cmd, def, qemuCaps) < 0)
9119d9
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
9119d9
index 633ff71..aa40c9e 100644
9119d9
--- a/src/qemu/qemu_command.h
9119d9
+++ b/src/qemu/qemu_command.h
9119d9
@@ -78,7 +78,8 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
9119d9
                                    virDomainSnapshotObjPtr current_snapshot,
9119d9
                                    virNetDevVPortProfileOp vmop,
9119d9
                                    qemuBuildCommandLineCallbacksPtr callbacks,
9119d9
-                                   bool forXMLToArgv)
9119d9
+                                   bool forXMLToArgv,
9119d9
+                                   bool enableFips)
9119d9
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11);
9119d9
 
9119d9
 /* Generate '-device' string for chardev device */
9119d9
@@ -273,4 +274,7 @@ int qemuGetDriveSourceString(virStorageSourcePtr src,
9119d9
                              char **source);
9119d9
 
9119d9
 int qemuCheckDiskConfig(virDomainDiskDefPtr disk);
9119d9
+
9119d9
+bool
9119d9
+qemuCheckFips(void);
9119d9
 #endif /* __QEMU_COMMAND_H__*/
9119d9
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
9119d9
index f914081..98b214a 100644
9119d9
--- a/src/qemu/qemu_driver.c
9119d9
+++ b/src/qemu/qemu_driver.c
9119d9
@@ -6126,7 +6126,8 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn,
9119d9
                                      NULL, -1, NULL,
9119d9
                                      VIR_NETDEV_VPORT_PROFILE_OP_NO_OP,
9119d9
                                      &buildCommandLineCallbacks,
9119d9
-                                     true)))
9119d9
+                                     true,
9119d9
+                                     qemuCheckFips())))
9119d9
         goto cleanup;
9119d9
 
9119d9
     ret = virCommandToString(cmd);
9119d9
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
9119d9
index 776edee..a0eb845 100644
9119d9
--- a/src/qemu/qemu_process.c
9119d9
+++ b/src/qemu/qemu_process.c
9119d9
@@ -4304,7 +4304,8 @@ int qemuProcessStart(virConnectPtr conn,
9119d9
     if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig,
9119d9
                                      priv->monJSON, priv->qemuCaps,
9119d9
                                      migrateFrom, stdin_fd, snapshot, vmop,
9119d9
-                                     &buildCommandLineCallbacks, false)))
9119d9
+                                     &buildCommandLineCallbacks, false,
9119d9
+                                     qemuCheckFips())))
9119d9
         goto cleanup;
9119d9
 
9119d9
     /* now that we know it is about to start call the hook if present */
9119d9
diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
9119d9
index c8a379a..3b1b699 100644
9119d9
--- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
9119d9
+++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
9119d9
@@ -112,7 +112,6 @@
9119d9
     <flag name='usb-storage'/>
9119d9
     <flag name='usb-storage.removable'/>
9119d9
     <flag name='kvm-pit-lost-tick-policy'/>
9119d9
-    <flag name='enable-fips'/>
9119d9
     <flag name='usb-kbd'/>
9119d9
     <flag name='host-pci-multidomain'/>
9119d9
     <flag name='usb-audio'/>
9119d9
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
9119d9
index e10f030..21d4320 100644
9119d9
--- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
9119d9
+++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
9119d9
@@ -138,7 +138,6 @@
9119d9
     <flag name='boot-strict'/>
9119d9
     <flag name='pvpanic'/>
9119d9
     <flag name='reboot-timeout'/>
9119d9
-    <flag name='enable-fips'/>
9119d9
     <flag name='spice-file-xfer-disable'/>
9119d9
     <flag name='spiceport'/>
9119d9
     <flag name='usb-kbd'/>
9119d9
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
9119d9
index 4e5f9e5..2f2b73d 100644
9119d9
--- a/tests/qemucapabilitiestest.c
9119d9
+++ b/tests/qemucapabilitiestest.c
9119d9
@@ -31,7 +31,6 @@ typedef testQemuData *testQemuDataPtr;
9119d9
 struct _testQemuData {
9119d9
     virDomainXMLOptionPtr xmlopt;
9119d9
     const char *base;
9119d9
-    bool fips;
9119d9
 };
9119d9
 
9119d9
 static qemuMonitorTestPtr
9119d9
@@ -143,12 +142,6 @@ testQemuCaps(const void *opaque)
9119d9
                                   qemuMonitorTestGetMonitor(mon)) < 0)
9119d9
         goto cleanup;
9119d9
 
9119d9
-    /* So that our test does not depend on the contents of /proc, we
9119d9
-     * hoisted the setting of ENABLE_FIPS to virQEMUCapsInitQMP.  But
9119d9
-     * we do want to test the effect of that flag.  */
9119d9
-    if (data->fips)
9119d9
-        virQEMUCapsSet(capsComputed, QEMU_CAPS_ENABLE_FIPS);
9119d9
-
9119d9
     if (testQemuCapsCompare(capsProvided, capsComputed) < 0)
9119d9
         goto cleanup;
9119d9
 
9119d9
@@ -183,19 +176,18 @@ mymain(void)
9119d9
 
9119d9
     data.xmlopt = xmlopt;
9119d9
 
9119d9
-#define DO_TEST_FULL(name, use_fips)                 \
9119d9
-    data.base = name;                                \
9119d9
-    data.fips = use_fips;                            \
9119d9
-    if (virtTestRun(name, testQemuCaps, &data) < 0)  \
9119d9
-        ret = -1
9119d9
+#define DO_TEST(name)                                   \
9119d9
+    do {                                                \
9119d9
+        data.base = name;                               \
9119d9
+        if (virtTestRun(name, testQemuCaps, &data) < 0) \
9119d9
+            ret = -1;                                   \
9119d9
+    } while (0)
9119d9
 
9119d9
-#define DO_TEST(name) DO_TEST_FULL(name, false)
9119d9
-
9119d9
-    DO_TEST_FULL("caps_1.2.2-1", true);
9119d9
+    DO_TEST("caps_1.2.2-1");
9119d9
     DO_TEST("caps_1.3.1-1");
9119d9
     DO_TEST("caps_1.4.2-1");
9119d9
     DO_TEST("caps_1.5.3-1");
9119d9
-    DO_TEST_FULL("caps_1.6.0-1", true);
9119d9
+    DO_TEST("caps_1.6.0-1");
9119d9
     DO_TEST("caps_1.6.50-1");
9119d9
 
9119d9
     virObjectUnref(xmlopt);
9119d9
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.args b/tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.args
9119d9
new file mode 100644
9119d9
index 0000000..196f61f
9119d9
--- /dev/null
9119d9
+++ b/tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.args
9119d9
@@ -0,0 +1,6 @@
9119d9
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
9119d9
+/usr/bin/qemu \
9119d9
+-S -enable-fips -M pc -m 214 -smp 1 -nographic -monitor \
9119d9
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
9119d9
+-hda /dev/HostVG/QEMUGuest1 -net none -serial \
9119d9
+none -parallel none
9119d9
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.xml
9119d9
new file mode 100644
9119d9
index 0000000..a6b041d
9119d9
--- /dev/null
9119d9
+++ b/tests/qemuxml2argvdata/qemuxml2argv-fips-enabled.xml
9119d9
@@ -0,0 +1,25 @@
9119d9
+<domain type='qemu'>
9119d9
+  <name>QEMUGuest1</name>
9119d9
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
9119d9
+  <memory unit='KiB'>219100</memory>
9119d9
+  <currentMemory unit='KiB'>219100</currentMemory>
9119d9
+  <vcpu placement='static' cpuset='1-4,8-20,525'>1</vcpu>
9119d9
+  <os>
9119d9
+    <type arch='i686' machine='pc'>hvm</type>
9119d9
+    <boot dev='hd'/>
9119d9
+  </os>
9119d9
+  <clock offset='utc'/>
9119d9
+  <on_poweroff>destroy</on_poweroff>
9119d9
+  <on_reboot>restart</on_reboot>
9119d9
+  <on_crash>destroy</on_crash>
9119d9
+  <devices>
9119d9
+    <emulator>/usr/bin/qemu</emulator>
9119d9
+    <disk type='block' device='disk'>
9119d9
+      <source dev='/dev/HostVG/QEMUGuest1'/>
9119d9
+      <target dev='hda' bus='ide'/>
9119d9
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
9119d9
+    </disk>
9119d9
+    <controller type='ide' index='0'/>
9119d9
+    <memballoon model='virtio'/>
9119d9
+  </devices>
9119d9
+</domain>
9119d9
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
9119d9
index 68fc01b..a9129dc 100644
9119d9
--- a/tests/qemuxml2argvtest.c
9119d9
+++ b/tests/qemuxml2argvtest.c
9119d9
@@ -259,6 +259,7 @@ typedef enum {
9119d9
     FLAG_EXPECT_FAILURE     = 1 << 1,
9119d9
     FLAG_EXPECT_PARSE_ERROR = 1 << 2,
9119d9
     FLAG_JSON               = 1 << 3,
9119d9
+    FLAG_FIPS               = 1 << 4,
9119d9
 } virQemuXML2ArgvTestFlags;
9119d9
 
9119d9
 static int testCompareXMLToArgvFiles(const char *xml,
9119d9
@@ -360,7 +361,8 @@ static int testCompareXMLToArgvFiles(const char *xml,
9119d9
                                      (flags & FLAG_JSON), extraFlags,
9119d9
                                      migrateFrom, migrateFd, NULL,
9119d9
                                      VIR_NETDEV_VPORT_PROFILE_OP_NO_OP,
9119d9
-                                     &testCallbacks, false))) {
9119d9
+                                     &testCallbacks, false,
9119d9
+                                     (flags & FLAG_FIPS)))) {
9119d9
         if (!virtTestOOMActive() &&
9119d9
             (flags & FLAG_EXPECT_FAILURE)) {
9119d9
             ret = 0;
9119d9
@@ -443,6 +445,9 @@ testCompareXMLToArgvHelper(const void *data)
9119d9
     if (virQEMUCapsGet(info->extraFlags, QEMU_CAPS_MONITOR_JSON))
9119d9
         flags |= FLAG_JSON;
9119d9
 
9119d9
+    if (virQEMUCapsGet(info->extraFlags, QEMU_CAPS_ENABLE_FIPS))
9119d9
+        flags |= FLAG_FIPS;
9119d9
+
9119d9
     result = testCompareXMLToArgvFiles(xml, args, info->extraFlags,
9119d9
                                        info->migrateFrom, info->migrateFd,
9119d9
                                        flags);
9119d9
@@ -1454,6 +1459,8 @@ mymain(void)
9119d9
     DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC,
9119d9
             QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
9119d9
 
9119d9
+    DO_TEST("fips-enabled", QEMU_CAPS_ENABLE_FIPS);
9119d9
+
9119d9
     virObjectUnref(driver.config);
9119d9
     virObjectUnref(driver.caps);
9119d9
     virObjectUnref(driver.xmlopt);
9119d9
diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c
9119d9
index e8f70d6..b3a608c 100644
9119d9
--- a/tests/qemuxmlnstest.c
9119d9
+++ b/tests/qemuxmlnstest.c
9119d9
@@ -119,7 +119,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
9119d9
                                      vmdef, &monitor_chr, json, extraFlags,
9119d9
                                      migrateFrom, migrateFd, NULL,
9119d9
                                      VIR_NETDEV_VPORT_PROFILE_OP_NO_OP,
9119d9
-                                     &testCallbacks, false)))
9119d9
+                                     &testCallbacks, false, false)))
9119d9
         goto fail;
9119d9
 
9119d9
     if (!virtTestOOMActive()) {
9119d9
-- 
9119d9
2.1.1
9119d9