Adding attacks and countermeasures to SD threat model public docs #4244
Conversation
emkll
force-pushed
the
docs-update-threat-model
branch
from
2b0d7cd
to
5e2ca2a
Compare
olivemartini
requested review from
emkll,
kushaldas and
redshiftzero
as code owners
There was a problem hiding this comment.
@olivemartini thank you very much for submitting this PR !
I've rebased your branch against current develop branch, which appears to have fixed the ci "failures" 🎉
I've left a few comments inline for discussion, interested in seeing what you think !
docs/threat_model/mitigations.rst
Outdated
| - Communications vulnerability in *Source Interface* or *Journalist Interface* | ||
| - Error handling and logging vulnerability in *Source Interface* or *Journalist Interface* | ||
| - HTTP security configuration vulnerability in *Source Interface* or *Journalist Interface* | ||
| - File and resource vulnerability in *Journalist interface* |
There was a problem hiding this comment.
For File and Resource, Business logic and Web services vulnerabilities, they should be listed as threats to both source and journalist interfaces
There was a problem hiding this comment.
Business logic and web services i just added to our internal document due to omission, but file and resource were already there.
docs/threat_model/mitigations.rst
Outdated
| Attacks to the Application Code | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| - Configuration vulnerability in *Source Interface* or *Journalist Interface* |
There was a problem hiding this comment.
There's a lot of repetition here, what would you think about creating a subtitle called something like "source and journalist interface" or "SecureDrop Application - Source and journalist interface" ?
There was a problem hiding this comment.
That's a really good Idea. I will rework this section, and break out the Interfaces attacks for clarity and concision.
docs/threat_model/mitigations.rst
Outdated
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| - *Journalist Interface* is located behind an authenticated hidden service and only privileged users have required authorization token | ||
| - Tor hidden service protocol is end-to-end encrypted, and TLS is opt-in with EV cert, but no config option is supported |
There was a problem hiding this comment.
TLS is not currently available on the Journalist interface
docs/threat_model/mitigations.rst
Outdated
|
|
||
| - All source submissions are encrypted with GPG at rest using the airgapped submission key | ||
| - Sensitive source and submission data is sent through HTTP POST | ||
| - *Source Interface* runs on an end-to-end encrypted Tor onion service, and TLS is opt-in with an EV cert |
There was a problem hiding this comment.
Since it's optional and a distinct layer of encryption, it might make sense to list it in a separate bullet point
docs/threat_model/mitigations.rst
Outdated
| - All *Source Interface* session data (except language and locale selection) is discarded at logout, and fully deleted upon exiting the Tor Browser | ||
| - *A number of mitigations are in place as protection against malicious input vulnerabilities*: X-XSS-PROTECTION is enabled and Content-Security-Policy is set to self; SQLAlchemy is used as ORM for all database queries; and Application does not execute uploaded data | ||
| - *A number of mitigations are in place as protection against the risk of an HTTP misconfiguration*: Only HTTP GET, POST and HEAD are allowed; HTTP headers do not expose version information of system components; X-Content-Type is set to "nosniff;" Content-Security-Policy is set to "self;" and X-XSS-Protection is set to "1" | ||
| - *A number of mitigations are in place as protection against access control vulnerabilities*: Cache control header is set to “no store;” Source codenames are long and automatically generated, and stored in a database hashed with a unique salt; Source codename reset functionality is not available; Source login does not display information about prior submissions; Souce login requires 7-word codename to check Source Interface for replies |
There was a problem hiding this comment.
I think Cache-control would be best suited in the HTTP misconfiguration section above.
docs/threat_model/mitigations.rst
Outdated
|
|
||
| Countermeasures Against Malicious Tails or Ubuntu ISOs | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| - SecureDrop dmin guide (https://docs.securedrop.org/en/stable/admin.html) instructs Users/Admins to validate checksum/signatures of downloaded images |
docs/threat_model/mitigations.rst
Outdated
| Countermeasures in News Organization Corporate Network | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| - SecureDrop environment should be strictly segregated from corporate environment | ||
| - Most SecureDrop traffic goes over Tor and as such is encrypted end-to-end |
There was a problem hiding this comment.
nit: SecureDrop application traffic
docs/threat_model/mitigations.rst
Outdated
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| - SecureDrop environment should be strictly segregated from corporate environment | ||
| - Most SecureDrop traffic goes over Tor and as such is encrypted end-to-end | ||
| - Alert emails to Journalists and Admins are GPG-encrypted (but not signed) to provide confidentiality and prevent tampering |
There was a problem hiding this comment.
Tampering is not assured in this case since the message is not signed. Let's remove and prevent tampering here. This was an error in the original document, and it has been updated.
docs/threat_model/mitigations.rst
Outdated
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| - All SecureDrop infrastructure is provisioned via infrastructure-as-code (Ansible scripts). | ||
| - *Monitor Server* should only expose SSH via Tor hidden service. All other traffic should be blocked by firewall | ||
| - FPF performs vulnerability management for software dependencies as well automatic nightly updates for dependencies and OS packages |
There was a problem hiding this comment.
vulnerability management and automatic updates for os packages should be distinct bullets. Furthermore they are also countermeasures on the Application server as well
docs/threat_model/mitigations.rst
Outdated
| - For SecureDrop Developers, 2-factor authentication is mandated on GitHub | ||
| - Community trust is built through 3 trusted code owners and code reviews | ||
|
|
||
| Attacks and Countermeasures on the *Application Server* and *Monitor Server* |
There was a problem hiding this comment.
In this section, there's a lot of repetition for countermeasures, specifically os-level hardening items (grsecurity, apparmor, ossec...)Perhaps it would make sense to use sections and subsections to represent the hierarchy of what it would look like in the dataflow diagram, for example:
app server
- Application server
1.1 operating sytem
- grsec
- apparmor
1.2 securedrop application
- All the application-level countermeasures
1.2.1 application server dependencies
- vulnerability management
1.3 tor
- Hidden service auth
Do you think this would make sense in the context of this PR?
- Document users - Document adversaries - Document systems
emkll
force-pushed
the
docs-update-threat-model
branch
3 times, most recently
from
89a9e0f
to
b80328d
Compare
emkll
force-pushed
the
docs-update-threat-model
branch
from
a02f7ee
to
15d34f3
Compare
There was a problem hiding this comment.
Thanks @olivemartini for the fixes, this looks great! I've rebased this branch on develop due to some changes required in (unrelated) tests and appended two commits:
- 2ebc244 to modify some formatting/listing
- 15d34f3 to update these docs based on changes introduced Allow DELETE HTTP method for journalist interface #4023
I'm approving this PR but not merging it, could another maintainer take a look at my commits in this PR? Thanks
|
Splendid! Thank you for your hard work on this, @olivemartini! Took another pass through the docs changes, including the small clarifications @emkll appended most recently. Merging! |
Status
Work in progress
Description of Changes
Fixes #3349
Changes proposed in this pull request:
Testing
How should the reviewer test this PR? Please send feedback about formatting and style. All comments welcome.
Deployment
Any special considerations for deployment? Docs only.
Checklist
If you made changes to documentation:
make docs-lint) passed locally