All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Thomas Huth <thuth@redhat.com>
Subject: Re: [PATCH <RFC> 00/15] Encode object type security status in code
Date: Tue, 16 Sep 2025 17:43:48 +0100	[thread overview]
Message-ID: <aMmTxNB2eq6hSS9o@redhat.com> (raw)
In-Reply-To: <20250909165726.3814465-1-berrange@redhat.com>

Ping: anyone have thoughts on this idea of annotating security
status of our code against QOM classes ?

On Tue, Sep 09, 2025 at 05:57:11PM +0100, Daniel P. Berrangé wrote:
> Our docs/system/security.rst file loosely classifies code into that
> applicable for 'virtualization' vs 'non-virtualization' use cases.
> Only code relevant to the former group is eligible for security
> bug handling. Peter's recent proposal pointed out that we are
> increasingly hitting the limits of such a crude classification:
> 
>   https://lists.nongnu.org/archive/html/qemu-devel/2025-09/msg01520.html
> 
> Michael suggested that with the increased complexity, docs are not
> going to be an effective way to convey the information, and we
> need to re-consider embedding this info in code:
> 
>   https://lists.nongnu.org/archive/html/qemu-devel/2025-09/msg01566.html
> 
> This also allows users to validate a configuration's security status
> when starting a guest, or modifying a running guest. This series is
> an attempt to start the embedding process.
> 
> It starts with QOM, adding "bool secure" and "bool insecure"
> properties to the TypeInfo struct, which get turned into flags
> on the Type struct. This enables querying any ObjectClass to
> ask whether or not it is declared secure or insecure.
> 
> By default no statement will be made about whether a class is
> secure or insecure, reflecting our historical defaults. Over
> time we should annotate as many classes as possible with an
> explicit statement.
> 
> The "-machine" argument gains two new parameters
> 
>   * prohibit-insecure=yes|no  - a weak security boundary, only
>     excluding stuff that is explicitly declared insecure,
>     permiting stuff that is secure & anything without a stetement
> 
>   * require-secure=yes|no - a strong security boundary, only
>     permitting stuff that is explicitly declared secure,
>     excluding insecure stuff & anything without a statement
> 
> As illustration, I have added explicit annotations for many machine
> types, some accelerators, all NICs (all insecure except xen,
> e1000(e) and virtio), and all PCI virtio devices (all secure).
> 
> Example: TCG is explicitly insecure, KVM is explicitly secure,
>          qtest has no statement:
> 
>   $ qemu-system-x86_64 -display none -machine pc,prohibit-insecure=yes -accel tcg
>   qemu-system-x86_64: Type 'tcg-accel' is declared as insecure
> 
>   $ qemu-system-x86_64 -display none -machine pc,require-secure=yes -accel tcg
>   qemu-system-x86_64: Type 'tcg-accel' is not declared as secure
> 
>   $ qemu-system-x86_64 -display none -machine pc,prohibit-insecure=yes -accel kvm
>   ^Cqemu-system-x86_64: terminating on signal 2
> 
>   $ qemu-system-x86_64 -display none -machine pc,require-secure=yes -accel kvm
>   ^Cqemu-system-x86_64: terminating on signal 2
> 
>   $ qemu-system-x86_64 -display none -machine pc,prohibit-insecure=yes -accel qtest
>   ^Cqemu-system-x86_64: terminating on signal 2
> 
>   $ qemu-system-x86_64 -display none -machine pc,require-secure=yes -accel qtest
>   qemu-system-x86_64: Type 'qtest-accel' is not declared as secure
> 
> Example: isapc machine type is explicitly insecure
> 
>   $ qemu-system-x86_64 -display none -machine isapc,require-secure=yes -accel kvm
>   qemu-system-x86_64: Type 'isapc-machine' is not declared as secure
> 
> Example: devices which have no security statement are allowed if
>          merely excluding insecure devices:
> 
>   $ qemu-system-x86_64 -display none -machine pc,prohibit-insecure=yes -accel kvm -device i6300esb
>   ^Cqemu-system-x86_64: terminating on signal 2
> 
> Example: devices which have no security statement are rejected if
>          requiring explicit security:
> 
>   $ qemu-system-x86_64 -display none -machine pc,require-secure=yes -accel kvm -device i6300esb
>   qemu-system-x86_64: -device i6300esb: Type 'i6300esb' is not declared as secure
> 
> Example: checks also apply in HMP, rtl8139 is explicitly insecure,
>          virtio is explicitly secure
> 
>   $ qemu-system-x86_64 -display none -machine pc,require-secure=yes -accel kvm -monitor stdio
>   QEMU 10.1.50 monitor - type 'help' for more information
>   (qemu) device_add rtl8139
>   Error: Type 'rtl8139' is not declared as secure
>   (qemu) device_add virtio-net
> 
> Example: checks also apply in QMP:
> 
>   $ ./scripts/qmp/qmp-shell-wrap qemu-system-x86_64 -display none -machine pc,require-secure=yes -accel kvm
>   Welcome to the QMP low-level shell!
>   Connected
>   (QEMU) device_add driver=rtl8139
>   {"error": {"class": "GenericError", "desc": "Type 'rtl8139' is not declared as secure"}}
>   (QEMU) device_add driver=virtio-net
>   {"return": {}}
> 
> Some questions....
> 
>   * Is using '-machine' the right place to express the policy ?
> 
>   * Can we change '-accel help' to report 'secure' / 'insecure'
>     as we did for '-machine help' and '-device help'.
> 
>   * Should we have 'query-devices' for QMP to allow the 'secure'
>     or 'insecure' status to be queried for every device.
> 
>   * Should we have 'query-accel' for QMP to allow the 'secure'
>     or 'insecure' status to be queried for every accelerator.
> 
>   * Should we enforce checks for -object & object_add too ?
>     Easy to add code for this, but do we need the ability to
>     exclude some object backends of dubious code quality ?
> 
>   * Likewise for -chardev / -netdev / etc which are
>     conceptual specializations of -object
> 
>   * BlockDriver structs don't use QOM, so we can't mark
>     'vvfat' block backend as insecure
> 
> The first one about '-machine' is probably the main blocker
> from a design POV. Other things are just potential future
> incremental work.
> 
> This series has had only 1/2 a day's work / thought put into
> it, hence RFC status. It has been compiled and minimally tested
> with the examples shown above. I have not pushed this through
> CI nor considered tests yet. Still it gives a good illustration
> of what's involved in recording security info in code.
> 
> Daniel P. Berrangé (15):
>   qom: replace 'abstract' with 'flags'
>   qom: add tracking of security state of object types
>   machine: add 'require-secure' and 'prohibit-insecure' properties
>   machine: check security for machine and accelerator types
>   system: report machine security status in help output
>   system: check security of device types
>   system: report device security status in help output
>   hw/core: report secure/insecure status in query-machines
>   accel: mark 'kvm' as secure and 'tcg' as insecure
>   hw/virtio: mark all virtio PCI devices as secure
>   hw: mark x86, s390, ppc, arm versioned machine types as secure
>   hw: declare Xen & microvm machines as secure, isapc as insecure
>   hw/core: declare 'none' machine to be insecure
>   hw/net: mark all NICs as insecure except e1000, e1000e & xen
>   docs: expand security docs with info about secure/insecure markers
> 
>  accel/kvm/kvm-all.c            |  1 +
>  accel/tcg/tcg-all.c            |  1 +
>  docs/system/security.rst       | 41 +++++++++++++++++++++
>  hw/arm/virt.c                  |  1 +
>  hw/arm/xen-pvh.c               |  1 +
>  hw/core/machine-qmp-cmds.c     |  2 ++
>  hw/core/machine.c              | 66 ++++++++++++++++++++++++++++++++++
>  hw/core/null-machine.c         |  2 +-
>  hw/i386/isapc.c                |  2 +-
>  hw/i386/microvm.c              |  1 +
>  hw/i386/pc_piix.c              |  4 +--
>  hw/i386/xen/xen-pvh.c          |  1 +
>  hw/i386/xen/xen_pvdevice.c     |  1 +
>  hw/net/allwinner-sun8i-emac.c  |  1 +
>  hw/net/allwinner_emac.c        |  3 +-
>  hw/net/cadence_gem.c           |  1 +
>  hw/net/can/can_kvaser_pci.c    |  1 +
>  hw/net/can/can_mioe3680_pci.c  |  1 +
>  hw/net/can/can_pcm3680_pci.c   |  1 +
>  hw/net/can/ctucan_pci.c        |  1 +
>  hw/net/can/xlnx-versal-canfd.c |  1 +
>  hw/net/can/xlnx-zynqmp-can.c   |  1 +
>  hw/net/dp8393x.c               |  1 +
>  hw/net/e1000.c                 |  1 +
>  hw/net/e1000e.c                |  1 +
>  hw/net/eepro100.c              |  1 +
>  hw/net/fsl_etsec/etsec.c       |  1 +
>  hw/net/ftgmac100.c             |  1 +
>  hw/net/igb.c                   |  1 +
>  hw/net/igbvf.c                 |  1 +
>  hw/net/imx_fec.c               |  2 ++
>  hw/net/lan9118.c               |  1 +
>  hw/net/lan9118_phy.c           |  1 +
>  hw/net/lance.c                 |  1 +
>  hw/net/lasi_i82596.c           |  1 +
>  hw/net/mcf_fec.c               |  1 +
>  hw/net/msf2-emac.c             |  1 +
>  hw/net/mv88w8618_eth.c         |  1 +
>  hw/net/ne2000-isa.c            |  1 +
>  hw/net/ne2000-pci.c            |  1 +
>  hw/net/npcm7xx_emc.c           |  1 +
>  hw/net/npcm_gmac.c             |  1 +
>  hw/net/npcm_pcs.c              |  1 +
>  hw/net/opencores_eth.c         |  1 +
>  hw/net/pcnet-pci.c             |  1 +
>  hw/net/rocker/rocker.c         |  1 +
>  hw/net/rtl8139.c               |  1 +
>  hw/net/smc91c111.c             |  1 +
>  hw/net/spapr_llan.c            |  1 +
>  hw/net/stellaris_enet.c        |  1 +
>  hw/net/sungem.c                |  1 +
>  hw/net/sunhme.c                |  1 +
>  hw/net/tulip.c                 |  1 +
>  hw/net/virtio-net.c            |  1 +
>  hw/net/vmxnet3.c               |  1 +
>  hw/net/xen_nic.c               |  1 +
>  hw/net/xgmac.c                 |  1 +
>  hw/net/xilinx_axienet.c        |  1 +
>  hw/net/xilinx_ethlite.c        |  1 +
>  hw/ppc/spapr.c                 |  1 +
>  hw/s390x/s390-virtio-ccw.c     |  1 +
>  hw/virtio/virtio-pci.c         |  3 ++
>  hw/xen/xen-pvh-common.c        |  1 +
>  hw/xenpv/xen_machine_pv.c      |  2 +-
>  include/hw/boards.h            | 18 +++++++++-
>  include/hw/i386/pc.h           |  5 ++-
>  include/qom/object.h           | 24 +++++++++++++
>  qapi/machine.json              |  9 ++++-
>  qom/object.c                   | 40 +++++++++++++++++----
>  system/qdev-monitor.c          | 10 ++++++
>  system/vl.c                    |  6 ++--
>  71 files changed, 275 insertions(+), 18 deletions(-)
> 
> -- 
> 2.50.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  parent reply	other threads:[~2025-09-16 16:44 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-09 16:57 [PATCH <RFC> 00/15] Encode object type security status in code Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 01/15] qom: replace 'abstract' with 'flags' Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 02/15] qom: add tracking of security state of object types Daniel P. Berrangé
2025-09-22 21:33   ` Eric Blake
2025-09-09 16:57 ` [PATCH 03/15] machine: add 'require-secure' and 'prohibit-insecure' properties Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 04/15] machine: check security for machine and accelerator types Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 05/15] system: report machine security status in help output Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 06/15] system: check security of device types Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 07/15] system: report device security status in help output Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 08/15] hw/core: report secure/insecure status in query-machines Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 09/15] accel: mark 'kvm' as secure and 'tcg' as insecure Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 10/15] hw/virtio: mark all virtio PCI devices as secure Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 11/15] hw: mark x86, s390, ppc, arm versioned machine types " Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 12/15] hw: declare Xen & microvm machines as secure, isapc as insecure Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 13/15] hw/core: declare 'none' machine to be insecure Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 14/15] hw/net: mark all NICs as insecure except e1000, e1000e & xen Daniel P. Berrangé
2025-09-09 16:57 ` [PATCH 15/15] docs: expand security docs with info about secure/insecure markers Daniel P. Berrangé
2025-09-16 16:43 ` Daniel P. Berrangé [this message]
2025-09-16 16:51   ` [PATCH <RFC> 00/15] Encode object type security status in code Peter Maydell
2025-09-18 11:35 ` Markus Armbruster
2025-09-18 12:29   ` Daniel P. Berrangé
2025-09-18 14:44     ` Markus Armbruster
2025-09-18 14:51       ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aMmTxNB2eq6hSS9o@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.