Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add usbvm #1442

Merged
merged 16 commits into from
Nov 9, 2023
Merged

Add usbvm #1442

merged 16 commits into from
Nov 9, 2023

Conversation

jandryuk
Copy link
Contributor

OpenXT/vusb-daemon#30
OpenXT/manager#188
OpenXT/xsm-policy#39

This PR adds a new usbvm machine and image. It's off by default. usbvm can be enabled by db-write /xenmgr/usbvm-enable true. Sometimes the USB controller has trouble with PCI passthrough :(

All USB controllers are assigned to the usbvm. vusb-daemon in dom0 gets notified about devices by vusb-daemon in usbvm over xenstore. Only one usbvm is supported at this time, with reserved uuid 00000000-0000-0000-0000-000000000003. (Multiple could be supported by looking for VM "type" "usbvm" and tracking the domid for a given device, but that isn't implemented here.)

USB input devices have input events passed to dom0 by way of qubes-input-proxy over argo. This contains USB processing inside the usbvm. Passing though input devices over pv-usb would still require USB processing in dom0. My first thought was to use a xen-kbdfront in dom0 to receive input events, and that requires something to provide xen-kbdback in usbvm. qemu or input_server were possibilities there, but would have required refactoring to make a standalone component for usbvm. I peeked at how Qubes does it and found qubes-input-proxy which provides the simple components I was looking for.

Also, there are refpolicy changes to split up labeling of xen devices. xenpriv_device_t is for privcmd and hypercalls while xenstore_dev_t is for using xenstore.

The use of the upstream Xen XSM Flask policy is no longer included here. ndvm_t is basically equivalent, so typealiases are added in xsm-policy.git to allow it to be reused.

@jandryuk
Copy link
Contributor Author

a100950 breaks #1434
I expect that will go in first, and this will need an addition of dev_rw_xenpriv(varstored_t)

@jandryuk
Copy link
Contributor Author

jandryuk commented Nov 3, 2021

Made dev_rw_xenpriv(varstored_t) change.
Fixed whitespace
Changed github SRC_URI for qubes-input-proxy to protocol=https

Copy link
Contributor

@crogers1 crogers1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments on this stuff. Most of the changes to xenclient-oe here seem straightforward and other than the argo-exec stuff, I don't think I would have implemented the policy changes or image/recipe changes any differently.

@jandryuk
Copy link
Contributor Author

Thanks for the review, @crogers1

@deepin125
Copy link

Have you looked at this article about RMRR? Not sure if has anything to do with issues with USB PCI PT, but interesting security implications for this type of application.
https://access.redhat.com/sites/default/files/attachments/rmrr-wp1.pdf

@jandryuk
Copy link
Contributor Author

@deepin125 I had not seen that particular document before. Thanks for the pointer
I found this interesting: "Furthermore, since the usage of RMRRs by USB devices is understood, an exemption is provided in the kernel to allow assignment of these devices to continue." Xen does the identity assignment of RMRRs to the guest, and it checks the memory ranges are reserved (by default). So it seems like a reasonable approach.

@deepin125
Copy link

It seems that from the research done at RedHat, they seem to not think that USB controllers pose much of a threat to the integrity of the guest/host. It seems to be expected that they have the RMRR setup probably for legacy ps2 emulation, so as long as its not being used as an attack vector (like the use case of it being connected to a system controller or several devices all using a shared memory space), then you might be able to patch xen to make an exemption.

@jandryuk
Copy link
Contributor Author

@deepin125 Xen doesn't need a patch because it 1-to-1 maps the RMRR ranges when passing through devices with RMRRs. So it doesn't implement the Linux policy of disallowing passthrough of PCI devices with RMRRs except for USB & Graphics.

I think the Red Hat position is for safey. Don't assign a device that could potentially crash the host. They are assuming any VM is an untrusted user one. There, you don't want the VMs to crash a host.

With usbvm being a servicevm, it is not immediately malicious. We are moving the USB stack into a dedicated VM and out of Dom0. It's our kernel and software. Attacks on the USB stack would be in usbvm instead of Dom0. So if an attacker can attack and abuse the RMRR, the other scenario would be in Dom0 which is worse anyway.

@jandryuk
Copy link
Contributor Author

Force pushed with removal of dead code in argo-exec.c and a reformatting of the SRC_URI in argo-exec.bb.

@jandryuk
Copy link
Contributor Author

It's rebased with a 5.15 defconfig. I also pulled in dd43713 which was in #1445 as it mentions fixing a usbvm issue.

@jandryuk jandryuk force-pushed the usbvm-no-xsm-5.10 branch from 64c7857 to 5e47add Compare May 22, 2023 17:15
@jandryuk
Copy link
Contributor Author

Force pushed again to drop the two comment-ed out lines from argo-exec.c (//close(fd_stdin[1]);) which slipped back in when switching to the 5.15 support.

@jandryuk
Copy link
Contributor Author

jandryuk commented Jul 5, 2023

Locally, I checked out master and did git merge usbvm-no-mxs-5.10. The merge put the new defconfig under 6.1, so that looks good. If github does that, then it should be fine. However, it doesn't seem like there is a way to check what github will actually do without just merging.

So merging this might work, and if not, it should just be a small fixup to git mv the file.

@crogers1
Copy link
Contributor

crogers1 commented Nov 3, 2023

@jandryuk this one also needs some merge conflicts resolved, then I can merge.

commit 5af2803 "vusb: Fix initscript shutdown" switch to using
pkill for stopping vusb-daemon.  That is fine for dom0, but usbvm uses
busybox instead of coreutils, and the command wasn't build.  At shutdown
there would be a pkill command not found error.  Build pkill into
busybox so it works.

vusb-daemon shutdown didn't work before commit 5af2803, so the lack
of pkill wasn't changing that.

Signed-off-by: Jason Andryuk <[email protected]>
Replace the three open-codings of the inittab modification by new
IMAGE_FEATURE ctrlaltdel-reboot.  It will bre re-used by usbvm.

Signed-off-by: Jason Andryuk <[email protected]>
usbvm is a 64bit vm to control the PCI USB controllers and provide PV
USB backends.

Signed-off-by: Jason Andryuk <[email protected]>
Add a minimized defconfig for usbvm.  It's designed to run as an HVM.
It has PCI driver for USB controllers and shares USB devices with the
Xen PV USB backend driver.  Actual USB device drivers are limited to USB
HID devices.

SELinux and yama security modules are enabled.

Signed-off-by: Jason Andryuk <[email protected]>
Add fstab & fstab.early for usbvm.

Signed-off-by: Jason Andryuk <[email protected]>
Allow starting the vusb-daemon in stub-mode when the relevant file is
present.  Place that file into its own package.

Signed-off-by: Jason Andryuk <[email protected]>
qubes-input-proxy will proxy the input events from usbvm to dom0.  A
receiver package runs in dom0 and a sender in usbvm.  Dom0 needs
/dev/uinput to inject the input events, but that is already built into
the dom0 kernel.

Signed-off-by: Jason Andryuk <[email protected]>
argo-exec execs a command with the commands stdin/stdout proxied over
argo.  This is analagous to qubes rpc.

argo-input-receiver and argo-input-sender are wrappers to call
qubes-input-sender/receiver with argo-exec.

argo-input-receiver listens on argo port 7777 and forks off
sub-processes for incoming connections.  argo-input-receiver-wrapper
ensures logger runs in argo_input_receiver_t and not initrc_t.

argo-input-sender hits an ordering problem between udev and modutils
initscripts.  udev will launch argo-input-sender process for detected
input devices, but the xen-argo kernel module isn't loaded until
later in modutils.  argo-input-sender-kick is a one-off init script to
trigger udev to spawn argo-input-sender processes.

Signed-off-by: Jason Andryuk <[email protected]>
Add argo-input-receiver to dom0.

Signed-off-by: Jason Andryuk <[email protected]>
This is a basic recipe for the ubsvm image.  It runs vusb-daemon in
stub-mode and launches qubes-input-sender through argo-input-sender to
relay input.

Signed-off-by: Jason Andryuk <[email protected]>
/dev/tty0 is only present when a kernel is configured with
CONFIG_VT_CONSOLE, so the output redirection is incorrect when /dev/tty0
is not present.  A regular file will be created as /dev/tty0.

Use /dev/tty since "it is a synonym for the controlling
terminal of a process, if any." (from tty(4) manual page).  But that may
not be present either.  Use /dev/console which as the system console
makes sense for printing these messages.

Signed-off-by: Jason Andryuk <[email protected]>
This module provides SELinux labeling for the argo-input-sender and
argo-input-receiver.

Signed-off-by: Jason Andryuk <[email protected]>
Make the image SELinux enabled.

Signed-off-by: Jason Andryuk <[email protected]>
We run an SELinux-enabled system, but the busybox support is disabled.
Turn it on to enable the -Z option is assorted commands.

Signed-off-by: Jason Andryuk <[email protected]>
Label /dev/xen/xenbus as xenstore_dev_t so xenstore permissions can be
separated out from other xen_device_t permissions.  xenstore access is
useful on its own and many programs only use xenstore access without the
other xen devices.

OXT-1731

Signed-off-by: Jason Andryuk <[email protected]>
/dev/xen/privcmd is much more powerful than the other xen_device_t
interfaces, so label it separately.

OXT-1731

Signed-off-by: Jason Andryuk <[email protected]>
@jandryuk
Copy link
Contributor Author

jandryuk commented Nov 9, 2023

Refpolicy fixed up. Moved defconfig to 6.1 folder and refreshed under 6.1.

@crogers1
Copy link
Contributor

crogers1 commented Nov 9, 2023

Merging soon.

@crogers1 crogers1 merged commit c010d13 into OpenXT:master Nov 9, 2023
@jandryuk jandryuk deleted the usbvm-no-xsm-5.10 branch November 9, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants