| Message ID | 20210908232024.2399215-7-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | security: Introduce qemu_security_policy_taint() API | expand |
On Thu, Sep 09, 2021 at 01:20:20AM +0200, Philippe Mathieu-Daudé wrote: > Add DeviceClass::taints_security_policy field to allow an > unsafe device to eventually taint the global security policy > in DeviceRealize(). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/hw/qdev-core.h | 6 ++++++ > hw/core/qdev.c | 11 +++++++++++ > 2 files changed, 17 insertions(+) > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index bafc311bfa1..ff9ce6671be 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -122,6 +122,12 @@ struct DeviceClass { > */ > bool user_creatable; > bool hotpluggable; > + /* > + * %false if the device is within the QEMU security policy boundary, > + * %true if there is no guarantee this device can be used safely. > + * See: https://www.qemu.org/contribute/security-process/ > + */ > + bool taints_security_policy; > > /* callbacks */ > /* Although your use case is for devices, it probably makes more sense to push this up into the Object base class. I think it will need to be a tri-state value too, not a simple bool. It isn't feasible to mark all devices with this property, so initially we'll have no information about whether most devices are secure or insecure. This patch gives the implication that all devices are secure, except for the few that have been marked otherwise, which is not a good default IMHO. We want to be able to make it clear when introspecting, that we have no information on security available for most devices ie - unset => no information on security (the current default) - true => considered secure against malicious guest - false => considered insecure against malicious guest Then we can also extend 'ObjectTypeInfo' to have a '*secure': 'bool' to make 'qom-list-types' be able to introspect this upfront. > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index cefc5eaa0a9..a5a00f3564c 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -31,6 +31,7 @@ > #include "qapi/qmp/qerror.h" > #include "qapi/visitor.h" > #include "qemu/error-report.h" > +#include "qemu-common.h" > #include "qemu/option.h" > #include "hw/hotplug.h" > #include "hw/irq.h" > @@ -257,6 +258,13 @@ bool qdev_hotplug_allowed(DeviceState *dev, Error **errp) > MachineClass *mc; > Object *m_obj = qdev_get_machine(); > > + if (qemu_security_policy_is_strict() > + && DEVICE_GET_CLASS(dev)->taints_security_policy) { > + error_setg(errp, "Device '%s' can not be hotplugged when" > + " 'strict' security policy is in place", > + object_get_typename(OBJECT(dev))); Do you need a 'return' here to stop execution after reportig the error ? > + } > + > if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { > machine = MACHINE(m_obj); > mc = MACHINE_GET_CLASS(machine); > @@ -385,6 +393,9 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp) > } else { > assert(!DEVICE_GET_CLASS(dev)->bus_type); > } > + qemu_security_policy_taint(DEVICE_GET_CLASS(dev)->taints_security_policy, > + "device type %s", > + object_get_typename(OBJECT(dev))); > > return object_property_set_bool(OBJECT(dev), "realized", true, errp); > } > -- > 2.31.1 > Regards, Daniel
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index bafc311bfa1..ff9ce6671be 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -122,6 +122,12 @@ struct DeviceClass { */ bool user_creatable; bool hotpluggable; + /* + * %false if the device is within the QEMU security policy boundary, + * %true if there is no guarantee this device can be used safely. + * See: https://www.qemu.org/contribute/security-process/ + */ + bool taints_security_policy; /* callbacks */ /* diff --git a/hw/core/qdev.c b/hw/core/qdev.c index cefc5eaa0a9..a5a00f3564c 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -31,6 +31,7 @@ #include "qapi/qmp/qerror.h" #include "qapi/visitor.h" #include "qemu/error-report.h" +#include "qemu-common.h" #include "qemu/option.h" #include "hw/hotplug.h" #include "hw/irq.h" @@ -257,6 +258,13 @@ bool qdev_hotplug_allowed(DeviceState *dev, Error **errp) MachineClass *mc; Object *m_obj = qdev_get_machine(); + if (qemu_security_policy_is_strict() + && DEVICE_GET_CLASS(dev)->taints_security_policy) { + error_setg(errp, "Device '%s' can not be hotplugged when" + " 'strict' security policy is in place", + object_get_typename(OBJECT(dev))); + } + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { machine = MACHINE(m_obj); mc = MACHINE_GET_CLASS(machine); @@ -385,6 +393,9 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp) } else { assert(!DEVICE_GET_CLASS(dev)->bus_type); } + qemu_security_policy_taint(DEVICE_GET_CLASS(dev)->taints_security_policy, + "device type %s", + object_get_typename(OBJECT(dev))); return object_property_set_bool(OBJECT(dev), "realized", true, errp); }
Add DeviceClass::taints_security_policy field to allow an unsafe device to eventually taint the global security policy in DeviceRealize(). Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/qdev-core.h | 6 ++++++ hw/core/qdev.c | 11 +++++++++++ 2 files changed, 17 insertions(+)