Use of the -enable-fips option is being deprecated in QEMU >= 5.2.0. If
FIPS compliance is required, QEMU must be built with libcrypt which will
unconditionally enforce it.

Thus there is no need for libvirt to pass -enable-fips to modern QEMU.
Unfortunately there was never any way to probe for -enable-fips in the
first instance, it was enabled by libvirt based on version number
originally, and then later unconditionally enabled when libvirt dropped
support for older QEMU. Similarly we now use a version number check to
decide when to stop passing -enable-fips.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/qemu/qemu_capabilities.c |  8 ++++++++
 src/qemu/qemu_capabilities.h |  1 +
 src/qemu/qemu_command.c      | 12 +++++++++++-
 src/qemu/qemu_command.h      |  2 +-
 src/qemu/qemu_driver.c       |  2 +-
 src/qemu/qemu_process.c      |  2 +-
 6 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 81d9ecd886..b4271cd863 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -601,6 +601,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
               /* 380 */
               "usb-host.hostdevice",
               "virtio-balloon.free-page-reporting",
+              "fips-implied",
     );
 
 
@@ -5151,6 +5152,13 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps)
     /* TCG couldn't be disabled nor queried until QEMU 2.10 */
     if (qemuCaps->version < 2010000)
         virQEMUCapsSet(qemuCaps, QEMU_CAPS_TCG);
+
+    /* -enable-fips is deprecated in QEMU 5.2.0, and QEMU
+     * should be built with gcrypt to achieve FIPS compliance
+     * automatically / implicitly
+     */
+    if (qemuCaps->version >= 5002000)
+        virQEMUCapsSet(qemuCaps, QEMU_CAPS_FIPS_IMPLIED);
 }
 
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 44c45589f0..2976879fa3 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -581,6 +581,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
     /* 380 */
     QEMU_CAPS_USB_HOST_HOSTDEVICE, /* -device usb-host.hostdevice */
     QEMU_CAPS_VIRTIO_BALLOON_FREE_PAGE_REPORTING, /*virtio balloon free-page-reporting */
+    QEMU_CAPS_FIPS_IMPLIED, /* -enable-fips is no longer required, delegate to gcrypt */
 
     QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 697a2db62b..a8cb608c28 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1089,10 +1089,20 @@ qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk)
  *                          old QEMU            new QEMU
  * FIPS enabled             doesn't start       VNC auth disabled
  * FIPS disabled/missing    VNC auth enabled    VNC auth enabled
+ *
+ * In QEMU 5.2.0, use of -enable-fips was deprecated. In scenarios
+ * where FIPS is required, QEMU must be built against libgcrypt
+ * which automatically enforces FIPS compliance.
  */
 bool
-qemuCheckFips(void)
+qemuCheckFips(virDomainObjPtr vm)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    virQEMUCapsPtr qemuCaps = priv->qemuCaps;
+
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FIPS_IMPLIED))
+        return false;
+
     if (virFileExists("/proc/sys/crypto/fips_enabled")) {
         g_autofree char *buf = NULL;
 
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 8a30f2852c..8d46c65fcc 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -215,7 +215,7 @@ qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk);
 
 
 bool
-qemuCheckFips(void);
+qemuCheckFips(virDomainObjPtr vm);
 
 virJSONValuePtr qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu)
     ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 825bdd9119..53e4b9d085 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6452,7 +6452,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn,
     }
 
     if (!(cmd = qemuProcessCreatePretendCmd(driver, vm, NULL,
-                                            qemuCheckFips(), true, false,
+                                            qemuCheckFips(vm), true, false,
                                             VIR_QEMU_PROCESS_START_COLD)))
         goto cleanup;
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5bc76a75e3..db5d834b7c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6820,7 +6820,7 @@ qemuProcessLaunch(virConnectPtr conn,
                                      incoming ? incoming->launchURI : NULL,
                                      snapshot, vmop,
                                      false,
-                                     qemuCheckFips(),
+                                     qemuCheckFips(vm),
                                      &nnicindexes, &nicindexes, 0)))
         goto cleanup;
 
-- 
2.26.2

On Tue, Oct 20, 2020 at 17:48:59 +0100, Daniel Berrange wrote:
> Use of the -enable-fips option is being deprecated in QEMU >= 5.2.0. If
> FIPS compliance is required, QEMU must be built with libcrypt which will
> unconditionally enforce it.
> 
> Thus there is no need for libvirt to pass -enable-fips to modern QEMU.
> Unfortunately there was never any way to probe for -enable-fips in the
> first instance, it was enabled by libvirt based on version number
> originally, and then later unconditionally enabled when libvirt dropped
> support for older QEMU. Similarly we now use a version number check to
> decide when to stop passing -enable-fips.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/qemu/qemu_capabilities.c |  8 ++++++++
>  src/qemu/qemu_capabilities.h |  1 +
>  src/qemu/qemu_command.c      | 12 +++++++++++-
>  src/qemu/qemu_command.h      |  2 +-
>  src/qemu/qemu_driver.c       |  2 +-
>  src/qemu/qemu_process.c      |  2 +-
>  6 files changed, 23 insertions(+), 4 deletions(-)

[...]

> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 44c45589f0..2976879fa3 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -581,6 +581,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
>      /* 380 */
>      QEMU_CAPS_USB_HOST_HOSTDEVICE, /* -device usb-host.hostdevice */
>      QEMU_CAPS_VIRTIO_BALLOON_FREE_PAGE_REPORTING, /*virtio balloon free-page-reporting */
> +    QEMU_CAPS_FIPS_IMPLIED, /* -enable-fips is no longer required, delegate to gcrypt */
>  

Another option would be to re-start using QEMU_CAPS_ENABLE_FIPS which is
currently used for questionable testing in qemuxml2argvtest.

Consider this a

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

but I'll try looking at qemuxml2argvtest and it's usage of
QEMU_CAPS_ENABLE_FIPS in a moment to see whether it can be improved and
alternatively even test this change.