-
Notifications
You must be signed in to change notification settings - Fork 694
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
[xenial] Manages PaX flags on Apache under Xenial #4114
Conversation
Installs paxctld from distro repos under Xenial, edits the config in place, and bounces the service. Using paxctld rather than paxctl to ensure that flags are restored after security updates to the apache2 package via apt, which may occur outside the scope of SecureDrop updates. Includes changes to PaX flag management against Firefox (used for testing) because paxctld ships with suitable flags for firefox already, via the upstream config. Therefore we can stick to paxctl for Trusty, but leverage paxctld under Xenial. Reminder that we plan to axe the entire `app-test` role once we have external browser testing working.
Reminder that the new flag enforcement logic is not present on |
Tested on staging VMs as per plan, file uploads no longer generate the memprotect or temporaryfile errors (did see some larger file uploads fail but that's a known and unrelated issue) Also removed and reinstalled apache2 on app-staging (as a naive way to test if pax flags are applied on new apache2 binary):
File uploads continue to work afterwards. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local testing following the test plan looks good to me:
-
make build-debs-xenial
-
make staging-xenial
- Make a test submission, with both a text and file upload component.
- Confirm submission successful.
Furthermore, I can confirm that:
- paxctld systemd service is running (and restarts on reboot)
- all infra tests pass (since only trusty infra tests are run in ci)
As noted in the PR message, once this is merged we should rebuild packages and test the trusty->xenial upgrade scenario to ensure paxctld is installed when ./securedrop-admin install
is invoked post-xenial-upgrade.
A couple of minor comments, which shouldn't block merge but might be worth exploring for maintainability:
-
We are using paxctl elsewhere to handle PaX flags for grub binaries (https://github.com/freedomofpress/securedrop/blob/develop/install_files/ansible-base/roles/grsecurity/tasks/paxctl.yml). Would it make sense to open a ticket to consolidate the PaX flag management?
-
paxctld by default installs a 97-line config file that sets PaX flags for binaries that either do not exist or do not require it. Would it be less risky for us to manage the paxctld.conf file, instead of relying on the default provided config and appending via postinst? (perhaps similar to what was done in the workstation context?: https://github.com/freedomofpress/securedrop-debian-packaging/tree/master/securedrop-workstation-svs-disp).
@@ -5,7 +5,7 @@ DEB_DH_INSTALL_ARGS=-X .git | |||
# Set distro-specific packages here, for interpolation in Depends field. | |||
# All other deps can be reused, regardless of distro. | |||
TRUSTY_DEPS=apache2-mpm-worker | |||
XENIAL_DEPS=apache2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that paxctld is in the universe
channel for xenial, but not in the security
channel: https://packages.ubuntu.com/xenial/paxctld . Since the cron-apt logic uses only security as the source list (https://github.com/freedomofpress/securedrop/blob/develop/install_files/ansible-base/roles/common/files/5-security), the paxctld dependency will not be resolved if the package is upgraded via the unattended apt upgrades.
In the current upgrade scenario this should work work, since the first version of the xenial package will be installed via ./securedrop-admin install
, once the app server has been upgraded to Xenial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
make build-debs-xenial
-
make staging-xenial
- Make a test submission, with both a text and file upload component.
- Confirm submission successful.
Status
Ready for review.
Description of Changes
Fixes #4110.
Changes proposed in this pull request:
Installs paxctld from distro repos under Xenial, edits the config in
place, and bounces the service. Using paxctld rather than paxctl to
ensure that flags are restored after security updates to the apache2
package via apt, which may occur outside the scope of SecureDrop
updates.
Includes changes to PaX flag management against Firefox (used for
testing) because paxctld ships with suitable flags for firefox already,
via the upstream config. Therefore we can stick to paxctl for Trusty,
but leverage paxctld under Xenial. Reminder that we plan to axe
the entire
app-test
role once we have external browser testingworking.
Testing
make build-debs-xenial
make staging-xenial
Deployment
Quite necessary for Xenial compatibility.
Checklist
If you made changes to the server application code:
make ci-lint
) and tests (make -C securedrop test
) pass in the development containerIf you made changes to
securedrop-admin
:make -C admin test
) pass in the admin development containerIf you made changes to the system configuration:
If you made non-trivial code changes:
If you made changes to documentation:
make docs-lint
) passed locally