[libvirt] [PATCHV2] qemu: ask for -enable-fips when FIPS is required

[Peter Krempa]

On 12/13/13 19:51, Eric Blake wrote:
> 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.  So, if we
> detect FIPS mode, then we unconditionally ask for FIPS; either
> qemu is new enough to have the option and then correctly
> cripple insecure VNC passwords, or it is so old that we are
> correctly avoiding a FIPS violation by preventing qemu from
> starting.  Meanwhile, if we don't detect FIPS mode, then
> omitting the argument is safe whether the qemu has the option
> (but it would do nothing because FIPS is disabled) or whether
> qemu lacks the option (including in the case where we are not
> running on Linux).
> 
> The testsuite was a bit interesting: we don't want our test
> to depend on whether it is being run in FIPS mode, so I had
> to tweak things to set the capability bit outside of our
> normal interaction with capability parsing.
> 
> 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 (virQEMUCapsInitQMP): Conditionally
> set capability according to detection of FIPS mode.
> * src/qemu/qemu_command.c (qemuBuildCommandLine): Use it.
> * tests/qemucapabilitiestest.c (testQemuCaps): Conditionally set
> capability to test expected output.
> * tests/qemucapabilitiesdata/caps_1.2.2-1.caps: Update list.
> * tests/qemucapabilitiesdata/caps_1.6.0-1.caps: Likewise.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/qemu/qemu_capabilities.c                 | 28 +++++++++++++++++++++++++++-
>  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.6.0-1.caps |  1 +
>  tests/qemucapabilitiestest.c                 | 20 +++++++++++++++-----
>  6 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 5e9c65e..9470814 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c

...

> @@ -2630,6 +2631,31 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
>      config.data.nix.path = monpath;
>      config.data.nix.listen = false;
> 
> +    /* Qemu 1.2 and later have a binary flag -enable-fips that must be
> +     * used for VNC auth to obey FIPS settings; but the flag only
> +     * exists on Linux, and with no way to probe for it via QMP.  Our
> +     * solution: if FIPS mode is required, then unconditionally use
> +     * the flag, regardless of qemu version, for the following matrix:
> +     *
> +     *                          old QEMU            new QEMU
> +     * FIPS enabled             doesn't start       VNC auth disabled
> +     * FIPS disabled/missing    VNC auth enabled    VNC auth enabled
> +     *
> +     * Setting the flag here instead of in virQEMUCapsInitQMPMonitor
> +     * or virQEMUCapsInitHelp also allows the testsuite to be
> +     * independent of FIPS setting.
> +     */
> +    if (virFileExists("/proc/sys/crypto/fips_enabled")) {
> +        char buf[sizeof("1\n")];

No need for the above buffer as virFileReadAll actually allocates the
buffer itself.

> +        char *ptr = buf;

This is bogous and will be overwritten, instead ptr should be
initialized to NULL.

> +
> +        if (virFileReadAll("/proc/sys/crypto/fips_enabled",
> +                           sizeof(buf), &ptr) < 0)
> +            goto cleanup;
> +        if (*buf == '1')

This line should compare *ptr instead of *buf which wasn't initialized.
Also I'd rather use STREQ and a bit larger buffer.

> +            virQEMUCapsSet(qemuCaps, QEMU_CAPS_ENABLE_FIPS);
> +    }
> +

You also need to free the pointer allocated by virFileReadAll.

>      VIR_DEBUG("Try to get caps via QMP qemuCaps=%p", qemuCaps);
> 
>      /*

Peter


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131216/93b0c383/attachment-0001.sig>