3:54 p.m.
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(a)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
4:21 p.m.
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(a)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
8:58 a.m.
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(a)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
9:06 a.m.
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
9:15 a.m.
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 :|
9:22 a.m.
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
10:19 a.m.
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
10:15 a.m.
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
10:22 a.m.
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
10:26 a.m.
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 :|
10:32 a.m.
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
10:33 a.m.
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
10:36 a.m.
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 :|
10:26 a.m.
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
10:09 a.m.
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
3969
days inactive
3977
days old
14 comments
5 participants
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark -
Michal Privoznik -
Paolo Bonzini