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

Apply 2.5.1 fixes to develop #6704

Merged
merged 7 commits into from Dec 9, 2022
Merged

Apply 2.5.1 fixes to develop #6704

merged 7 commits into from Dec 9, 2022

Conversation

legoktm
Copy link
Contributor

@legoktm legoktm commented Dec 8, 2022

Status

Ready for review

Description of Changes

Rebase of the 2.5.1 patches onto develop.

Testing

Other than that, all the commits have already gone through pretty rigorous testing.

/var/www/securedrop is now root-owned but world readable. This ensures
that code can only be added/modified/removed by root, not www-data.

The package already installs files /var/www/securedrop with the correct
permissions, we were just clobbering it in the postinst, which is now
removed. We have an explicit chmod to adjust directory permissions.

We need special handling for config.py since it is installed by ansible
instead of the package. Since it contains secrets, we want it to be
only-readable by root/www-data and no one else.

The custom_logo.png is writable by www-data so administrators can change
the logo from the web interface. In the future this could be moved
somewhere else. The file may not exist yet (fresh installs), so we create
it if necessary.

testinfra checks have been updated with the new permissions and took the
time to de-duplicate the list of app directories in 7 different yaml files.
An attacker with access to www-data could have gpg-agent.conf be a
symlink to a root-owned file (e.g. /etc/passwd) and trick the postinst
into clobbering that file as root. Because the content is fixed, it's
pretty low risk but straightforward to fix.
* Add a postinst snippet to migrate the root crontab over to www-data
  on upgrade.
* Modify ansible task to install cron jobs for www-data on new installs
* Update testinfra checks to look at the www-data cron for the tmp
  cleaning job and verify that root's crontab is empty.
There's no need for us to run this as root, all the files alembic needs
to touch (primarily the SQLite database) are writable by www-data. We
specifically run the backup copying and deletion steps as www-data too
so symlink attacks can't be used to have root clobber other files.

For `mod_wsgi-express module-config`, we ship the generated file in the
package itself. In a future (non-hotfix) release we will generate the
file at build-time instead of hardcoding it.
Makes it possible to run CI in forks. For a private repository,
you'll need to set up a personal access token[1] and store it as a
"GITHUB_AUTH" environment variable in CircleCI.

[1] https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token
@legoktm legoktm requested a review from a team as a code owner December 8, 2022 20:07
@zenmonkeykstop zenmonkeykstop self-assigned this Dec 9, 2022
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

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

LGTM (replicated the cherry-picking process and compared just to be sure).

@zenmonkeykstop zenmonkeykstop merged commit aa8fe5b into develop Dec 9, 2022
8 checks passed
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.

None yet

2 participants