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

[Markus Armbruster <armbru@redhat.com>]

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".

> +    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!

  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?

>  ERST
>  
>  DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg,