Added rule to detect LKM module injects using insmod used by rootkits for kernel hooking #1401

josehelps · 2020-09-14T21:18:31Z

What type of PR is this?

/kind rule-create

Any specific area of the project related to this PR?

/area rules

What this PR does / why we need it:

This PR adds a new falco rule that looks for when insmod is called as part of a execve event. Injecting LKM modules on (post build) running production instances should be rare and is a common way for rootkits that employ kernel hooking to obfuscate themselves.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

This is a rehashed of PR#1389 just cleaned up DCO

Does this PR introduce a user-facing change?:

Yes new rule in falco_rules.yml

rule(Linux Kernel Module injection detected): adds a new rule that detects when a LKM module is inject using insmod typically used by rookits looking to obfuscate their behavior via kernel hooking.

Signed-off-by: divious1 <[email protected]>

josehelps · 2020-09-14T21:34:49Z

/assign @Kaizhe

leogr · 2020-09-15T09:14:13Z

Thanks @d1vious !

PS: I have just edited the PR description to fix the release note block syntax.

josehelps · 2020-09-15T12:48:30Z

Thank you 😊

leodido

Thanks @d1vious!

Waiting for @Kaizhe review but just a comment: shouldn't this new macro exclude the Falco kernel module ?

josehelps · 2020-10-05T15:47:43Z

@leodido we can add exclusion for a bunch of commonly used modules, if you like we can collage a list and tack it to the end of the query. Otherwise I suspect a customer would have to do some tuning.

leogr

Thank you for this PR 👍

The rule seems good to me, but I believe it worth adding an exclusion list for known modules.
IMHO, the list should at least include the Falco kernel module, then users can extend it.

Kaizhe · 2020-10-12T23:53:22Z

rules/falco_rules.yaml

@@ -3010,6 +3010,12 @@
  output: Drift detected (open+create), new executable created in a container (user=%user.name user_loginuid=%user.loginuid command=%proc.cmdline filename=%evt.arg.filename name=%evt.arg.name mode=%evt.arg.mode event=%evt.type)
  priority: ERROR

+# find when a new kernel module is injected
+- rule: Linux Kernel Module injection detected


Can you please capitalized the beginning letter of every word?

Kaizhe · 2020-10-12T23:55:53Z

rules/falco_rules.yaml

@@ -3010,6 +3010,12 @@
  output: Drift detected (open+create), new executable created in a container (user=%user.name user_loginuid=%user.loginuid command=%proc.cmdline filename=%evt.arg.filename name=%evt.arg.name mode=%evt.arg.mode event=%evt.type)
  priority: ERROR

+# find when a new kernel module is injected
+- rule: Linux Kernel Module injection detected
+  desc: It is very uncommon for kernel modules to be injected in running production instances, used rookits to obfuscate their behavior via kernel hooking 


Can you rephrase a little bit of the description to something as simple as

Detect kernel module was injected (from container).

Kaizhe · 2020-10-12T23:57:07Z

rules/falco_rules.yaml

+# find when a new kernel module is injected
+- rule: Linux Kernel Module injection detected
+  desc: It is very uncommon for kernel modules to be injected in running production instances, used rookits to obfuscate their behavior via kernel hooking 
+  condition: evt.type=execve and proc.name=insmod


I would suggest to limit to activities happen from inside a container. So the condition would be something like:
spawned_process and container and proc.name=insmod

Agreed with @leogr , need to have a whitelisted images/containers that perform kernel module injection like falco.

Kaizhe · 2020-10-12T23:57:19Z

rules/falco_rules.yaml

+  desc: It is very uncommon for kernel modules to be injected in running production instances, used rookits to obfuscate their behavior via kernel hooking 
+  condition: evt.type=execve and proc.name=insmod
+  output: Linux Kernel Module injection using insmod detected (user=%user.name user_loginuid=%user.loginuid parent_process=%proc.pname module=%proc.args)
+  priority: WARNING


Please add tags fields with process

leogr · 2020-10-16T12:50:09Z

Hey @d1vious

Could you also rebase this PR, please?

Signed-off-by: Leonardo Grasso <[email protected]>

On previously modified files. Signed-off-by: Leonardo Grasso <[email protected]>

Signed-off-by: Leonardo Grasso <[email protected]>

…lco_common Signed-off-by: Leonardo Grasso <[email protected]>

…_started` macro Signed-off-by: Leonardo Di Donato <[email protected]>

Signed-off-by: Leonardo Grasso <[email protected]>

Reviewed-by: Spencer Krum <[email protected]> Reviewed-by: Leonardo Grasso <[email protected]> Reviewed-by: Leonardo Di Donato <[email protected]> Signed-off-by: Lorenzo Fontana <[email protected]>

Previously, formatters were freed by LUA code when re-opening outputs. Since now, outputs are not controlling anymore the falco_formats class (see #1412), we just free formatters only if were already initialized. That is needed when the engine restarts (see #1446). By doing so, we also ensure that correct inspector instance is set to the formatter cache. Signed-off-by: Leonardo Grasso <[email protected]>

Co-Authored-By: Leonardo Di Donato <[email protected]> Signed-off-by: Leonardo Grasso <[email protected]>

Use the right list name in the rule Full K8s Administrative Access--it was using the nonexistent list admin_k8s_users, so it was just using the string "admin_k8s_users". Signed-off-by: Mark Stemm <[email protected]>

…cts storage proposals Co-authored-by: Lorenzo Fontana <[email protected]> Signed-off-by: Leonardo Di Donato <[email protected]>

Co-authored-by: Lorenzo Fontana <[email protected]> Signed-off-by: Leonardo Di Donato <[email protected]>

…prefix Signed-off-by: Christian Zunker <[email protected]>

poiana · 2020-11-03T02:26:14Z

@d1vious: Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

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.

Signed-off-by: divious1 <[email protected]>

…dd_lkm_inject_rule

poiana · 2020-11-03T02:34:18Z

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

📝 Please follow instructions in the contributing guide to update your commits with the DCO

Full details of the Developer Certificate of Origin can be found at developercertificate.org.

The list of commits missing DCO signoff:

b963dda fixed merge conflix
b3630ce Merge branch 'add_lkm_inject_rule' of github.com:d1vious/falco into add_lkm_inject_rule

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. I understand the commands that are listed here.

poiana · 2020-11-03T02:34:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign fntlnz
You can assign the PR to them by writing /assign @fntlnz in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

josehelps · 2020-11-03T02:35:59Z

@leogr rebased as requested. Also added the changes you asked for @Kaizhe. Let me know what else I can help with 👍

leogr · 2020-11-03T09:17:31Z

@leogr rebased as requested. Also added the changes you asked for @Kaizhe. Let me know what else I can help with

Hey @d1vious

Thank you, but unfortunately, I see some issues now. There're merge commits in this PR and commits missing the sign-off that are both not allowed in this repo (please check out the comments from our bot). Moreover, integration tests are failing. :(

Thus, I'd suggest reverting the merge then use git rebase -i master please. If you need any help, feel free to contact me via DM on Slack.

josehelps · 2020-11-04T19:13:44Z

@leogr This literally has been more work than is worth IMHO, if you want to rebase and include the changes feel free. I was hoping to contribute back to the project this detection which I found useful in my production environment at Splunk, but contributing here has been extremely cumbersome due to all the steps/hoops required and will likely not contribute again unfortunately. My advice is make this process easier for contributors if you really want the community to be part of the project. Feel free to close the PR if you did not find it useful, otherwise enjoy the free detection 😄

Kaizhe · 2020-11-04T20:51:40Z

rules/falco_rules.yaml

+
+- rule: Linux Kernel Module Injection Detected
+  desc: Detect kernel module was injected (from container).
+  condition: spawned_process and container and evt.type=execve and proc.name=insmod


evt.type=execve is redundant. Can you please remove it?

leogr · 2020-11-05T12:10:38Z

@leogr This literally has been more work than is worth IMHO, if you want to rebase and include the changes feel free. I was hoping to contribute back to the project this detection which I found useful in my production environment at Splunk, but contributing here has been extremely cumbersome due to all the steps/hoops required and will likely not contribute again unfortunately. My advice is make this process easier for contributors if you really want the community to be part of the project. Feel free to close the PR if you did not find it useful, otherwise enjoy the free detection

Hey @d1vious

I'm sorry you felt unhappy with the process we have in place. Your contribution is valuable, and we appreciate it. We never thought it was unuseful.

However, I'm here to help with this PR. So let's try to solve the issue.
We actually just need to rebase it, then @Kaizhe's suggestions can be applied easily.

Unfortunately, GitHub does not allow us to force push on your branch, so I cannot rebase this PR by myself atm. I can propose two solutions (almost effortless for you):

giving me collaborator access to your fork so that I can rebase your branch
or, I can create a new PR if that's ok for you

In both cases, I will cherry-pick and keep your two commits (that have been signed-off by you, so it's perfectly fine) then we will also be able to include our suggestions on top of that.

WDYT?

josehelps · 2020-11-06T03:13:06Z

cleaned up, added the fixes and DCO signature on #1478, closing this one

detect insmod trying to inject LKM modules

c65cb0b

Signed-off-by: divious1 <[email protected]>

poiana added kind/rule-create dco-signoff: yes area/rules do-not-merge/release-note-label-needed labels Sep 14, 2020

poiana requested review from fntlnz and leodido September 14, 2020 21:18

poiana added the size/XS label Sep 14, 2020

josehelps mentioned this pull request Sep 14, 2020

Added rule to detect LKM module injects using insmod used by rootkits for kernel hooking #1389

Closed

poiana assigned Kaizhe Sep 14, 2020

poiana added release-note and removed do-not-merge/release-note-label-needed labels Sep 15, 2020

krisnova added this to the 0.27.0 milestone Sep 24, 2020

leodido reviewed Sep 29, 2020

View reviewed changes

leogr reviewed Oct 8, 2020

View reviewed changes

Kaizhe suggested changes Oct 12, 2020

View reviewed changes

leogr and others added 9 commits October 26, 2020 11:21

update(userspace/falco): move gRPC queue to proper namespace

6e36afd

Signed-off-by: Leonardo Grasso <[email protected]>

chore(userspace/falco): remove leftover from outputs

7b8f67f

Signed-off-by: Leonardo Grasso <[email protected]>

chore: update copyright year to 2020

511a9fa

On previously modified files. Signed-off-by: Leonardo Grasso <[email protected]>

chore(userspace/falco): remove unused dep from falco_outputs

ca04145

Signed-off-by: Leonardo Grasso <[email protected]>

chore(userspace/falco): output class does not need to inherit from fa…

c188f4a

…lco_common Signed-off-by: Leonardo Grasso <[email protected]>

update(falco/rules): re-use spawned_process macro inside `container…

bc9a2f3

…_started` macro Signed-off-by: Leonardo Di Donato <[email protected]>

fix(userspace/falco): use given priority for msg

81de65e

Signed-off-by: Leonardo Grasso <[email protected]>

update(proposals): proposal for moving the drivers to S3

61bfd5a

Reviewed-by: Spencer Krum <[email protected]> Reviewed-by: Leonardo Grasso <[email protected]> Reviewed-by: Leonardo Di Donato <[email protected]> Signed-off-by: Lorenzo Fontana <[email protected]>

leogr and others added 6 commits October 27, 2020 15:12

update(userspace/engine): handle formatters with smart pointer

c8703b8

Co-Authored-By: Leonardo Di Donato <[email protected]> Signed-off-by: Leonardo Grasso <[email protected]>

rule(Full K8s... Access): fix users list

26f2aaa

Use the right list name in the rule Full K8s Administrative Access--it was using the nonexistent list admin_k8s_users, so it was just using the string "admin_k8s_users". Signed-off-by: Mark Stemm <[email protected]>

docs(proposals): new drivers storage obsolate part of existing artifa…

a575625

…cts storage proposals Co-authored-by: Lorenzo Fontana <[email protected]> Signed-off-by: Leonardo Di Donato <[email protected]>

update: new DRIVERS_REPO default

b3679f8

Co-authored-by: Lorenzo Fontana <[email protected]> Signed-off-by: Leonardo Di Donato <[email protected]>

rule(list falco_privileged_images): add calico/node without registry …

294804d

…prefix Signed-off-by: Christian Zunker <[email protected]>

fixed merge conflix

b963dda

poiana added dco-signoff: no and removed dco-signoff: yes labels Nov 3, 2020

poiana added do-not-merge/contains-merge-commits size/S and removed size/XS labels Nov 3, 2020

josehelps added 2 commits November 2, 2020 21:32

detect insmod trying to inject LKM modules

4fb050d

Signed-off-by: divious1 <[email protected]>

Merge branch 'add_lkm_inject_rule' of github.com:d1vious/falco into a…

b3630ce

…dd_lkm_inject_rule

poiana added size/L and removed size/S labels Nov 3, 2020

Kaizhe suggested changes Nov 4, 2020

View reviewed changes

josehelps mentioned this pull request Nov 6, 2020

adding lkm rule #1478

Merged

josehelps closed this Nov 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added rule to detect LKM module injects using insmod used by rootkits for kernel hooking #1401

Added rule to detect LKM module injects using insmod used by rootkits for kernel hooking #1401

josehelps commented Sep 14, 2020 •

edited by leogr

Loading

josehelps commented Sep 14, 2020

leogr commented Sep 15, 2020

josehelps commented Sep 15, 2020

leodido left a comment

josehelps commented Oct 5, 2020

leogr left a comment

Kaizhe Oct 12, 2020

Kaizhe Oct 12, 2020

Kaizhe Oct 12, 2020

Kaizhe Oct 12, 2020

Kaizhe Oct 12, 2020

leogr commented Oct 16, 2020

poiana commented Nov 3, 2020

poiana commented Nov 3, 2020

poiana commented Nov 3, 2020

josehelps commented Nov 3, 2020 •

edited

Loading

leogr commented Nov 3, 2020

josehelps commented Nov 4, 2020

Kaizhe Nov 4, 2020

leogr commented Nov 5, 2020

josehelps commented Nov 6, 2020

Added rule to detect LKM module injects using insmod used by rootkits for kernel hooking #1401

Added rule to detect LKM module injects using insmod used by rootkits for kernel hooking #1401

Conversation

josehelps commented Sep 14, 2020 • edited by leogr Loading

josehelps commented Sep 14, 2020

leogr commented Sep 15, 2020

josehelps commented Sep 15, 2020

leodido left a comment

Choose a reason for hiding this comment

josehelps commented Oct 5, 2020

leogr left a comment

Choose a reason for hiding this comment

Kaizhe Oct 12, 2020

Choose a reason for hiding this comment

Kaizhe Oct 12, 2020

Choose a reason for hiding this comment

Kaizhe Oct 12, 2020

Choose a reason for hiding this comment

Kaizhe Oct 12, 2020

Choose a reason for hiding this comment

Kaizhe Oct 12, 2020

Choose a reason for hiding this comment

leogr commented Oct 16, 2020

poiana commented Nov 3, 2020

poiana commented Nov 3, 2020

poiana commented Nov 3, 2020

josehelps commented Nov 3, 2020 • edited Loading

leogr commented Nov 3, 2020

josehelps commented Nov 4, 2020

Kaizhe Nov 4, 2020

Choose a reason for hiding this comment

leogr commented Nov 5, 2020

josehelps commented Nov 6, 2020

josehelps commented Sep 14, 2020 •

edited by leogr

Loading

josehelps commented Nov 3, 2020 •

edited

Loading