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

app code attempting to delete /var/lib/securedrop/store #5031

Closed
redshiftzero opened this issue Nov 26, 2019 · 3 comments · Fixed by #5037
Closed

app code attempting to delete /var/lib/securedrop/store #5031

redshiftzero opened this issue Nov 26, 2019 · 3 comments · Fixed by #5037

Comments

@redshiftzero
Copy link
Contributor

Description

When I'm deleting files I'm seeing an OSSEC alert indicating that the app code is trying to rm /var/lib/securedrop/store (which contains all submissions). It's being stopped thanks to an AppArmor denial but this means there's a potentially very destructive bug and we need to carefully investigate why this is happening.

Expected behavior

No OSSEC alert

Actual behavior

OSSEC HIDS Notification.
2019 Nov 25 21:07:04

Received From: (app) 10.20.2.2->/var/log/syslog
Rule: 100012 fired (level 7) -> "Apparmor denied event"
Portion of the log(s):

Nov 25 21:07:04 app kernel: [ 2160.102070] audit: type=1400 audit(1574734024.249:26): apparmor="DENIED" operation="rmdir" profile="/usr/sbin/apache2" name="/var/lib/securedrop/store/" pid=3067 comm="apache2" requested_mask="d" denied_mask="d" fsuid=33 ouid=33



 --END OF NOTIFICATION

Comments

I would first investigate this method as it was recently merged and is performing deletions:

def clear_shredder(self):

@redshiftzero redshiftzero added this to the 1.2.0 milestone Nov 26, 2019
@eloquence eloquence added this to Current Sprint - 11/20 -12/4 in SecureDrop Team Board Nov 26, 2019
@redshiftzero
Copy link
Contributor Author

redshiftzero commented Nov 26, 2019

Reproducer in dev:

  1. Run on ae09438 (adds shredder service to dev env) this is the case even without the shredder running..
  2. Sign into Journalist Interface
  3. Select all -> Delete
  4. Watch the store directory get deleted since we don't have the guard of AppArmor in the dev env

@redshiftzero
Copy link
Contributor Author

this is due to the use of os.renames in store.Storage.move_to_shredder. The underlying issue is due a difference in behavior between os.rename and os.renames. From the docs:

After the rename, directories corresponding to rightmost path segments of the old name will be pruned away using removedirs().

this is what is deleting /var/lib/securedrop/store

@redshiftzero redshiftzero self-assigned this Nov 26, 2019
@redshiftzero redshiftzero moved this from Current Sprint - 11/20 -12/4 to In Development in SecureDrop Team Board Nov 26, 2019
@zenmonkeykstop
Copy link
Contributor

Gonna update the test plan to cover this case. Good catch!

redshiftzero added a commit that referenced this issue Nov 26, 2019
os.renames will silently delete directories, from the docs [0]:

After the rename, directories corresponding to rightmost path
segments of the old name will be pruned away using removedirs().

This caused the deletion of /var/lib/securedrop/store in #5031

Instead, let's use a modified version of os.renames that doesn't
prune.

[0] https://docs.python.org/3/library/os.html#os.renames
[1] https://github.com/python/cpython/blob/master/Lib/os.py#L250
redshiftzero added a commit that referenced this issue Nov 26, 2019
os.renames will silently delete directories, from the docs [0]:

After the rename, directories corresponding to rightmost path
segments of the old name will be pruned away using removedirs().

This caused the deletion of /var/lib/securedrop/store in #5031

Instead, let's use a modified version of os.renames that doesn't
prune.

[0] https://docs.python.org/3/library/os.html#os.renames
[1] https://github.com/python/cpython/blob/master/Lib/os.py#L250
SecureDrop Team Board automation moved this from In Development to Done Nov 26, 2019
zenmonkeykstop pushed a commit to zenmonkeykstop/securedrop that referenced this issue Nov 26, 2019
os.renames will silently delete directories, from the docs [0]:

After the rename, directories corresponding to rightmost path
segments of the old name will be pruned away using removedirs().

This caused the deletion of /var/lib/securedrop/store in freedomofpress#5031

Instead, let's use a modified version of os.renames that doesn't
prune.

[0] https://docs.python.org/3/library/os.html#os.renames
[1] https://github.com/python/cpython/blob/master/Lib/os.py#L250

(cherry picked from commit 1fccef1)
This was referenced Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants