On Fri, Jun 19, 2020 at 01:56:55PM +0200, Ján Tomko wrote:
> On a Friday in 2020, Daniel P. Berrangé wrote:
> > The concept we're really testing for is whether QEMU supports
> > the seccomp syscall filter groups. We need to keep one place
> > using the old term to deal with upgrades from existing hosts
> > with running VMs.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> > ---
> > src/qemu/qemu.conf | 2 +-
> > src/qemu/qemu_capabilities.c | 4 ++--
> > src/qemu/qemu_capabilities.h | 2 +-
> > src/qemu/qemu_command.c | 4 ++--
> > src/qemu/qemu_domain.c | 10 +++++++---
> > tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 2 +-
> > tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 2 +-
> > tests/qemustatusxml2xmldata/backup-pull-in.xml | 2 +-
> > tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml | 2 +-
> > tests/qemuxml2argvtest.c | 2 +-
> > 37 files changed, 45 insertions(+), 41 deletions(-)
> >
> > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> > index f89dbd2c3a..99b9ce53e5 100644
> > --- a/src/qemu/qemu.conf
> > +++ b/src/qemu/qemu.conf
> > @@ -704,7 +704,7 @@
> > # If it is unset (or -1), then seccomp will be enabled
> > # only if QEMU >= 2.11.0 is detected, otherwise it is
> > # left disabled. This ensures the default config gets
> > -# protection for new QEMU using the blacklist approach.
> > +# protection for new QEMU with filter groups.
> > #
> > #seccomp_sandbox = 1
> >
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 68fcbd3c4f..310be800e2 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -468,7 +468,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
> > /* 285 */
> > "qcow2-luks",
> > "pcie-pci-bridge",
> > - "seccomp-blacklist",
> > + "seccomp-filter-groups",
> > "query-cpus-fast",
> > "disk-write-cache",
> >
> > @@ -3292,7 +3292,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
> > { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS },
> > { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT },
> > { "sandbox", "enable", QEMU_CAPS_SECCOMP_SANDBOX },
> > - { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST },
> > + { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_FILTER_GROUPS },
> > { "chardev", "fd", QEMU_CAPS_CHARDEV_FD_PASS },
> > { "overcommit", NULL, QEMU_CAPS_OVERCOMMIT },
> > { "smp-opts", "dies", QEMU_CAPS_SMP_DIES },
> > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> > index ad93816d41..0ee3e357cb 100644
> > --- a/src/qemu/qemu_capabilities.h
> > +++ b/src/qemu/qemu_capabilities.h
> > @@ -448,7 +448,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
> > /* 285 */
> > QEMU_CAPS_QCOW2_LUKS, /* qcow2 format support LUKS encryption */
> > QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE, /* -device pcie-pci-bridge */
> > - QEMU_CAPS_SECCOMP_BLACKLIST, /* -sandbox.elevateprivileges */
> > + QEMU_CAPS_SECCOMP_FILTER_GROUPS, /* -sandbox.elevateprivileges */
> > QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */
> > QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache param */
> >
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index f27246b4c6..37113a433a 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -9517,8 +9517,8 @@ qemuBuildSeccompSandboxCommandLine(virCommandPtr cmd,
> > return 0;
> > }
> >
> > - /* Use blacklist by default if supported */
> > - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_BLACKLIST)) {
> > + /* Block undesirable syscall groups by default if supported */
> > + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_FILTER_GROUPS)) {
>
> While 'filter groups' describes the underlying QEMU functionality
> better, we only use it to deny syscalls. So using 'blocklist' as
> proposed in the RFC you linked would better show the contrast between
> this and the old approach.
I don't want to name it based on libvirt's /current/ usage, as we
could alter that usage in future, hence naming it based on the QEMU
conceptual feature.
> > virCommandAddArgList(cmd, "-sandbox",
> > "on,obsolete=deny,elevateprivileges=deny,"
> > "spawn=deny,resourcecontrol=deny",
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 72874ee4fd..56ec5c0352 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -3851,9 +3851,13 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
> > if (str) {
> > int flag = virQEMUCapsTypeFromString(str);
> > if (flag < 0) {
> > - virReportError(VIR_ERR_INTERNAL_ERROR,
> > - _("Unknown qemu capabilities flag %s"), str);
> > - goto error;
> > + if (g_str_equal(str, "seccomp-blacklist")) {
> > + flag = QEMU_CAPS_SECCOMP_FILTER_GROUPS;
>
> I'd just leave the XML as-is, to avoid introducing this special-casing.
Renaming the capability lets us eliminate this from all the capabilities
test data files we have (and the ones we cointinue to add in future), so
I think it is a net win to just have this 2 line special case.
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 :|
