On 09/18/2014 09:52 AM, Pavel Hrdina wrote:
> We are not detecting the presence of FIPS from QEMU, but from procfs and
> that means it's not QEMU capability. It was decided that we will pass
> this flag to QEMU even if it's not supported by old QEMU binaries.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135431
>
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>
> Note: The original bug is that we are not detecting whether libvirtd
> binary has been updated, we detect that only for QEMU binary. So you
> could update libvirt without updating QEMU and new capabilities that could
> already exists in QEMU, but was recently implemented in libvirt wasn't
> enabled. I'll post a patch to fix this bug.
>
> src/qemu/qemu_capabilities.c | 24 ------------------------
> src/qemu/qemu_command.c | 25 +++++++++++++++++++++++--
> 2 files changed, 23 insertions(+), 26 deletions(-)
>
I agree with moving the detection of the bit out of cached capability
bit and delaying it instead to qemu startup time (although it means a
stat() call for every qemu spawned, instead of the former behavior of
checking only once). I also agree with the way you removed setting the
bit in qemu_capabilities.c but not the bit itself (the bit is still
defined and named from qemu_capabilities.h, so that if we upgrade
libvirtd and parse the XML for a running qemu domain that was started
from earlier libvirt, we don't mis-behave when seeing that capability
bit, even if we no longer use it for anything).
However, I still think you need a v2:
> +++ b/src/qemu/qemu_command.c
> @@ -7656,8 +7656,29 @@ qemuBuildCommandLine(virConnectPtr conn,
> if (!standalone)
> virCommandAddArg(cmd, "-S"); /* freeze CPU */
>
> - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_FIPS))
> - virCommandAddArg(cmd, "-enable-fips");
> + /* 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")) {
Ouch. This will make our testsuite differ based on whether it is run on
Linux in FIPS mode (where FIPS might exist) or on any other setup. I
think you need to hoist the check for virFileExists() to the caller, and
pass in the result as a new bool parameter to this function, so that the
testsuite has full control over the boolean without regards to the
current system's level of FIPS support.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140918/77d49586/attachment-0001.sig>