Invalid VM name handling in qrexec-policy

[Jean-Philippe Ouellet]

Currently, non-existing VMs match $anyvm in qrexec-policy. I suggest
that this is potentially dangerous when combined with innocent
mistakes elsewhere, and should not be the default.

From https://github.com/QubesOS/qubes-gui-daemon/pull/10:

> I did not detect this sooner because if qubes-clipboard.bin.source had
> a trailing newline, then evaluate_clipboard_policy() would not fail,
> and my dom0 clipboard-copyout script produced such a trailing newline.

This to me sounds like something that may cause trouble in the future,
both in false-denies and more importantly potentially false-allows.

If source vm names are being mangled somehow, `--assume-yes-for-ask`
allows a specific policy to fall through in a potentially surprising
way.

Consider a policy like:
$anyvm protected-thing deny
$anyvm $anyvm ask
or
bad-vm $anyvm deny
$anyvm $anyvm ask

Consider this example:
[user@dom0 qubes]$ /usr/lib/qubes/qrexec-policy --assume-yes-for-ask \
> --just-evaluate dummy_id some_source_vm_name_that_got_mangled_somehow \
> sys-firewall qubes.OpenInVM 0 && echo pwned || echo safe
pwned

This invocation of qrexec-policy was taken from
gui-daemon/gui-daemon/xside.c:
https://github.com/QubesOS/qubes-gui-daemon/blob/95417c573d9b24269d50b3733164c3c9e390851c/gui-daemon/xside.c#L753-L754

So, I propose we make qrexec-policy actually verify that an evaluated
VM actually exists in order to match $anyvm. This would mean invalid
sources would fall all the way through to the implicit deny, even in
case of `--assume-yes-for-ask`.

Thoughts?

[Jean-Philippe Ouellet]

On Sat, Nov 26, 2016 at 7:05 PM, Jean-Philippe Ouellet <j...@vt.edu> wrote:
> Consider a policy like:
> $anyvm protected-thing deny
> $anyvm $anyvm ask
> or
> bad-vm $anyvm deny
> $anyvm $anyvm ask

This pattern is even recommended in the official docs [1].

[1]: https://www.qubes-os.org/doc/copy-paste/#clipboard-automatic-policy-enforcement

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Generally source VM name always come from trusted place - out of VM
control. If you wrote it manually to qubes-clipboard.bin.source, well,
that's also "trusted" place.

Anyway, this looks like a good idea to verify it and deny the call if
there is no such VM.

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJYOinoAAoJENuP0xzK19csA5UH/A7sGIiZF/G4Hqi9ZGLqKi0Q
NLafL5/0ZRM83bhfxn4zcPdOD6Y+9uc64lDlT58Tnp4FsfxrGlJrgU62Ob/WtM9Q
wl5rGr1Msro9gK63kfMqd+CHVdCys43RKiicy3jQ5/n6rXL+QTe1iaYQJLiB9Wr0
GzGJlEnyYNjf5DavBKLlorIsYD+x5GTf07xfoNZqHoBZx/SS+4A8mEf0/Cv4ZXIS
X+F6rSOkrmWtvJbEDu56ZLKt6AVQlN/MTTl7MmT7a3xZFhcpumIOvHi7FDDPBV1e
3aWr3j6bQEjknck3cRmIg6lyp9F8DOoYSflLFSdCzaeIxNJHGIJbcD18kt8uYBI=
=kVfN
-----END PGP SIGNATURE-----

[Jean-Philippe Ouellet]

On Sat, Nov 26, 2016 at 7:33 PM, Marek Marczykowski-Górecki
<marm...@invisiblethingslab.com> wrote:
> Generally source VM name always come from trusted place - out of VM
> control. If you wrote it manually to qubes-clipboard.bin.source, well,
> that's also "trusted" place.

Yes, but trusted in no way implies bug free.

I may not be the only one to make this particular mistake in the
future, therefore a general mitigation sounds like a good idea to me.

> Anyway, this looks like a good idea to verify it and deny the call if
> there is no such VM.

Noted. Tracking here:
- https://github.com/QubesOS/qubes-issues/issues/2460
- https://github.com/QubesOS/qubes-issues/issues/2461

Feel free to assign those issues to me if you'd like. I will get to
them when I can.