-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On Mon, Oct 11, 2021 at 10:35:11AM -0400, Demi Marie Obenour wrote: > On Mon, Oct 11, 2021 at 04:28:01PM +0200, Marek Marczykowski-Górecki wrote: > > On Mon, Oct 11, 2021 at 09:13:18AM -0400, Demi Marie Obenour wrote: > > > On Fri, Oct 08, 2021 at 02:21:58AM +0200, Marek Marczykowski-Górecki > > > wrote: > > > > On Fri, Oct 08, 2021 at 02:12:08AM +0200, Simon Gaiser wrote: > > > > > Hi Marek and list, > > > > > > > > > > a qrexec service started by qrexec-agent (directly or via > > > > > qrexec-fork-server) can request that instead of using stdin and stdout > > > > > both side switch to using stdin bi-directional. To do so it needs to > > > > > send SIGUSR1 to QREXEC_AGENT_PID. > > > > > > > > > > According to the git history [1] this has been added for usbip. As > > > > > hinted in the commit message and in the comment [2] in the code this > > > > > mechanism has a problem. To send a signal you need to be root or run > > > > > as > > > > > the same user as the receiving process. > > > > > > > > > > This lead to a corner case in the Python port of split-gpg2 [3] that > > > > > tried to always switch to the bi-directional mode. During normal > > > > > operation things worked since it's spawned through qrexec-fork-server > > > > > that is running as user. But if the VM wasn't running the qrexec > > > > > service > > > > > in the freshly booted VM is spawned directly by qrexec-agent that runs > > > > > as root and therefore sending SIGUSR1 failed. > > > > > > > > I think fixing that "TODO" shouldn't be that hard. The data handling > > > > process (the one pointed with QREXEC_AGENT_PID) is already a separate > > > > process, and I don't think it needs to keep root privileges. > > > > This should be rather simple change, am I missing anything? > > > > > > I don’t think this is a good idea for two reasons. First, using SIGUSR1 > > > is inherently not very reliable, as it is asynchronous and doesn’t > > > provide any confirmation of signal receipt. > > > > I don't think it was ever an issue, but theoretically you are correct. > > Has this ever had any users other than USB pass-through? If not, it > might explain some of the random failures there.
Can you point at some specific bug reports? I'm only aware of two types of USB passthrough issues: - resetting USB device disconnects it (especially an issue for connecting Android phones, which looks like a device reset on mode switch) - unsupported data transfer mode (should be better now, since the current USBIP driver has XHCI support) > > > A better approach would be > > > to pass an AF_UNIX stream socket as file descriptor 3, > > > > Then, the qrexec doesn't know where to send the data... > > I meant to use file descriptor 3 for out-of-band control messages. On > further thought, I would actually prefer a *datagram* socket, I see, but TBH I don't like it that much. This makes writing a service significantly harder, because (in some cases) it is no longer enough to use standard read/write (or kill) which you can do trivially in any language/tool etc, sending/receiving datagram over AF_UNIX is rather uncommon thing (and often even sending UDP packets requires much less common API). On the other hand, I think one can rather easily get confirmation when the parent qrexec processes the SIGUSR1 - simply by waiting for EOF on FD 1. An alternative, is to use socket service. If you don't like an extra process laying around, you can use socket systemd unit, with Accept=true if necessary. But then, you need to read the service call handshake before using it for the actual data (not a big deal). > with > checks that the message actually came from the correct child process. No, that's a bad idea. This forces specific process structure of a service, which will be limiting factor. > > > or else just > > > unconditionally accept data on either file descriptor 0 or 1. > > > > Almost, closing FD 1 must not be sent as EOF (after which, sending more > > data over FD 0 wouldn't be possible anymore). And BTW, a heuristic of > > "don't send EOF on FD 1 close, if any data was received over FD 0 > > already" won't work, as for the usbip connection, the actual data is > > exchanged only after FD is passed on to the kernel (after closing other > > FDs). > > That is true. > > > > Second, > > > this would allow the child process to ptrace() the parent process and > > > obtain access to Xen file descriptors, which could potentially be used > > > to escalate privilege within the qube. While this isn’t an explicit > > > security boundary in Qubes OS, my understanding is that Qubes OS doesn’t > > > try to deliberately weaken it either. > > > > Well, user processes must have the ability to open vchan connections > > anyway, either for qrexec-client-vm to work, or for qrexec-fork-server. > > My understanding is that this typically requires membership in the > “qubes” group, which is the same group that > ‘qubes-core-agent-passwordless-root’ gives passwordless sudo access to. Yes. Which means, it would only change anything if running service as an user who is not member of the 'qubes' group. > > > > > For split-gpg2 I switched, at least for now, to always use stdin/-out > > > > > (maintaining two code paths would be ugly). > > > > > > > > > > Do we have plans to improve this? > > > > > > > > > > Beside the uspip case, are there any advantages of having a > > > > > bi-directional stdin? > > > > > > > > Yes, I'd consider making split-gpg2 a socket-based service (with one > > > > process handling several requests, to avoid process startup delay). For > > > > this, it needs to transfer data over a single socket FD. > > > > > > I fully support this, but with two significant caveats. The first is > > > that one must ensure the socket is present and listening before qrexec > > > tries to connect to it. This isn’t trivial for a service not running as > > > root. > > > > We could have a system unit with User= setting. > > Good point. > > > > The second is that right now, one can use user= directives in > > > qrexec policy to control which qube has access to which keys. While > > > this is not best practice, on memory-constrained hardware it can be the > > > difference between being able to use split-gpg and not being able to do > > > so. > > > > That's true. In fact it's yet another case, where overriding the call > > argument at the policy level could be useful (something I consider > > adding for a long time already) - then you could have different sockets > > for different users. > > That is a good idea that I had not considered. That said, it does > conflict with having split-gpg2 use the call argument for filtering. Depending on what you put into that call argument. It may actually ease it, for example: qubes.Gpg2+Sign enforced to qubes.Gpg2+Sign+0xKEYID You may need to create some symlinks to the socket for various operations, but it shouldn't be an issue. - -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmFkXLkACgkQ24/THMrX 1ywWbgf/XzQ2yZHC86gJTlPw8eAd3wa3EZUKrDFZFGgCKgJo4M/nTjAs1EyFsKSP HATNzKAZqBbg2pMORNNB7cRSr9s6MXgp9G52MFijGeCTTrYabymgDGuQDkd0vLob kdf60a4edyr8wzBluoxvFaWySugDiHqZM8kdNJup3PtRJjIqLT2p1b811gKvviSX C+HgcBXZWmnwGZ2OfObsXZ52gONKru7Jyen6yekZk5d7qRZQ7qTohbIMg65tc4Py rGz7EB/IVFBxd5NdVNNnPltkZ+rY/UHf1QycPCs4Rzc4+ulGN4hf2/gpEHLGx0AS BkAno4sJj3DtfQBW5UTZoZUCrw6Kgg== =pSeT -----END PGP SIGNATURE----- -- You received this message because you are subscribed to the Google Groups "qubes-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to qubes-devel+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/qubes-devel/YWRcupk7Q0UEw1ny%40mail-itl.
