Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure safe perms for svs.sqlite and sync_flag #1256

Merged
merged 1 commit into from
May 11, 2021

Conversation

sssoleileraaa
Copy link
Contributor

@sssoleileraaa sssoleileraaa commented Apr 29, 2021

Description

Fixes #1232

Test Plan

  • Build deb for the client and install on sd-app or just patch it with this PR change
  • Delete ~/.securedrop_client (you can manually copy config.json if you want but it's not necessary to test this change)
  • Start the client via commandline by running: securedrop-client
  • Confirm svs.sqlite and sync_flag now have 600 perms instead of 644

@sssoleileraaa sssoleileraaa requested a review from a team as a code owner April 29, 2021 18:49
@emkll emkll force-pushed the tighten-perms-on-sqlite-file branch from f3d2732 to b3f16cc Compare April 30, 2021 18:15
@emkll
Copy link
Contributor

emkll commented Apr 30, 2021

rebased on latest main

emkll
emkll previously approved these changes May 4, 2021
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes here work well, but it's important to note that it will not update existing svs.sqlite and sync_flag files: These changes will only work if/when those files are created by the client. I think this is fine to merge as-is.

@sssoleileraaa
Copy link
Contributor Author

@emkll - to address your feedback, i updated the client to fix insecure perms when it starts. follow the test plan, but instead of deleting ~/.securedrop_client just modify the permissions of the files to something like 644 to test this new behavior.

@sssoleileraaa sssoleileraaa force-pushed the tighten-perms-on-sqlite-file branch 2 times, most recently from a196e85 to 00a9c08 Compare May 6, 2021 00:57
Signed-off-by: Allie Crevier <allie@freedom.press>
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @creviera for the changes, went through another round of testing, looks good to me!

@emkll emkll merged commit 09621da into main May 11, 2021
@emkll emkll deleted the tighten-perms-on-sqlite-file branch May 11, 2021 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tighten svs.sqlite file permissions
2 participants