-
Notifications
You must be signed in to change notification settings - Fork 687
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
configure OSSEC server gnupg directory permissions in securedrop-osse… #5330
configure OSSEC server gnupg directory permissions in securedrop-osse… #5330
Conversation
Visually this looks sound to me. In your testing, does using the |
The reason I suggested looking in However, I'm poking at this now to update my test plan! |
959a7a8
to
03e0cd2
Compare
Hi @rocodes, sorry if the original issue trail is a bit confusing, as I mentioned on #5265 (comment) the alerts did send successfully to me after the reboot cycle, but I see the procmail error logged even though the alert is sent successfully. |
Not sure about the CI failure here. I did rebase on latest develop. |
Hi, yup that was clear to me-- the test path involves checking to make sure the alert is sent, and then checking to make sure there are no errors in |
For simplicity, I've tested this by running the two $ find /var/ossec/.gnupg -type f -exec chmod 600 {} \;
$ find /var/ossec/.gnupg -type d -exec chmod 700 {} \; This change broke OSSEC email delivery, with more severe permission errors. The reason is that on my long-running prod instance, the files in Note that in the Ansible code you're removing, there's logic to set the owner to - name: Ensure correct permissions on contents of OSSEC GPG homedir.
file:
state: file
path: "/var/ossec/.gnupg/{{ item.item }}"
mode: "0600"
owner: ossec
group: "{{ ossec_group }}"
with_items: "{{ gpg_keyring_status.results }}"
when: item.stat.exists
tags:
- gpg At minimum I think we need to preserve this logic by adding I don't know why the files are owned by |
Re-running the playbook does appear to have correctly adjusted the ownership of the files in I think if we just add |
Checked with @rocodes and they have offered to pick this up again during the 10/15-10/28 sprint. |
03e0cd2
to
d232986
Compare
So far I've tested manually setting the permissions as you do here in postinst, and that permission configuration seems to make |
Staging scenario:
In fact, |
One entry is fine since you triggered a test alert, but you shouldn't see an additional one (which would have previously been triggered by the trustdb thing). Sorry this was unclear. |
I'm observing 700 permissions on the directory, and 600 permissions on its contents.
|
tl;dr of my findings above:
|
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.
The perms issue wasn't reproducible for me on current staging or prod, but tested in the upgrade scenario by changing keyring file permissions manually after make upgrade-start
. They were successfully changed to the correct values by the postint.
Once @eloquence's formatting nit is addressed, this is good to go IMO.
…c-server postinst. Remove configuration from configure_server task.
d232986
to
c639a03
Compare
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.
Took another spin through the upgrade scenario with the most recent changes. File perms are set to expected values after upgrade and diff looks good.
Configure OSSEC server gnupg directory permissions in
securedrop-ossec-server
post-installation script. Remove permissions configuration fromconfigure_server
task.Status
Ready for Review
Description of Changes
Fixes #5265.
Changes proposed in this pull request:
Remove OSSEC directory permission configuration steps from configure_server task and set them in
securedrop-ossec-server
post-installation script.Modify OSSEC
/var/ossec/.gnupg
directory permission, and sub-directories if existing (required forprivate-keys.d
if ever created) to 700, and files in/var/ossec/.gnupg/*
to 600.Testing
Staging
Check out this branch and
make build-debs
.Clean install prereqs
make staging
Upgrade scenario prereqs
make upgrade-start
/var/ossec/.gnupg
and 550 permissions on/var/ossec/.gnupg/*
make upgrade-test-local
On both:
/var/ossec/.gnupg
var/ossec/.gnupg/*
/var/ossec/logs/alerts/alerts.log
/var/ossec/logs/alerts/alerts.log
gpg: Permission denied
,trustdb not writable
, orExecuting "/var/ossec/send_encrypted_alarm.sh,ossec"
in/var/log/procmail.log
Prod/Release testing
/var/ossec/.gnupg
in the cron-apt upgrade scenario for QADeployment
securedrop-ossec-server
package is upgradedChecklist
If you made changes to the system configuration: