| Message ID | 20210908232024.2399215-1-philmd@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | security: Introduce qemu_security_policy_taint() API | expand |
On Thu, Sep 09, 2021 at 01:20:14AM +0200, Philippe Mathieu-Daudé wrote: > Hi, > > This series is experimental! The goal is to better limit the > boundary of what code is considerated security critical, and > what is less critical (but still important!). > > This approach was quickly discussed few months ago with Markus > then Daniel. Instead of classifying the code on a file path > basis (see [1]), we insert (runtime) hints into the code > (which survive code movement). Offending unsafe code can > taint the global security policy. By default this policy > is 'none': the current behavior. It can be changed on the > command line to 'warn' to display warnings, and to 'strict' > to prohibit QEMU running with a tainted policy. Ok, so I infer that you based this idea on the "--compat-policy" arg used to control behaviour wrt to deprecations. With the deprecation support, the QAPI introspection data can report deprecations for machines / CPUs (and in theory devices later). Libvirt records this deprecation info and can report it to the user before even starting a guest, so they can avoid using a deprecated device in the first place. We also use this info to mark a guest as tainted + deprecation at the libvirt level and let mgmt apps query this status. The --compat-policy support has been integrated into libvirt but it is not something we expect real world deployments to use - rather it is targeted as a testing framework. Essentially I see the security reporting as needing to operate in a pretty similar manner. IOW, the reporting via QAPI introspetion is much more important for libvirt and mgmt apps, than any custom cli arg / printfs at the QEMU level. In terms of QAPI design we currently have 'deprecated': 'bool' field against MachineInfo and CpuDefinitionInfo types. it feels like we need 'secure': 'bool' field against the relevant types here too, though I think maybe we might need to make it an optional field to let us distinguish lack of information, since it will take a long time to annotate all areas. eg '*secure': 'bool' - not set => no info available on security characteristics - true => device is considered secure wrt malicious guest - false => device is not considered secure wrt malicious guest > As examples I started implementing unsafe code taint from > 3 different pieces of code: > - accelerators (KVM and Xen in allow-list) > - block drivers (vvfat and parcial null-co in deny-list) > - qdev (hobbyist devices regularly hit by fuzzer) > > I don't want the security researchers to not fuzz QEMU unsafe > areas, but I'd like to make it clearer what the community > priority is (currently 47 opened issues on [3]). Regards, Daniel
On 210909 0120, Philippe Mathieu-Daudé wrote: > Hi, > > This series is experimental! The goal is to better limit the > boundary of what code is considerated security critical, and > what is less critical (but still important!). > > This approach was quickly discussed few months ago with Markus > then Daniel. Instead of classifying the code on a file path > basis (see [1]), we insert (runtime) hints into the code > (which survive code movement). Offending unsafe code can > taint the global security policy. By default this policy > is 'none': the current behavior. It can be changed on the > command line to 'warn' to display warnings, and to 'strict' > to prohibit QEMU running with a tainted policy. > > As examples I started implementing unsafe code taint from > 3 different pieces of code: > - accelerators (KVM and Xen in allow-list) > - block drivers (vvfat and parcial null-co in deny-list) > - qdev (hobbyist devices regularly hit by fuzzer) Just looking through the list of hci, storage, network and graphics devices available on i386 to see which others are potential good candidates for this tag. Obviously a lot of guesswork here: USB devices: name "ich9-usb-ehci1", bus PCI name "ich9-usb-ehci2", bus PCI name "ich9-usb-uhci1", bus PCI name "ich9-usb-uhci2", bus PCI name "ich9-usb-uhci3", bus PCI name "ich9-usb-uhci4", bus PCI name "ich9-usb-uhci5", bus PCI name "ich9-usb-uhci6", bus PCI name "nec-usb-xhci", bus PCI name "pci-ohci", bus PCI, desc "Apple USB Controller" name "piix3-usb-uhci", bus PCI name "piix4-usb-uhci", bus PCI name "qemu-xhci", bus PCI name "usb-ehci", bus PCI Not sure about these. Maybe ohci isn't sensitive? Storage devices: === Sensitive === name "floppy", bus floppy-bus, desc "virtual floppy drive" name "ide-cd", bus IDE, desc "virtual IDE CD-ROM" name "ide-hd", bus IDE, desc "virtual IDE disk" name "isa-fdc", bus ISA, desc "virtual floppy controller" name "isa-ide", bus ISA name "piix3-ide", bus PCI name "piix3-ide-xen", bus PCI name "piix4-ide", bus PCI name "scsi-block", bus SCSI, desc "SCSI block device passthrough" name "scsi-cd", bus SCSI, desc "virtual SCSI CD-ROM" name "scsi-generic", bus SCSI, desc "pass through generic scsi device (/dev/sg*)" name "scsi-hd", bus SCSI, desc "virtual SCSI disk" name "vhost-scsi", bus virtio-bus name "vhost-scsi-pci", bus PCI name "vhost-scsi-pci-non-transitional", bus PCI name "vhost-scsi-pci-transitional", bus PCI name "vhost-user-blk", bus virtio-bus name "vhost-user-blk-pci", bus PCI name "vhost-user-blk-pci-non-transitional", bus PCI name "vhost-user-blk-pci-transitional", bus PCI name "vhost-user-fs-device", bus virtio-bus name "vhost-user-fs-pci", bus PCI name "vhost-user-scsi", bus virtio-bus name "vhost-user-scsi-pci", bus PCI name "vhost-user-scsi-pci-non-transitional", bus PCI name "vhost-user-scsi-pci-transitional", bus PCI name "virtio-9p-device", bus virtio-bus name "virtio-9p-pci", bus PCI, alias "virtio-9p" name "virtio-9p-pci-non-transitional", bus PCI name "virtio-9p-pci-transitional", bus PCI name "virtio-blk-device", bus virtio-bus name "virtio-blk-pci", bus PCI, alias "virtio-blk" name "virtio-blk-pci-non-transitional", bus PCI name "virtio-blk-pci-transitional", bus PCI name "virtio-pmem", bus virtio-bus name "virtio-scsi-device", bus virtio-bus name "virtio-scsi-pci", bus PCI, alias "virtio-scsi" name "virtio-scsi-pci-non-transitional", bus PCI name "virtio-scsi-pci-transitional", bus PCI === Tainting/Not Sensitive === name "am53c974", bus PCI, desc "AMD Am53c974 PCscsi-PCI SCSI adapter" name "dc390", bus PCI, desc "Tekram DC-390 SCSI adapter" name "ich9-ahci", bus PCI, alias "ahci" name "lsi53c810", bus PCI name "lsi53c895a", bus PCI, alias "lsi" name "megasas", bus PCI, desc "LSI MegaRAID SAS 1078" name "megasas-gen2", bus PCI, desc "LSI MegaRAID SAS 2108" name "mptsas1068", bus PCI, desc "LSI SAS 1068" name "nvdimm", desc "DIMM memory module" name "nvme", bus PCI, desc "Non-Volatile Memory Express" name "nvme-ns", bus nvme-bus, desc "Virtual NVMe namespace" name "nvme-subsys", desc "Virtual NVMe subsystem" name "pvscsi", bus PCI name "sd-card", bus sd-bus name "sdhci-pci", bus PCI name "usb-bot", bus usb-bus name "usb-mtp", bus usb-bus, desc "USB Media Transfer Protocol device" name "usb-storage", bus usb-bus name "usb-uas", bus usb-bus Network devices: === Sensitive === name "e1000", bus PCI, alias "e1000-82540em", desc "Intel Gigabit Ethernet" name "e1000e", bus PCI, desc "Intel 82574L GbE Controller" name "virtio-net-device", bus virtio-bus name "virtio-net-pci", bus PCI, alias "virtio-net" name "virtio-net-pci-non-transitional", bus PCI name "virtio-net-pci-transitional", bus PCI === Tainting/Not Sensitive === name "e1000-82544gc", bus PCI, desc "Intel Gigabit Ethernet" name "e1000-82545em", bus PCI, desc "Intel Gigabit Ethernet" name "i82550", bus PCI, desc "Intel i82550 Ethernet" name "i82551", bus PCI, desc "Intel i82551 Ethernet" name "i82557a", bus PCI, desc "Intel i82557A Ethernet" name "i82557b", bus PCI, desc "Intel i82557B Ethernet" name "i82557c", bus PCI, desc "Intel i82557C Ethernet" name "i82558a", bus PCI, desc "Intel i82558A Ethernet" name "i82558b", bus PCI, desc "Intel i82558B Ethernet" name "i82559a", bus PCI, desc "Intel i82559A Ethernet" name "i82559b", bus PCI, desc "Intel i82559B Ethernet" name "i82559c", bus PCI, desc "Intel i82559C Ethernet" name "i82559er", bus PCI, desc "Intel i82559ER Ethernet" name "i82562", bus PCI, desc "Intel i82562 Ethernet" name "i82801", bus PCI, desc "Intel i82801 Ethernet" name "ne2k_isa", bus ISA name "ne2k_pci", bus PCI name "pcnet", bus PCI name "rocker", bus PCI, desc "Rocker Switch" name "rtl8139", bus PCI name "tulip", bus PCI name "usb-net", bus usb-bus name "vmxnet3", bus PCI, desc "VMWare Paravirtualized Ethernet v3" Display devices: === Sensitive === name "isa-vga", bus ISA name "qxl", bus PCI, desc "Spice QXL GPU (secondary)" name "qxl-vga", bus PCI, desc "Spice QXL GPU (primary, vga compatible)" name "vhost-user-gpu", bus virtio-bus name "vhost-user-gpu-pci", bus PCI name "vhost-user-vga", bus PCI name "virtio-gpu-device", bus virtio-bus name "virtio-gpu-pci", bus PCI, alias "virtio-gpu" name "virtio-vga", bus PCI name "VGA", bus PCI === Tainting/Not Sensitive === name "ati-vga", bus PCI name "bochs-display", bus PCI name "cirrus-vga", bus PCI, desc "Cirrus CLGD 54xx VGA" name "isa-cirrus-vga", bus ISA name "ramfb", bus System, desc "ram framebuffer standalone device" name "secondary-vga", bus PCI name "sga", bus ISA, desc "Serial Graphics Adapter" name "vmware-svga", bus PCI Sound devices: === Sensitive === name "hda-duplex", bus HDA, desc "HDA Audio Codec, duplex (line-out, line-in)" name "hda-micro", bus HDA, desc "HDA Audio Codec, duplex (speaker, microphone)" name "hda-output", bus HDA, desc "HDA Audio Codec, output-only (line-out)" name "ich9-intel-hda", bus PCI, desc "Intel HD Audio Controller (ich9)" === Tainting/Not Sensitive === name "AC97", bus PCI, alias "ac97", desc "Intel 82801AA AC97 Audio" name "adlib", bus ISA, desc "Yamaha YM3812 (OPL2)" name "cs4231a", bus ISA, desc "Crystal Semiconductor CS4231A" name "ES1370", bus PCI, alias "es1370", desc "ENSONIQ AudioPCI ES1370" name "gus", bus ISA, desc "Gravis Ultrasound GF1" name "intel-hda", bus PCI, desc "Intel HD Audio Controller (ich6)" name "sb16", bus ISA, desc "Creative Sound Blaster 16" name "usb-audio", bus usb-bus
Hello Philippe, all >On Thursday, 9 September, 2021, 03:58:40 pm IST, Daniel P. Berrangé <berrange@redhat.com> wrote: >On Thu, Sep 09, 2021 at 01:20:14AM +0200, Philippe Mathieu-Daudé wrote: >> This series is experimental! The goal is to better limit the >> boundary of what code is considerated security critical, and >> what is less critical (but still important!). >> >> This approach was quickly discussed few months ago with Markus >> then Daniel. Instead of classifying the code on a file path >> basis (see [1]), we insert (runtime) hints into the code >> (which survive code movement). Offending unsafe code can >> taint the global security policy. By default this policy >> is 'none': the current behavior. It can be changed on the >> command line to 'warn' to display warnings, and to 'strict' >> to prohibit QEMU running with a tainted policy. > * Thanks so much for restarting this thread. I've been at it intermittently last few months, thinking about how could we annotate the source/module objects. -> [*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04642.html * Last time we discussed about having both compile and run time options for users to be able to select the qualified objects/backends/devices as desired. * To confirm: How/Where is the security policy defined? Is it device/module specific OR QEMU project wide? > IOW, the reporting via QAPI introspetion is much more important > for libvirt and mgmt apps, than any custom cli arg / printfs > at the QEMU level. > * True, while it makes sense to have a solution that is conversant with the management/libvirt layers, It'll be great if we could address qemu/cli other use cases too. >it feels like we need > 'secure': 'bool' * Though we started the (above [*]) discussion with '--security' option in mind, I wonder if 'secure' annotation is much specific. And if we could widen its scope. --- x --- Source annotations: I've been thinking over following approaches =================== 1) Segregate the QEMU sources under ../staging/ <= devel/experimental, not for production usage ../src/ <= good for production usage, hence security relevant ../deprecated/ <= Bad for production usage, not security relevant - This is similar to Linux staging drivers' tree. - Staging drivers are not considered for production usage and hence CVE allocation. - At build time by default we only build sources under ../src/ tree. - But we can still have options to build /staging/ and /deprecated/ trees. - It's readily understandable to end users. 2) pkgconfig(1) way: - If we could define per device/backend a configuration (.pc) file which is then used at build/run time to decide which sources are suitable for the build or usage. - I'm trying to experiment with this. 3) We annotate QEMU devices/backends/modules with macros which define its status. It can then be used at build/run times to decide if it's suitable for usage. For ex: $ cat annotsrc.h #include <inttypes.h> enum SRCSTATUS { DEVEL = 0, STAGING, PRODUCTION, DEPRECATED }; uint8_t get_src_status(void); === $ cat libx.c #include <annotsrc.h> #define SRC_STATUS DEPRECATED uint8_t get_src_status(void) { return SRC_STATUS; } === $ cat testlibx.c #include <stdio.h> #include <annotsrc.h> int main (int argc, char *argv[]) { printf("LibX status: %d\n", get_src_status()); return 0; } --- x --- * Approach 3) above is similar to the _security_policy_taint() API, but works at the source/object file level, rather than specific 'struct type' field. * Does adding a field to struct type (ex. DeviceClass) scale to all objects/modules/backends etc? Does it have any limitations to include/cover other sources/objects? * I'd really appreciate any feedback/inputs/suggestions you may have. Thank you. --- -P J P http://feedmug.com
On Tuesday, 14 September, 2021, 07:00:27 pm IST, P J P <pjp@fedoraproject.org> wrote: >* Thanks so much for restarting this thread. I've been at it intermittently last few > months, thinking about how could we annotate the source/module objects. > > -> [*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04642.html > >* Last time we discussed about having both compile and run time options for users > to be able to select the qualified objects/backends/devices as desired. > >* To confirm: How/Where is the security policy defined? Is it device/module specific OR QEMU project wide? > >>> it feels like we need >> 'secure': 'bool' > >* Though we started the (above [*]) discussion with '--security' option in mind, > I wonder if 'secure' annotation is much specific. And if we could widen its scope. > > >Source annotations: I've been thinking over following approaches >=================== > >1) Segregate the QEMU sources under > > ../staging/ <= devel/experimental, not for production usage > ../src/ <= good for production usage, hence security relevant > ../deprecated/ <= Bad for production usage, not security relevant > > - This is similar to Linux staging drivers' tree. > - Staging drivers are not considered for production usage and hence CVE allocation. > - At build time by default we only build sources under ../src/ tree. > - But we can still have options to build /staging/ and /deprecated/ trees. > - It's readily understandable to end users. > >2) pkgconfig(1) way: > - If we could define per device/backend a configuration (.pc) file which is then used > at build/run time to decide which sources are suitable for the build or usage. > > - I'm trying to experiment with this. > >3) We annotate QEMU devices/backends/modules with macros which define its status. > It can then be used at build/run times to decide if it's suitable for usage. > For ex: > > $ cat annotsrc.h > > #include <inttypes.h> > > enum SRCSTATUS { > DEVEL = 0, > STAGING, > PRODUCTION, > DEPRECATED > }; > ... > > >* Approach 3) above is similar to the _security_policy_taint() API, > but works at the source/object file level, rather than specific 'struct type' field. > >* Does adding a field to struct type (ex. DeviceClass) scale to all objects/modules/backends etc? > Does it have any limitations to include/cover other sources/objects? > >* I'd really appreciate your feedback/inputs/suggestions. Ping...!? --- -P J P http://feedmug.com
On Tue, Sep 14, 2021 at 01:30:27PM +0000, P J P wrote: > Hello Philippe, all > > >On Thursday, 9 September, 2021, 03:58:40 pm IST, Daniel P. Berrangé <berrange@redhat.com> wrote: > >On Thu, Sep 09, 2021 at 01:20:14AM +0200, Philippe Mathieu-Daudé wrote: > >> This series is experimental! The goal is to better limit the > >> boundary of what code is considerated security critical, and > >> what is less critical (but still important!). > >> > >> This approach was quickly discussed few months ago with Markus > >> then Daniel. Instead of classifying the code on a file path > >> basis (see [1]), we insert (runtime) hints into the code > >> (which survive code movement). Offending unsafe code can > >> taint the global security policy. By default this policy > >> is 'none': the current behavior. It can be changed on the > >> command line to 'warn' to display warnings, and to 'strict' > >> to prohibit QEMU running with a tainted policy. > > > > * Thanks so much for restarting this thread. I've been at it intermittently last few > months, thinking about how could we annotate the source/module objects. > > -> [*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04642.html > > * Last time we discussed about having both compile and run time options for users > to be able to select the qualified objects/backends/devices as desired. Right, we have multiple different use cases here - People building QEMU who want to cut down what they ship to minimize the stuff they support, which is outside the security guarantee. This can be OS distros, but also any other consumer of QEMU eg. RHEL wants to cut out almost all non-virtualization stuff. There is a quirk here though, that RHEL still includes TCG which is considered outside the security guarantee. So a simple build time "--secure on|off" doesn't do the job on its own. We need something to let people understand the consequences of the build options they are enabling. NB, when I talk of build options, I include both configure/meson args, and also the CONFIG_* options set in configs/**/*.mak - Application developers want to check that they're not using stuff outside the security guarantee, even if the distro has enable it. These need to be able to query whether the VM they've launched has undesirable configuration choices. Some people fall into both groups, some people fall into neither group. > * To confirm: How/Where is the security policy defined? Is it > device/module specific OR QEMU project wide? Currently our only definition is in the docs https://qemu-project.gitlab.io/qemu/system/security.html#security-requirements Philippe's patch is proposing tagging against internal QEMU objects of various types. I further proposed that we expose this in QMP so it is introspectable. I think there's scope for doing stuff at build time with configure args and *mak CONFIG_* options, but haven't thought what that might look like. > > IOW, the reporting via QAPI introspetion is much more important > > for libvirt and mgmt apps, than any custom cli arg / printfs > > at the QEMU level. > > > > * True, while it makes sense to have a solution that is conversant with > the management/libvirt layers, It'll be great if we could address qemu/cli > other use cases too. > > >it feels like we need > > 'secure': 'bool' > > * Though we started the (above [*]) discussion with '--security' option in mind, > I wonder if 'secure' annotation is much specific. And if we could widen its scope. > --- x --- > > > Source annotations: I've been thinking over following approaches > =================== > > 1) Segregate the QEMU sources under > > ../staging/ <= devel/experimental, not for production usage > ../src/ <= good for production usage, hence security relevant > ../deprecated/ <= Bad for production usage, not security relevant > > - This is similar to Linux staging drivers' tree. > - Staging drivers are not considered for production usage and hence CVE allocation. > - At build time by default we only build sources under ../src/ tree. > - But we can still have options to build /staging/ and /deprecated/ trees. > - It's readily understandable to end users. I don't think we should be working in terms of source files at all. Some files contain multiple distinct bits of functionality that are not easily separated, and will have distinct security levels. Also IMHO it is unpleasant to be moving files around in git to when code switches between levels. Also there are distinct criteria here, both security levels, and support levels - there can be stuff which is fully supported but considered insecure, and stuff that is deprecated but considered secure. > 2) pkgconfig(1) way: > - If we could define per device/backend a configuration (.pc) file which is then used > at build/run time to decide which sources are suitable for the build or usage. > > - I'm trying to experiment with this. For build time configuration, we have a pretty clear set of toggles between the configure/meson options, and the CONFIG_* make options. I don't think we need to complicate things by trying to add pkg-config into the mix here. > 3) We annotate QEMU devices/backends/modules with macros which define its status. > It can then be used at build/run times to decide if it's suitable for usage. What is what Philippe's patches are doing in essence Regards, Daniel