[libvirt] [PATCH] qemu: always ask for -enable-fips
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. As far as I can tell, unconditionally using the option when it is available has no negative consequences (the option has no change to qemu behavior except when FIPS is enabled, at which point it cripples insecure VNC passwords which is the one thing that libvirt must not allow when FIPS is active). 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 (virQEMUCapsInitQMPBasic): Conditionally set capability. * src/qemu/qemu_command.c (qemuBuildCommandLine): Use it. * tests/qemucapabilitiestest.c (testQemuCaps): Unconditionally set capability. * tests/qemucapabilitiesdata/caps_1.2.2-1.caps: Update list. * tests/qemucapabilitiesdata/caps_1.3.1-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.4.2-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.5.3-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.6.0-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.6.50-1.caps: Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +++++++ 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.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiestest.c | 6 ++++++ 10 files changed, 22 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a68e555..4d1a961 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -245,6 +245,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "kvm-pit-lost-tick-policy", "boot-strict", /* 160 */ + "enable-fips", ); struct _virQEMUCaps { @@ -2469,6 +2470,12 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT); virQEMUCapsSet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE); virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); +#ifdef __linux__ + /* We always want to honor FIPS mode; but for some strange reason, + * qemu choose to require the use of a command line option, and + * worse, to provide the option only on Linux. */ + virQEMUCapsSet(qemuCaps, QEMU_CAPS_ENABLE_FIPS); +#endif } /* Capabilities that are architecture depending diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index aea64ea..87489ab 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -199,6 +199,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */ QEMU_CAPS_KVM_PIT_TICK_POLICY = 159, /* kvm-pit.lost_tick_policy */ QEMU_CAPS_BOOT_STRICT = 160, /* -boot strict */ + QEMU_CAPS_ENABLE_FIPS = 161, /* -enable-fips */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9539be7..a04725d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7747,6 +7747,8 @@ qemuBuildCommandLine(virConnectPtr conn, } } virCommandAddArg(cmd, "-S"); /* freeze CPU */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_FIPS)) + virCommandAddArg(cmd, "-enable-fips"); if (qemuBuildMachineArgStr(cmd, def, qemuCaps) < 0) goto error; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index 73a561d..c3ae814 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -112,4 +112,5 @@ <flag name='usb-storage'/> <flag name='usb-storage.removable'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index da15d8b..7f629c8 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -126,4 +126,5 @@ <flag name='usb-storage'/> <flag name='usb-storage.removable'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index c419068..1111f42 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -127,4 +127,5 @@ <flag name='usb-storage.removable'/> <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 2b00449..a154bf6 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -132,4 +132,5 @@ <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> <flag name='boot-strict'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 7bce4aa..3ccfb4f 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -136,4 +136,5 @@ <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> <flag name='boot-strict'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index bfaab9d..244985f 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -135,4 +135,5 @@ <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> <flag name='boot-strict'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index d912171..a86e070 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -192,6 +192,12 @@ testQemuCaps(const void *opaque) qemuMonitorTestGetMonitor(mon)) < 0) goto cleanup; + /* Unfortunately, qemu made '-enable-fips' a Linux-only option, + * with no way to probe it via QMP. But as our data files aren't + * set up to use #ifdef, we fake that the option is always present + * if we can probe QMP, even if not on Linux. */ + virQEMUCapsSet(capsComputed, QEMU_CAPS_ENABLE_FIPS); + if (testQemuCapsCompare(capsProvided, capsComputed) < 0) goto cleanup; -- 1.8.3.1
ping On 12/05/2013 02:54 PM, 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. As far as I can tell, unconditionally using the option when it is available has no negative consequences (the option has no change to qemu behavior except when FIPS is enabled, at which point it cripples insecure VNC passwords which is the one thing that libvirt must not allow when FIPS is active).
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 (virQEMUCapsInitQMPBasic): Conditionally set capability. * src/qemu/qemu_command.c (qemuBuildCommandLine): Use it. * tests/qemucapabilitiestest.c (testQemuCaps): Unconditionally set capability. * tests/qemucapabilitiesdata/caps_1.2.2-1.caps: Update list. * tests/qemucapabilitiesdata/caps_1.3.1-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.4.2-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.5.3-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.6.0-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.6.50-1.caps: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +++++++ 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.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiestest.c | 6 ++++++ 10 files changed, 22 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a68e555..4d1a961 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -245,6 +245,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "kvm-pit-lost-tick-policy",
"boot-strict", /* 160 */ + "enable-fips", );
struct _virQEMUCaps { @@ -2469,6 +2470,12 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT); virQEMUCapsSet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE); virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); +#ifdef __linux__ + /* We always want to honor FIPS mode; but for some strange reason, + * qemu choose to require the use of a command line option, and + * worse, to provide the option only on Linux. */ + virQEMUCapsSet(qemuCaps, QEMU_CAPS_ENABLE_FIPS); +#endif }
/* Capabilities that are architecture depending diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index aea64ea..87489ab 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -199,6 +199,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */ QEMU_CAPS_KVM_PIT_TICK_POLICY = 159, /* kvm-pit.lost_tick_policy */ QEMU_CAPS_BOOT_STRICT = 160, /* -boot strict */ + QEMU_CAPS_ENABLE_FIPS = 161, /* -enable-fips */
QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9539be7..a04725d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7747,6 +7747,8 @@ qemuBuildCommandLine(virConnectPtr conn, } } virCommandAddArg(cmd, "-S"); /* freeze CPU */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_FIPS)) + virCommandAddArg(cmd, "-enable-fips");
if (qemuBuildMachineArgStr(cmd, def, qemuCaps) < 0) goto error; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index 73a561d..c3ae814 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -112,4 +112,5 @@ <flag name='usb-storage'/> <flag name='usb-storage.removable'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index da15d8b..7f629c8 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -126,4 +126,5 @@ <flag name='usb-storage'/> <flag name='usb-storage.removable'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index c419068..1111f42 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -127,4 +127,5 @@ <flag name='usb-storage.removable'/> <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 2b00449..a154bf6 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -132,4 +132,5 @@ <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> <flag name='boot-strict'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 7bce4aa..3ccfb4f 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -136,4 +136,5 @@ <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> <flag name='boot-strict'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index bfaab9d..244985f 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -135,4 +135,5 @@ <flag name='ich9-intel-hda'/> <flag name='kvm-pit-lost-tick-policy'/> <flag name='boot-strict'/> + <flag name='enable-fips'/> </qemuCaps> diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index d912171..a86e070 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -192,6 +192,12 @@ testQemuCaps(const void *opaque) qemuMonitorTestGetMonitor(mon)) < 0) goto cleanup;
+ /* Unfortunately, qemu made '-enable-fips' a Linux-only option, + * with no way to probe it via QMP. But as our data files aren't + * set up to use #ifdef, we fake that the option is always present + * if we can probe QMP, even if not on Linux. */ + virQEMUCapsSet(capsComputed, QEMU_CAPS_ENABLE_FIPS); + if (testQemuCapsCompare(capsProvided, capsComputed) < 0) goto cleanup;
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05.12.2013 22:54, 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. As far as I can tell, unconditionally using the option when it is available has no negative consequences (the option has no change to qemu behavior except when FIPS is enabled, at which point it cripples insecure VNC passwords which is the one thing that libvirt must not allow when FIPS is active).
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 (virQEMUCapsInitQMPBasic): Conditionally set capability. * src/qemu/qemu_command.c (qemuBuildCommandLine): Use it. * tests/qemucapabilitiestest.c (testQemuCaps): Unconditionally set capability. * tests/qemucapabilitiesdata/caps_1.2.2-1.caps: Update list. * tests/qemucapabilitiesdata/caps_1.3.1-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.4.2-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.5.3-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.6.0-1.caps: Likewise. * tests/qemucapabilitiesdata/caps_1.6.50-1.caps: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +++++++ 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.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiestest.c | 6 ++++++ 10 files changed, 22 insertions(+)
Sigh, oh boy, <your favorite swear-word>. ACK. Michal
On Fri, Dec 13, 2013 at 15:58:55 +0100, Michal Privoznik wrote:
On 05.12.2013 22:54, 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. As far as I can tell, unconditionally using the option when it is available has no negative consequences (the option has no change to qemu behavior except when FIPS is enabled, at which point it cripples insecure VNC passwords which is the one thing that libvirt must not allow when FIPS is active).
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1035474
Sigh, oh boy, <your favorite swear-word>. ACK.
Don't we want to wait for QEMU to decide what they should be doing with -enable-fips to make it detectable? If we push this patch, we can't basically move into detecting the option and enabling it only when detected since that could cause regressions for older QEMU version that supported the option but did not advertise it. If we just wait for the option to be detectable and enable it only when we detect its support in QEMU, we won't enable it for all possible QEMU versions but we won't regress in any way. Jirka
On Fri, Dec 13, 2013 at 04:06:50PM +0100, Jiri Denemark wrote:
On Fri, Dec 13, 2013 at 15:58:55 +0100, Michal Privoznik wrote:
On 05.12.2013 22:54, 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. As far as I can tell, unconditionally using the option when it is available has no negative consequences (the option has no change to qemu behavior except when FIPS is enabled, at which point it cripples insecure VNC passwords which is the one thing that libvirt must not allow when FIPS is active).
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1035474
Sigh, oh boy, <your favorite swear-word>. ACK.
Don't we want to wait for QEMU to decide what they should be doing with -enable-fips to make it detectable? If we push this patch, we can't basically move into detecting the option and enabling it only when detected since that could cause regressions for older QEMU version that supported the option but did not advertise it. If we just wait for the option to be detectable and enable it only when we detect its support in QEMU, we won't enable it for all possible QEMU versions but we won't regress in any way.
QEMU already detects current FIPs enablement via the file /proc/sys/crypto/fips_enabled, but only if you use --enable-fips. This is really stupid given that all the crypto libraries that QEMU uses unconditonally look at the proc file. So by having this flag QEMU is in the insane situation where if FIPS is enabled then part of QEMU will honour FIPS settings but other parts of QEMU will not honour it until you pass --enable-fips. Insanity. So having libvirt pass --enable-fips unconditionally fixes this insanity as much as possible. Better yet if QEMU were to just remove the pointless --enable-fips arg and just respect the fips_enabled sysctl flag by default. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, Dec 13, 2013 at 15:15:59 +0000, Daniel Berrange wrote:
On Fri, Dec 13, 2013 at 04:06:50PM +0100, Jiri Denemark wrote:
On Fri, Dec 13, 2013 at 15:58:55 +0100, Michal Privoznik wrote:
On 05.12.2013 22:54, 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. As far as I can tell, unconditionally using the option when it is available has no negative consequences (the option has no change to qemu behavior except when FIPS is enabled, at which point it cripples insecure VNC passwords which is the one thing that libvirt must not allow when FIPS is active).
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1035474
Sigh, oh boy, <your favorite swear-word>. ACK.
Don't we want to wait for QEMU to decide what they should be doing with -enable-fips to make it detectable? If we push this patch, we can't basically move into detecting the option and enabling it only when detected since that could cause regressions for older QEMU version that supported the option but did not advertise it. If we just wait for the option to be detectable and enable it only when we detect its support in QEMU, we won't enable it for all possible QEMU versions but we won't regress in any way.
QEMU already detects current FIPs enablement via the file /proc/sys/crypto/fips_enabled, but only if you use --enable-fips. This is really stupid given that all the crypto libraries that QEMU uses unconditonally look at the proc file. So by having this flag QEMU is in the insane situation where if FIPS is enabled then part of QEMU will honour FIPS settings but other parts of QEMU will not honour it until you pass --enable-fips. Insanity. So having libvirt pass --enable-fips unconditionally fixes this insanity as much as possible. Better yet if QEMU were to just remove the pointless --enable-fips arg and just respect the fips_enabled sysctl flag by default.
Of course, I don't question this part. I just don't like the black magic we use to decide whether we can use -enable-fips or not and if we go this black route, we will have to stick with it even if QEMU provides a proper way of detecting -enable-fips. We could only use the detection in case our black magic decides the option is not supported. Jirka
On 12/13/2013 08:22 AM, Jiri Denemark wrote:
QEMU already detects current FIPs enablement via the file /proc/sys/crypto/fips_enabled, but only if you use --enable-fips. This is really stupid given that all the crypto libraries that QEMU uses unconditonally look at the proc file. So by having this flag QEMU is in the insane situation where if FIPS is enabled then part of QEMU will honour FIPS settings but other parts of QEMU will not honour it until you pass --enable-fips. Insanity. So having libvirt pass --enable-fips unconditionally fixes this insanity as much as possible. Better yet if QEMU were to just remove the pointless --enable-fips arg and just respect the fips_enabled sysctl flag by default.
Of course, I don't question this part. I just don't like the black magic we use to decide whether we can use -enable-fips or not and if we go this black route, we will have to stick with it even if QEMU provides a proper way of detecting -enable-fips. We could only use the detection in case our black magic decides the option is not supported.
The only black magic involved is: For any version of qemu that does not report binary flags, repeat the encoding used in those versions of qemu for whether the option exists. That happens to be hard-coded to being compiled for Linux, for versions 1.2 through the present, and that set of versions also happens to be the set of qemu that supports QMP probing (so no -help scraping needed). If future qemu is enhanced to support binary flag probing via QMP, then that will take precedence (and work regardless of platform); where it is only when we can't detect the newer means for binary flag scraping do we have to fall back to our black magic reproduction of qemu behavior. I don't like it either, but I don't see any alternatives. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/13/2013 08:15 AM, Daniel P. Berrange wrote:
QEMU already detects current FIPs enablement via the file /proc/sys/crypto/fips_enabled, but only if you use --enable-fips. This is really stupid given that all the crypto libraries that QEMU uses unconditonally look at the proc file. So by having this flag QEMU is in the insane situation where if FIPS is enabled then part of QEMU will honour FIPS settings but other parts of QEMU will not honour it until you pass --enable-fips. Insanity. So having libvirt pass --enable-fips unconditionally fixes this insanity as much as possible. Better yet if QEMU were to just remove the pointless --enable-fips arg and just respect the fips_enabled sysctl flag by default.
Agreed that qemu's current stance is insane, and that libvirt being forced to deal with it is not the ideal solution. But we've tried to fight the battle of getting qemu to just enable the FIPS check unconditionally (ie. make -enable-fips a no-op, still existing for back-compat reasons, but behaving as if it were always requested), and so far have not had any luck. I'd rather patch libvirt now than wait for a future qemu (especially if it is still contentious to change the qemu behavior). Shall I go ahead and push this libvirt patch? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Il 13/12/2013 16:15, Daniel P. Berrange ha scritto:
QEMU already detects current FIPs enablement via the file /proc/sys/crypto/fips_enabled, but only if you use --enable-fips. This is really stupid given that all the crypto libraries that QEMU uses unconditonally look at the proc file. So by having this flag QEMU is in the insane situation where if FIPS is enabled then part of QEMU will honour FIPS settings but other parts of QEMU will not honour it until you pass --enable-fips. Insanity. So having libvirt pass --enable-fips unconditionally fixes this insanity as much as possible. Better yet if QEMU were to just remove the pointless --enable-fips arg and just respect the fips_enabled sysctl flag by default.
Could libvirt look at /proc/sys/crypto/fips_enabled itself, and pass -enable-fips unconditionally (always: this means rejecting QEMUs that do not support FIPS mode if you're in FIPS mode) if it is enabled? Paolo
On Fri, Dec 13, 2013 at 05:22:30PM +0100, Paolo Bonzini wrote:
Il 13/12/2013 16:15, Daniel P. Berrange ha scritto:
QEMU already detects current FIPs enablement via the file /proc/sys/crypto/fips_enabled, but only if you use --enable-fips. This is really stupid given that all the crypto libraries that QEMU uses unconditonally look at the proc file. So by having this flag QEMU is in the insane situation where if FIPS is enabled then part of QEMU will honour FIPS settings but other parts of QEMU will not honour it until you pass --enable-fips. Insanity. So having libvirt pass --enable-fips unconditionally fixes this insanity as much as possible. Better yet if QEMU were to just remove the pointless --enable-fips arg and just respect the fips_enabled sysctl flag by default.
Could libvirt look at /proc/sys/crypto/fips_enabled itself, and pass -enable-fips unconditionally (always: this means rejecting QEMUs that do not support FIPS mode if you're in FIPS mode) if it is enabled?
QEMU already looks at the /proc file itself - the -enable-fips option is just enabling that bit of checking code. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/13/2013 09:26 AM, Daniel P. Berrange wrote:
Could libvirt look at /proc/sys/crypto/fips_enabled itself, and pass -enable-fips unconditionally (always: this means rejecting QEMUs that do not support FIPS mode if you're in FIPS mode) if it is enabled?
QEMU already looks at the /proc file itself - the -enable-fips option is just enabling that bit of checking code.
But the point is that if libvirt looks at the file as its decision to pass '-enable-fips', then we have the following situations: old qemu (no -enable-fips, but also no checking of /proc), FIPS mode off: libvirt does not pass -enable-fips, and qemu still boots (good, no regression for people who don't care about fips) old qemu, FIPS mode on: libvirt passes -enable-fips, and qemu fails to boot for unrecognized option (good, since that version of qemu violates FIPS) old qemu, file not present: libvirt does not pass -enable-fips, qemu still boots (good, no regression for people on non-Linux) modern qemu (1.2 to present), FIPS mode off or file not present: libvirt does not pass -enable-fips, and qemu still boots (good, no regression for people who don't care about fips) modern qemu, FIPS mode on: libvirt passes -enable-fips, qemu boots and obeys fips (good, passes certification) future qemu (adding an ability to probe for binary commands): libvirt could be changed to take advantage of this, or stick to its existing behavior, but the end result is the same that qemu will always be booted in a FIPS-compliant mode when it matters -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Il 13/12/2013 17:26, Daniel P. Berrange ha scritto:
On Fri, Dec 13, 2013 at 05:22:30PM +0100, Paolo Bonzini wrote:
Il 13/12/2013 16:15, Daniel P. Berrange ha scritto:
QEMU already detects current FIPs enablement via the file /proc/sys/crypto/fips_enabled, but only if you use --enable-fips. This is really stupid given that all the crypto libraries that QEMU uses unconditonally look at the proc file. So by having this flag QEMU is in the insane situation where if FIPS is enabled then part of QEMU will honour FIPS settings but other parts of QEMU will not honour it until you pass --enable-fips. Insanity. So having libvirt pass --enable-fips unconditionally fixes this insanity as much as possible. Better yet if QEMU were to just remove the pointless --enable-fips arg and just respect the fips_enabled sysctl flag by default.
Could libvirt look at /proc/sys/crypto/fips_enabled itself, and pass -enable-fips unconditionally (always: this means rejecting QEMUs that do not support FIPS mode if you're in FIPS mode) if it is enabled?
QEMU already looks at the /proc file itself - the -enable-fips option is just enabling that bit of checking code.
Yes, but we cannot always pass -enable-fips because that would completely break old QEMU that doesn't have the option. If libvirt checks the /proc file too, we have: old QEMU new QEMU FIPS enabled doesn't start VNC auth disabled FIPS disabled VNC auth enabled VNC auth enabled It also has the side effect of passing the option only on Linux, without the ugly #ifdef. Paolo
On Fri, Dec 13, 2013 at 05:33:15PM +0100, Paolo Bonzini wrote:
Il 13/12/2013 17:26, Daniel P. Berrange ha scritto:
On Fri, Dec 13, 2013 at 05:22:30PM +0100, Paolo Bonzini wrote:
Il 13/12/2013 16:15, Daniel P. Berrange ha scritto:
QEMU already detects current FIPs enablement via the file /proc/sys/crypto/fips_enabled, but only if you use --enable-fips. This is really stupid given that all the crypto libraries that QEMU uses unconditonally look at the proc file. So by having this flag QEMU is in the insane situation where if FIPS is enabled then part of QEMU will honour FIPS settings but other parts of QEMU will not honour it until you pass --enable-fips. Insanity. So having libvirt pass --enable-fips unconditionally fixes this insanity as much as possible. Better yet if QEMU were to just remove the pointless --enable-fips arg and just respect the fips_enabled sysctl flag by default.
Could libvirt look at /proc/sys/crypto/fips_enabled itself, and pass -enable-fips unconditionally (always: this means rejecting QEMUs that do not support FIPS mode if you're in FIPS mode) if it is enabled?
QEMU already looks at the /proc file itself - the -enable-fips option is just enabling that bit of checking code.
Yes, but we cannot always pass -enable-fips because that would completely break old QEMU that doesn't have the option.
If libvirt checks the /proc file too, we have:
old QEMU new QEMU FIPS enabled doesn't start VNC auth disabled FIPS disabled VNC auth enabled VNC auth enabled
It also has the side effect of passing the option only on Linux, without the ugly #ifdef.
Oh, I see what you mean. That's a kind of neat idea. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/13/2013 09:22 AM, Paolo Bonzini wrote:
Il 13/12/2013 16:15, Daniel P. Berrange ha scritto:
QEMU already detects current FIPs enablement via the file /proc/sys/crypto/fips_enabled, but only if you use --enable-fips. This is really stupid given that all the crypto libraries that QEMU uses unconditonally look at the proc file. So by having this flag QEMU is in the insane situation where if FIPS is enabled then part of QEMU will honour FIPS settings but other parts of QEMU will not honour it until you pass --enable-fips. Insanity. So having libvirt pass --enable-fips unconditionally fixes this insanity as much as possible. Better yet if QEMU were to just remove the pointless --enable-fips arg and just respect the fips_enabled sysctl flag by default.
Could libvirt look at /proc/sys/crypto/fips_enabled itself, and pass -enable-fips unconditionally (always: this means rejecting QEMUs that do not support FIPS mode if you're in FIPS mode) if it is enabled?
A little less blatant than hard-coding the solution in libvirt to trying the flag if compiled for Linux. I can do a v2 patch along those lines, for comparison, if desired. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/13/2013 08:06 AM, Jiri Denemark wrote:
On Fri, Dec 13, 2013 at 15:58:55 +0100, Michal Privoznik wrote:
On 05.12.2013 22:54, 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. As far as I can tell, unconditionally using the option when it is available has no negative consequences (the option has no change to qemu behavior except when FIPS is enabled, at which point it cripples insecure VNC passwords which is the one thing that libvirt must not allow when FIPS is active).
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1035474
Sigh, oh boy, <your favorite swear-word>. ACK.
Don't we want to wait for QEMU to decide what they should be doing with -enable-fips to make it detectable?
I tried, and got no response: https://lists.gnu.org/archive/html/qemu-devel/2013-12/msg00946.html adding qemu-devel in CC for another try.
If we push this patch, we can't basically move into detecting the option and enabling it only when detected since that could cause regressions for older QEMU version that supported the option but did not advertise it.
Not necessarily. We can code things along these lines: if qemu new enough to provide binary option detection: use detection results, use if present else: if Linux: hard-code on else: assume unavailable
If we just wait for the option to be detectable and enable it only when we detect its support in QEMU, we won't enable it for all possible QEMU versions but we won't regress in any way.
I'm not worried about regressions - if someone backports binary option detection, we'll do the right thing; if they don't, then trying to the option on a build where it is not present will give a nice loud failure from qemu about an unrecognized command line option, which we would get anyways even if binary option detection is not added in qemu and our hard-code guess is wrong. I'd rather have libvirt turn the option on NOW (since it is a FIPS certification nightmare if we don't) than wait for some future qemu to fix binary option detection and hope it gets backported to any version of qemu that libvirt must drive in a FIPS environment. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark -
Michal Privoznik -
Paolo Bonzini