-
Notifications
You must be signed in to change notification settings - Fork 902
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
rule(Write below etc): whitelist automount writing under /etc/mtab #957
rule(Write below etc): whitelist automount writing under /etc/mtab #957
Conversation
This commit allows automount to write under /etc/mtab without flagging it as an error. Signed-off-by: Nicolas Marier <nmarier@coveo.com>
Welcome @marier-nico! It looks like this is your first PR to falcosecurity/falco 🎉 |
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.
Thanks @marier-nico ! 🎉
This LGTM, just last checks we are approaching together on Slack and then will merge it! :)
@@ -1153,6 +1153,9 @@ | |||
- macro: etcd_manager_updating_dns | |||
condition: (container and proc.name=etcd-manager and fd.name=/etc/hosts) | |||
|
|||
- macro: automount_using_mtab | |||
condition: (proc.pname = automount and fd.name startswith /etc/mtab) |
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.
condition: (proc.pname = automount and fd.name startswith /etc/mtab) | |
condition: (proc.pname=automount and fd.name=/etc/mtab) |
WDYT?
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.
After having spent some hours with Nicolas on this we decided to merge PR as is.
He used startswith
because he had automount
daemon writing to /etc/mtab.[something]
file (which to me seems something happening when the /etc/mtab
file cannot be written because it's locked, maybe some other process writing to it?).
In the next days he's going to observe again the original event in order to understand whether it is better to use equal
or startswith
here.
LGTM label has been added. Git tree hash: 2763b7ed0a52c1eabf0adaae0ba6c2c45bfbe702
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fntlnz, leodido The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Merging as is. But pls see note above. cc @marier-nico |
Type of PR
/kind rule-update
/area rules
What this PR does / why we need it:
This will reduce false-positives related to
automount
writing to/etc/mtab
. For the sake of completeness (and to help anyone that might not be aware), "mtab lists currently mounted file systems and is used by the mount and unmount commands when you want to list your mounts or unmount all. It's not used by the kernel, which maintains its own list." (https://serverfault.com/questions/267609/how-to-understand-etc-mtab).automount
is expected to write under/etc/mtab
, but there was no macro to whitelist it.Which issue(s) this PR fixes: I didn't open an issue for this.
Does this PR introduce a user-facing change?: