freedomofpress
Replace ExportStatus parsing with Exit Codes and Deduplicate Status Messaging
Description
The communication between sd-app and sd-devices for export workflows is not exactly one-way. The server component in sd-devices sends status information as a string and then the client convert it to an Enum. This also means that status information needs to be duplicated in the client and in the export code bases .
Although it does not seem to present a lot of freedom for for an attacker (Enum from string). But if unstrusted parsing is avoidable it should be avoided. Not to mention side-effects like any text unintentionally sent back (e.g. via a subprocess or a python print) leading to a GENERIC_ERROR on the client.
This is something that was already solved in Dangerzone. It was using simple JSON objects to send progress information and error status. The solution was to instead rely on exit codes. No more JSON being sent back and parsed. Importantly, qrexec does pass exit codes back. Otherwise, none of this would work.
Solution proposal:
- replace all status parsing with exit codes (all code paths seem to assume that the status is received on process exit anyways)
- have a shared exit-code-to-exit-message mapping that is shared between the client and export (will avoid all the duplication)
How will this impact SecureDrop users?
No impact. As I understand it, the server does not communicate any more information back to the client other than the simple status string. We are effectively closing the gap for more granular information (e.g. progress). But (A) this wasn't being used already and (B) we can always add that and it shouldn't have any intersection with the final status transmitted at the end.
How would this affect the SecureDrop Workstation threat model?
Should not. If anything it should harden it a bit.
User Stories
As a developer...
- I would prefer to avoid parsing untrusted status information where possible.
- I would like to avoid having duplicated status strings in various parts of the code
- I would like not to have to think about the program not sending any prints (e.g. via imported module or suprocess), risking breaking it.
Related / complementary idea: https://github.com/freedomofpress/securedrop-workstation/issues/671#issuecomment-2772678342
I think this is contingent on the issue you linked about using a custom rpc instead of qopen, and making sure that the nonzero exit code doesn't trigger fallback mime handling in the vm
Related (ish) in concept, due to looking for ways to solve the mime handling issue: https://github.com/freedomofpress/securedrop-workstation/issues/1139
I came across an example of Qubes using exit codes in the same way I am advocating for. This is in the qubes-salt repo