Re: [PATCH 7/8] qemu-options: document -compat insecure-input parameter

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Oct 29, 2026 at 07:52:36AM +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> 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é <berrange@redhat.com>
>> > ---
>> >  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?