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 exe_running_docker_save in the "Create Hidden Files or Directories" and "Mkdir binary dirs" rules #1386

Merged
merged 2 commits into from
Jan 8, 2021

Conversation

jhwbarlow
Copy link

@jhwbarlow jhwbarlow commented Sep 8, 2020

What type of PR is this?

/kind bug

/kind rule-update

Any specific area of the project related to this PR?

/area rules

What this PR does / why we need it:

Adds exe_running_docker_save as an exception to the "Create Hidden Files or Directories" and "Mkdir binary dirs" rules, as these rules can be triggerred when a container is created. This brings these rules in line with similar rules which already contain this exception.

Does this PR introduce a user-facing change?:

Yes.

rules(Mkdir binary dirs): Adds exe_running_docker_save as an exception as this rules can be triggerred when a container is created.
rules(Create Hidden Files): Adds exe_running_docker_save as an exception as this rules can be triggerred when a container is created.

@poiana
Copy link

poiana commented Sep 8, 2020

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

@poiana poiana added the size/XS label Sep 8, 2020
@jhwbarlow
Copy link
Author

/assign @mstemm

@leodido
Copy link
Member

leodido commented Sep 8, 2020

/cc @Kaizhe

@poiana poiana requested a review from Kaizhe September 8, 2020 16:44
@Kaizhe
Copy link
Contributor

Kaizhe commented Sep 8, 2020

@jhwbarlow , thank you for the PR. I need couple things from this PR, can you please provide more detail about:

  1. Which docker version did you trigger rule?
  2. What was the event look like ?

Thanks a lot!

Kaizhe

@jhwbarlow
Copy link
Author

Hi @Kaizhe

uname -r; docker --version
4.19.140-flatcar
Docker version 18.06.3-ce, build d7080c1

The "Mkdir binary rules" can be triggered with a Dockerfile like so:

FROM alpine
RUN mkdir /usr/bin/james-was-here
CMD sleep 99d

Results in:

{"output":"15:28:21.995837530: Error Directory below known binary directory created (user=root command=exe /var/lib/docker/overlay2/437d9e72822e5e410d49407e7b5bde22caa2ebb5e5ff288143cd76da9ab6f0b6/diff directory=/usr/bin/james-was-here container_id=host image=<NA>) k8s.ns=<NA> k8s.pod=<NA> container=host k8s.ns=<NA> k8s.pod=<NA> container=host k8s.ns=<NA> k8s.pod=<NA> container=host k8s.ns=<NA> k8s.pod=<NA> container=host","priority":"Error","rule":"Mkdir binary dirs","time":"2020-09-04T15:28:21.995837530Z", "output_fields": {"container.id":"host","container.image.repository":null,"evt.arg.path":"/usr/bin/james-was-here","evt.time":1599233301995837530,"k8s.ns.name":null,"k8s.pod.name":null,"proc.cmdline":"exe /var/lib/docker/overlay2/437d9e72822e5e410d49407e7b5bde22caa2ebb5e5ff288143cd76da9ab6f0b6/diff","user.name":"root"}}

For the other rule, I wasn't able to provoke an alert as it is at NOTICE level, and we run our Falco only at ERROR, but it was changed for consistency with the other rules.

@krisnova krisnova added this to the 0.27.0 milestone Sep 24, 2020
@poiana
Copy link

poiana commented Dec 23, 2020

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

@jhwbarlow
Copy link
Author

/remove-lifecycle stale

@leogr
Copy link
Member

leogr commented Jan 7, 2021

/cc @Kaizhe

@Kaizhe
Copy link
Contributor

Kaizhe commented Jan 7, 2021

/lgtm

@poiana
Copy link

poiana commented Jan 7, 2021

LGTM label has been added.

Git tree hash: 9fa199ad9e4a5300c67e126c9eaca6b47fa2db6f

Kaizhe
Kaizhe previously approved these changes Jan 7, 2021
@poiana poiana added the approved label Jan 7, 2021
leogr
leogr previously approved these changes Jan 8, 2021
@leogr
Copy link
Member

leogr commented Jan 8, 2021

/close

@poiana poiana closed this Jan 8, 2021
@poiana
Copy link

poiana commented Jan 8, 2021

@leogr: Closed this PR.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@leogr
Copy link
Member

leogr commented Jan 8, 2021

/reopen

@poiana
Copy link

poiana commented Jan 8, 2021

@leogr: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@poiana poiana reopened this Jan 8, 2021
fntlnz
fntlnz previously approved these changes Jan 8, 2021
leodido
leodido previously approved these changes Jan 8, 2021
James Barlow added 2 commits January 8, 2021 18:15
Signed-off-by: James Barlow <james.barlow@finbourne.com>
…save

Signed-off-by: James Barlow <james.barlow@finbourne.com>
@leogr leogr dismissed stale reviews from leodido, fntlnz, Kaizhe, and themself via 1acf106 January 8, 2021 17:16
@poiana poiana removed the lgtm label Jan 8, 2021
@poiana poiana added the lgtm label Jan 8, 2021
@poiana
Copy link

poiana commented Jan 8, 2021

LGTM label has been added.

Git tree hash: 738b8c0309d722f92a65f61b48118d4b9b397dba

@poiana
Copy link

poiana commented Jan 8, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, Kaizhe, leodido, 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

@poiana poiana merged commit 7f33b08 into falcosecurity:master Jan 8, 2021
@fntlnz
Copy link
Contributor

fntlnz commented Jan 15, 2021

Updated the release notes to follow our requirements for the release.

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.

None yet

8 participants