Ping: any review comments from QGA maintainers ?
On Tue, Jun 04, 2024 at 04:32:28PM +0100, Daniel P. Berrangé wrote:
> The QGA supports dynamically filtering what commands are enabled via a
> combination of allow lists and deny lists. This is very flexible, but
> at the same time very fragile.
>
> Consider that a user wants to block all commands that allow unrestricted
> file access/command execution, so they set the deny list when starting
> QGA. Now their OS vendor issues a software update which includes a new
> version of QGA. This new QGA version is liable to contain new commands,
> some of which might undermine the intent of the user's configured deny
> list.
>
> IOW, the generic deny list functionality is inherently dangerous as a
> mechanism for limiting risk exposure.
>
> Using an allow list is much safer, but means on every update the user
> should check the list of new commands to decide which are safe or not,
> putting a burden on every user.
>
> In the context of RHEL, there has been a long term deny list that blocks
> use of guest-file and guest-exec commands, since they give unrestricted
> access to the guest.
>
> With the advent of confidential computing, a far greater number of QGA
> commands are very unsafe to permit, and it is unreasonable to expect
> each user and/or downstream vendor to repeat the work to figure out
> what commands are OK.
>
> This is a similar problem seen in the "seccomp" world where new syscalls
> appear frequently and users can't be expected to understand all of them.
> Systemd pioneered the approach of defining "profiles" which group
> together sets of syscalls, which we subsequently copied in QEMU.
>
> This series applies this same conceptual idea to QGA command filtering,
> making use of the QAPI "features" facility to associate commands into
> one or more groups.
>
> This grouping is then exposed via some new higher level command line
> arguments.
>
> * --no-unrestricted / -u
>
> A flag to block all the guest-file and guest-exec commands
>
> This replicates the policy RHEL currently defines via a deny list.
>
> * --no-user-auth / -e
>
> A flag to block all the commands for manipulating user account
> authentication credentials.
>
> * --confidential / -i
>
> A flag to block all commands, except for those which have been
> explicitly marked as not violating guest owner data privacy
>
> This feature mechanism is further utilized internally to track the
> commands which are safe to use while FS are frozen.
>
> A key benefit of using the QAPI "features" facility is that these
> groupings are visible in the documentation of the QGA commands.
>
> By using these high level command lines arguments, deployments will
> be safe wrt software upgrades, as long as QEMU maintainers apply
> appropriate tags to any new commands.
>
> The allow/deny list command line flags can still be used to further
> refine the command lines, but ideally that would be rare.
>
> A missing piece in this series is getting the --confidential flag to
> be automatically passed to QGA when running in a confidential VM. This
> is something that will likely be done via systemd unit files. My thought
> is that the existing 'qemu-guest-agent.service' would get a parameter
>
> ConditionSecurity=!cvm
>
> while a new qemu-guest-agent-confidential.service' would have the same
> content but with ConditionSecurity=cvm instead, and would pass the
> --confidential flag.
>
> This series depends on the one I sent earlier:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2024-06/msg00743.html
>
> Daniel P. Berrangé (14):
> qapi: use "QAPI_FEATURE" as namespace for special features
> qapi: add helper for checking if a command feature is set
> qapi: cope with special feature names containing a '-'
> qapi: add a 'command-features' pragma
> qapi: stop hardcoding list of special features
> qapi: define enum for custom special features on commands
> qga: use special feature to mark those that can run when FS are frozen
> qga: add command line to limit commands for confidential guests
> qga: define commands which can be run in confidential mode
> qga: add command line to block unrestricted command/file access
> qga: mark guest-file-* commands with 'unrestricted' flag
> qga: mark guest-exec-* commands with 'unrestricted' flag
> qga: add command line to block user authentication commands
> qga: mark guest-ssh-* / guest-*-password commands with 'unrestricted'
> flag
>
> include/qapi/qmp/dispatch.h | 1 +
> include/qapi/util.h | 6 +-
> qapi/qapi-util.c | 4 +-
> qapi/qmp-registry.c | 5 +
> qapi/qobject-output-visitor.c | 4 +-
> qga/main.c | 66 ++++++---
> qga/qapi-schema.json | 248 +++++++++++++++++++++++++++++++---
> scripts/qapi/commands.py | 20 +++
> scripts/qapi/gen.py | 2 +-
> scripts/qapi/parser.py | 2 +
> scripts/qapi/schema.py | 33 ++++-
> scripts/qapi/source.py | 2 +
> 12 files changed, 341 insertions(+), 52 deletions(-)
>
> --
> 2.45.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 :|