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

Allow to whitelist shell config modifiers #1938

Conversation

claudio-vellage
Copy link
Contributor

@claudio-vellage claudio-vellage commented Mar 13, 2022

What type of PR is this?

/kind bug
/kind feature

If contributing rules or changes to rules, please make sure to also uncomment one of the following line:

/kind rule-update

Any specific area of the project related to this PR?

/area rules

What this PR does / why we need it:

This PR allows to whitelist specific containers to modify the shell config files.

Which issue(s) this PR fixes:

When running on GKE with COS_CONTAINERD images, I get frequent messages about modified bash files, e.g.:

rule
Modify Shell Configuration File

container.id
host

fd.name
/var/lib/containerd/tmpmounts/containerd-mount584469562/root/.bashrc

Because this is a managed cluster, I can't influence what the nodes/host is doing and what processes might modify the bash config, so as a user, I'd like to be able to whitelist this false positive.

Special notes for your reviewer:

I can open an issue if required, but I think it's only a minor change.

Does this PR introduce a user-facing change?:

rule(macro user_known_shell_config_modifiers): allow to allowlist shell config modifiers

@poiana
Copy link
Contributor

poiana commented Mar 13, 2022

Welcome @claudio-vellage! It looks like this is your first PR to falcosecurity/falco 🎉

@poiana poiana added the size/XS label Mar 13, 2022
@leogr
Copy link
Member

leogr commented Mar 14, 2022

/milestone 0.32.0

/cc @Kaizhe

@poiana poiana requested a review from Kaizhe March 14, 2022 16:42
@poiana poiana added this to the 0.32.0 milestone Mar 14, 2022
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

LGTM

/approve

@poiana
Copy link
Contributor

poiana commented Mar 14, 2022

LGTM label has been added.

Git tree hash: c78ce5098504520e7c7565d3465ce55c9bbff83e

@poiana
Copy link
Contributor

poiana commented Mar 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: claudio-vellage, leogr

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@claudio-vellage
Copy link
Contributor Author

@leodido @mstemm @Kaizhe is someone able to approve this or do you have an ETA when this can be rolled out? If this will take a while, I will use a local copy of the chart. Currently on GKE (on preemptible nodes), I get the following warnings on our staging environment where we've migrated to COS_CONTAINERD:

image

It's crucial to get this merged, because GKE 1.24 and later only support node images that use the containerd runtime. GKE 1.24 will be released to the stable channel on 2022-04-19.

While there is still some time left, we prefer to migrate early so catch potential bugs early and don't get surprised last-minute. I hope I could clarify the importance of this PR a bit and thank you in advance for your feedback.

@poiana poiana merged commit 4705a92 into falcosecurity:master Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants