Re: [PATCH] os: deprecate the -enable-fips option and QEMU's FIPS enforcement

[Thomas Huth]

On 20/10/2020 18.22, Daniel P. Berrangé wrote:
> The -enable-fips option was added a long time ago to prevent the use of
> single DES when VNC when FIPS mode is enabled. It should never have been
> added, because apps are supposed to unconditionally honour FIPS mode
> based on the '/proc/sys/crypto/fips_enabled' file contents.
> 
> In addition there is more to achieving FIPS compliance than merely
> blocking use of certain algorithms. Those algorithms which are used
> need to perform self-tests at runtime.
> 
> QEMU's built-in cryptography provider has no support for self-tests,
> and neither does the nettle library.
> 
> If QEMU is required to be used in a FIPS enabled host, then it must be
> built with the libgcrypt library enabled, which will unconditionally
> enforce FIPS compliance in any algorithm usage.
> 
> Thus there is no need to keep either the -enable-fips option in QEMU, or
> QEMU's internal FIPS checking methods.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  docs/system/deprecated.rst | 11 +++++++++++
>  os-posix.c                 |  3 +++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 905628f3a0..faa2bc49b1 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -158,6 +158,17 @@ devices.  It is possible to use drives the board doesn't 
> pick up with
>  -device.  This usage is now deprecated.  Use ``if=none`` instead.
>  
>  

Nit: The two empty lines should be below the new entry (i.e. before the
"QMP" title below), not before it.

> +``--enable-fips`` (since 5.2)
> +
> +This option restricts usage of certain cryptographic algorithms when
> +the host is operating in FIPS mode.
> +
> +If FIPS compliance is required, QEMU should be built with the ``libgcrypt``
> +library enabled as a cryptography provider.
> +
> +Neither the ``nettle`` library, or the built-in cryptography provider are
> +supported on FIPS enabled hosts.
> +
>  QEMU Machine Protocol (QMP) commands
>  ------------------------------------
> diff --git a/os-posix.c b/os-posix.c
> index 1de2839554..a6846f51c1 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -153,6 +153,9 @@ int os_parse_cmd_args(int index, const char *optarg)
>          break;
>  #if defined(CONFIG_LINUX)
>      case QEMU_OPTION_enablefips:
> +        warn_report("-enable-fips is deprecated, please build QEMU with "
> +                    "the `libgcrypt` library as the cryptography provider "
> +                    "to enable FIPS compliance");
>          fips_set_state(true);
>          break;
>  #endif

With the nit fixed:
Reviewed-by: Thomas Huth <thuth@redhat.com>