Daniel P. Berrangé <[email protected]> writes: > On Thu, Oct 29, 2026 at 07:52:36AM +0100, Markus Armbruster wrote: >> Daniel P. Berrangé <[email protected]> writes: >> >> > This adds documentation for the previously added 'insecure-input' >> > parameter to -compat, which controls policy for insecure >> > configuration requests. >> > >> > Signed-off-by: Daniel P. Berrangé <[email protected]> >> > --- >> > qemu-options.hx | 22 +++++++++++++++++++++- >> > 1 file changed, 21 insertions(+), 1 deletion(-) >> > >> > diff --git a/qemu-options.hx b/qemu-options.hx >> > index 16d9c6c9072..0487e74bb3e 100644 >> > --- a/qemu-options.hx >> > +++ b/qemu-options.hx >> > @@ -4696,7 +4696,9 @@ DEF("compat", HAS_ARG, QEMU_OPTION_compat, >> > "-compat >> > [deprecated-input=accept|reject|crash|warn][,deprecated-output=accept|hide]\n" >> > " Policy for handling deprecated management >> > interfaces\n" >> > "-compat >> > [unstable-input=accept|reject|crash|warn][,unstable-output=accept|hide]\n" >> > - " Policy for handling unstable management >> > interfaces\n", >> > + " Policy for handling unstable management interfaces\n" >> > + "-compat [insecure-input=accept|reject|crash|warn]\n" >> > + " Policy for handling requests for insecure >> > configuration requests\n", >> > QEMU_ARCH_ALL) >> > SRST >> > ``-compat >> > [deprecated-input=<input-policy>][,deprecated-output=<output-policy>]`` >> > @@ -4734,6 +4736,24 @@ SRST >> > Suppress unstable command results and events >> > >> > Limitation: covers only syntactic aspects of QMP. >> > + >> > +``-compat [insecure-input=<input-policy>] >> > + Set policy for handling requests for insecure configuration >> > + requests: >> > + >> > + ``insecure-input=accept`` (default) >> > + Accept insecure configurations requests silently >> > + ``insecure-input=warn`` >> > + Accept insecure configuration requests with warning on stderr >> > + ``insecure-input=reject`` >> > + Reject insecure configuration requests >> > + ``insecure-input=crash`` >> > + Crash on insecure configuration requests >> > + >> > + Limitation: an insecure configuration is one where QEMU has >> > + not intended to provide a security boundary to protect against >> >> "Where QEMU has not intended to provide" sounds odd to me. Mind, I'm >> not a native speaker. "Where QEMU does not provide". > > Yeah, that language choice was me by pessimistic expecting that > QEMU is full of undiscovered flaws. I'll change to what you > suggested. > >> > + malicious usage. Covers only QOM types for devices, machine types >> > + and accelerators. >> >> The policy check is in >> >> * qemu_create_machine(), which covers machine creation. >> >> * do_configure_accelerator(), which covers accelerator creation. >> >> * qdev_device_add_from_qdict(), which covers *some* device creation: >> >> * device_add and -device always >> >> * Half of -audio model=hda: covers the "intel-hda" device, but not the >> "hda-duplex" device. See intel_hda_and_codec_init(). >> >> * The "primary device" created in failover_hide_primary_device() for >> the virtio-net standby feature. >> >> * The "usb-host" device device created in xen-usb.c for some reason. >> >> There's plenty of device creation it doesn't cover. Isn't this >> problematic? I certainly expected it to cover *all* device creation! > > Yes, eventually, but not yet.... > >> >> Say board code creates a certain onboard device with the usual low >> level interfaces like qdev_new() and qdev_realize(). Say we mark the >> onboard device insecure. The policy check doesn't apply. We need to >> manually mark the machine type insecure as well. Feels awfully prone >> to human error. >> >> Moreover, what if the onboard device is configurable? Then the >> machine type may be secure unless you configure this insecure device. >> Marking the machine type would be too coarse. >> >> Could we check policy in qdev_realize() instead? > > Yes and no. > > To make this a tractable problem I only wanted to add '.secure = true' > annotations for public facing types initially, so needed the check in > the device_add/-device code path only. > > This is sufficient to let us tell bug report filers to take their demo > command line and add > > '-compat insecure-input=reject -nodfaults' > > as a pre-requisite for a bug being considered a security flaw > egligible for CVE handling. > > > If we put a check in qdev_realize(), then we need to add annotations > for *all* types, before the -compat insecure-input=.... option is > viable to use. That requires a much bigger effort to mark all types. > > In the 12.0 dev cycle, my intention is to > > a) Expand ".secure = true" markers to all parents of leaf node types > marked secure, and enforce them. > b) Expand ".secure = true" markers to all built-in devices that are > indirectly created by a leaf node type marked secure, when > -nodefaults is present > > Once (b) is done, then qdev_realize() will likely be the better place > to put the policy check.
Let's see whether I got this. * A type can be known to be secure, known to be insecure, or unknown. * Exactly the types known to be secure are marked secure. Thus, the policy check must treat known to be insecure and unknown the same, which makes sense. * We start with all types unknown. * To move a type from unknown to known, we need to examine it. Examining all types in time for the next release is infeasible. Even examining all types usable with a useful set of machines is infeasible. What is feasible is examining a useful set of types available with certain user interfaces, so that's what we did. * We check the policy in these interfaces only. This lets users restrict these interfaces to known secure types, which is useful. * However, types created differently get a blanket exception. Not ideal, but no worse than before, unless the new policy creates a false sense of security. The best we can (and should) do about that is to clearly document the policy's current limitations. Right?
