Add functional RPC policy tests

[eloquence]

Our current RPC policy tests only test whether the policy files match the expected content, not whether they have the expected effect. We should test actual behavior, especially in light of the changes in Qubes 4.1/#751 which add significant complexity to the RPC policy behavior (two different formats, precedence rules between them).

[zenmonkeykstop]

Relevant to recent move to v2 policy format - we have a basic dom0 test verifying the existence of the policy files, but behaviour should also be checked.

[micahflee]

The RPC policies are defined in this file. Based on reading these, I think I should proceed like this:

• Let's not test qubes.USBAttach and qubes.USB, because it would involve USB disks plugged into sys-usb to run the tests
• Let's not test the ask rules, since it will pop up windows asking permission during tests so they couldn't really be automated
• Should we test qubes.OpenInVM rules and if so, how? And how do we clean up after? We can maybe test that a file is opened in a dispvm by looking at what appvms are open before, then opening the file in a dispvm, and looking what appvms are open after, and then killing the new dispvm if it has appeared?
• Let's test all other allow and deny rules

So given that, I should write the following tests:

• securedrop.Log from @tag:sd-workstation to sd-log should be allowed
• securedrop.Log from anything else to sd-log should be denied
• securedrop.Proxy from sd-app to sd-proxy should be allowed
• securedrop.Proxy from anything else to sd-proxy should be denied
• qubes.Gpg, qubes.GpgImportKey, and qubes.Gpg2 from @tag:sd-client to sd-gpg should be allowed
• qubes.Gpg, qubes.GpgImportKey, and qubes.Gpg2 from anything else to sd-gpg should be denied
• qubes.Filecopy from sd-proxy to @tag:sd-client should be allowed
• qubes.Filecopy from anything else to @tag:sd-client should be denied
• Maybe test qubes.OpenInVM from @tag:sd-client to @dispvm:sd-viewer
• Maybe test qubes.OpenInVM from @tag:sd-client to sd-devices
• Maybe test qubes.OpenInVM from sd-devices to @dispvm:sd-viewer

I'm curious if anyone has feedback on this plan.

[rocodes]

My initial impression is that we should probably focus our testing on securedrop.Log and securedrop.Proxy, which are our custom RPC policies, and not worry too much about writing tests for upstream policies, unless it's in the context of overall automating of functional testing/acceptance testing of the workstation.

Also, since we've just moved our rpc policy files to take precedence over the !compat files, we have less to worry about than when we filed this issue, in terms of simpler rules and everything being defined in one file.

[legoktm]

I believe the GPG RPC stuff will still prompt (something like "Do you allow X VM to access your GPG keys?")

• qubes.Filecopy from sd-proxy to @tag:sd-client should be allowed
• qubes.Filecopy from anything else to @tag:sd-client should be denied

We'll be removing these rules shortly once freedomofpress/securedrop-client#1718 lands, so I think you can skip testing them (also a good reminder that these rules exist, I'll file a cleanup task).

[micahflee]

My initial impression is that we should probably focus our testing on securedrop.Log and securedrop.Proxy, which are our custom RPC policies, and not worry too much about writing tests for upstream policies, unless it's in the context of overall automating of functional testing/acceptance testing of the workstation.

I've started implementing these tests. Rather than confirming that e.g. securedrop.Log actually logs stuff, I'm just confirming that when sd-app tries using securedrop.Log to sd-log it's allowed (return code 0), but when sys-firewall tries the same thing it's denied (return code 126).

So given that, I think it's fine to just ensure that the upstream policy rules still allow/deny when they should.

I believe the GPG RPC stuff will still prompt (something like "Do you allow X VM to access your GPG keys?")

Ah good point. Maybe I will just test the deny rule (which should just deny without a popup) but not test the allow rules.

And if the qubes.Filecopy rules are going away I won't bother testing them.

[zenmonkeykstop]

Would it make sense to move to pytest first (see #998)? It seems like the tests you're describing would be more manageable if you could parametrize them.

[legoktm]

In an ideal world yes, but pytest is more of a struggle than I anticipated, so I wouldn't block any work on it.

[micahflee]

This was simpler to implement than I thought it would be. Just made a PR!