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(a)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