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

Added rule to detect docker client in container #915

Merged
merged 2 commits into from
Nov 8, 2019
Merged

Added rule to detect docker client in container #915

merged 2 commits into from
Nov 8, 2019

Conversation

daviddetorres
Copy link
Contributor

@daviddetorres daviddetorres commented Nov 4, 2019

What type of PR is this?

/kind feature
/kind rule-create

Any specific area of the project related to this PR?

/area rules

What this PR does / why we need it:

The motivation of this new rule is to detect suspicious activity like seen in the Graboid threat (more details here: https://unit42.paloaltonetworks.com/graboid-first-ever-cryptojacking-worm-found-in-images-on-docker-hub/).

There are rules for detection of miners, but not the scenario where the container is used to attack other vulnerable hosts via a docker client.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:
The priority is set to WARNING, but I have doubts if it should be other. I don't have the experience enough to decide this part of the rule.

Does this PR introduce a user-facing change?:
NONE

rules(The docker client is executed in a container): detect the execution of the docker client in a container and logs it with WARNING priority. 
rules(list k8s_client_binaries): create and add docker, kubectl, crictl

@poiana
Copy link

poiana commented Nov 4, 2019

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

rules/falco_rules.yaml Outdated Show resolved Hide resolved
# This rule detects the execution of the docker client inside a container
- rule: The docker client is executed in a container
desc: An event will trigger every time you run a docker client in a container
condition: evt.type = execve and evt.dir=< and container.id != host and proc.name in (docker_binaries)
Copy link
Contributor

@Kaizhe Kaizhe Nov 5, 2019

Choose a reason for hiding this comment

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

can you make the changes to

- list: k8s_client_binaries
   items: [docker, kubectl, crictl]

- rule: K8s client tool executed inside a container
condition: spawned_process and container and proc.name in (k8s_client_binaries)

tags: [container, mitre_execution]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comments, Kaizhe. I didn't realize the macros for spawned process and container.

I'll modify the pull request and upload the changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I need some help for adding DCO signoff to a previous commit of this PR... I resolved a code review and forgot to add it. How could I add the DCO to an already existing commit?
(Sorry for the newbie mistake...).

Copy link
Contributor

Choose a reason for hiding this comment

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

No, not a problem, I had this issue before :) take a look this one: https://stackoverflow.com/questions/13043357/git-sign-off-previous-commits

Copy link
Contributor

Choose a reason for hiding this comment

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

Please rebase from dev too

@poiana poiana added size/S and removed size/XS labels Nov 6, 2019
- rule: The docker client is executed in a container
desc: An event will trigger every time you run a docker client in a container
condition: spawned_process and container and proc.name in (k8s_client_binaries)
output: "Docker or kubernetes client in container (user=%user.name %container.info parent=%proc.pname cmdline=%proc.cmdline)"
Copy link
Contributor

@Kaizhe Kaizhe Nov 6, 2019

Choose a reason for hiding this comment

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

Minor:
ouptut: Docker or kubernetes client executed in container
desc: Detect a k8s client tool executed inside a container

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Kaizhe for your support and help with my first PR in Falco... I'll commit this changes that you suggest, but before I'd like to fix the signature problem that it is giving in the this commit.

I followed the link that you suggested and I added a gpg key (verified) and also a "Signed-off-by" in the commit. Even though, the problem persist... Any clue about how to fix it?

Copy link
Contributor

Choose a reason for hiding this comment

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

can you try rebase HEAD~3 and squash the one didn't commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems it worked at last! Thanks again for the support :)

Commit pushed with your suggestions.

The rule detects the execution of the k8s client tool in a container and
logs it with WARNING priority.

Signed-off-by: David de Torres <detorres.david@gmail.com>
@@ -2599,3 +2599,9 @@
# there if you want to enable them by adding to
# falco_rules.local.yaml.

- rule: The docker client is executed in a container
desc: Detect a k8s client tool executed inside a container
condition: spawned_process and container and proc.name in (k8s_client_binaries)
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you accidentally delete the k8s_client_binaries list? And please move the rule above

# Application rules have moved to application_rules.yaml. Please look
# there if you want to enable them by adding to
# falco_rules.local.yaml.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks again Kaizhe. You are really being patience with me in this PR...

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for your contribution! It is really appreciated!

Added accidentally deleted lines for the list of k8s client binaries.

Signed-off-by: David de Torres <detorres.david@gmail.com>
@Kaizhe
Copy link
Contributor

Kaizhe commented Nov 7, 2019

/lgtm

@poiana poiana added the lgtm label Nov 7, 2019
@poiana
Copy link

poiana commented Nov 7, 2019

LGTM label has been added.

Git tree hash: 504143d6001bdfe5dff3c38cb8c4bcad52141c2f

@poiana poiana added the approved label Nov 7, 2019
@poiana
Copy link

poiana commented Nov 8, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, Kaizhe

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

@fntlnz
Copy link
Contributor

fntlnz commented Nov 8, 2019

Thanks for improving our ruleset @daviddetorres !!!

@fntlnz fntlnz merged commit ed76756 into falcosecurity:dev Nov 8, 2019
@daviddetorres daviddetorres deleted the new-rule-detect-docker-client-in-container branch November 19, 2019 22:09
@fntlnz fntlnz added this to the 0.19.0 milestone Dec 2, 2019
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

5 participants