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@xxxxxxxxxx> > --- > 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list