Re: [libvirt] [PATCH] qemu: always ask for -enable-fips

On a system that is enforcing FIPS, most libraries honor the current mode by default. Qemu, on the other hand, refused to honor FIPS mode unless you add the '-enable-fips' command line option; worse, this option is not discoverable via QMP, and is only present on binaries built for Linux. As far as I can tell, unconditionally using the option when it is available has no negative consequences (the option has no change to qemu behavior except when FIPS is enabled, at which point it cripples insecure VNC passwords which is the one thing that libvirt must not allow when FIPS is active). This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1035474 * src/qemu/qemu_capabilities.h (QEMU_CAPS_ENABLE_FIPS): New bit. * src/qemu/qemu_capabilities.c (virQEMUCapsInitQMPBasic): Conditionally set capability. * src/qemu/qemu_command.c (qemuBuildCommandLine): Use it. * tests/qemucapabilitiestest.c (testQemuCaps): Unconditionally set capability. * tests/qemucapabilitiesdata/caps_1.2.2-1.caps: Update list. * tests/qemucapabilitiesdata/caps_1.3.1-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.4.2-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.5.3-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.6.0-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.6.50-1.caps: Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 7 +++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 2 ++ tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiestest.c | 6 ++++++ 10 files changed, 22 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a68e555..4d1a961 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -245,6 +245,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "kvm-pit-lost-tick-policy", "boot-strict", /* 160 */ + "enable-fips", ); struct _virQEMUCaps { @@ -2469,6 +2470,12 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT); virQEMUCapsSet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE); virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); +#ifdef __linux__ + /* We always want to honor FIPS mode; but for some strange reason, + * qemu choose to require the use of a command line option, and + * worse, to provide the option only on Linux. */ + virQEMUCapsSet(qemuCaps, QEMU_CAPS_ENABLE_FIPS); +#endif } /* Capabilities that are architecture depending diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index aea64ea..87489ab 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -199,6 +199,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */ QEMU_CAPS_KVM_PIT_TICK_POLICY = 159, /* kvm-pit.lost_tick_policy */ QEMU_CAPS_BOOT_STRICT = 160, /* -boot strict */ + QEMU_CAPS_ENABLE_FIPS = 161, /* -enable-fips */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9539be7..a04725d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7747,6 +7747,8 @@ qemuBuildCommandLine(virConnectPtr conn, } } virCommandAddArg(cmd, "-S"); /* freeze CPU */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_FIPS)) + virCommandAddArg(cmd, "-enable-fips"); if (qemuBuildMachineArgStr(cmd, def, qemuCaps) < 0) goto error; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index 73a561d..c3ae814 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -112,4 +112,5 @@ <flag name='usb-storage'/> <flag name='usb-storage.removable'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index da15d8b..7f629c8 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -126,4 +126,5 @@ <flag name='usb-storage'/> <flag name='usb-storage.removable'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index c419068..1111f42 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -127,4 +127,5 @@ <flag name='usb-storage.removable'/> <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 2b00449..a154bf6 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -132,4 +132,5 @@ <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> <flag name='boot-strict'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 7bce4aa..3ccfb4f 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -136,4 +136,5 @@ <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> <flag name='boot-strict'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index bfaab9d..244985f 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -135,4 +135,5 @@ <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> <flag name='boot-strict'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index d912171..a86e070 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -192,6 +192,12 @@ testQemuCaps(const void *opaque) qemuMonitorTestGetMonitor(mon)) < 0) goto cleanup; + /* Unfortunately, qemu made '-enable-fips' a Linux-only option, + * with no way to probe it via QMP. But as our data files aren't + * set up to use #ifdef, we fake that the option is always present + * if we can probe QMP, even if not on Linux. */ + virQEMUCapsSet(capsComputed, QEMU_CAPS_ENABLE_FIPS); + if (testQemuCapsCompare(capsProvided, capsComputed) < 0) goto cleanup;