freedomofpress / securedrop-workstation

Qubes-based SecureDrop Journalist Workstation environment for submission handling
GNU Affero General Public License v3.0
138 stars 42 forks source link

Include timeouts on spawned processes #682

Open eloquence opened 3 years ago

eloquence commented 3 years ago

Informational finding TOB-SDW-021 from the 2020 SecureDrop Workstation audit (PDF) recommends using Popen.communicate() , Popen.wait() or Popen.terminate() in cases like this where we currently spawn processes like so:

https://github.com/freedomofpress/securedrop-export/blob/34a8cf6/securedrop_export/disk/actions.py#L48-L52

Quoth the report:

If those processes close their stdout, the securedrop-export will process their stdout, while they will further operate in the background.

The report recommends passing timeouts "to make sure the spawned processes don't halt the SecureDrop programs completely."

eloquence commented 3 years ago

(Tracked as epic; we'll likely want to identify a consistent approach across the board and then enumerate all calls that should be updated.)