Add hidden linux kernel module event #2714

OriGlassman · 2023-02-13T16:24:12Z

1. Explain what the PR does

This PR adds a "hidden_kernel_module" event, which periodically checks for a hidden kernel module in your system and emits the hidden kernel module address and name, if that is found.

2. Explain how to test it

./tracee-ebpf -t e=hidden_kernel_module

3. Other comments

CLAassistant · 2023-02-13T16:24:18Z

All committers have signed the CLA.

rafaeldtinoco

You're missing test results and a better PR description.

roikol

hey @OriGlassman

good work here!
i've left some comments for you to consider.

please also use clang-format in the .c and .h files. and use go-fmt on the to .go files.

pkg/ebpf/c/tracee.bpf.c

pkg/ebpf/c/maps.h

pkg/ebpf/c/tracee.bpf.c

pkg/ebpf/c/vmlinux.h

types/trace/trace.go

roikol · 2023-02-20T09:37:31Z

depends on: #2742

once it merges, trace package should be updated in go.mod

yanivagman

Before doing another round of review, please document the new event

aqua-ci · 2023-03-26T08:48:20Z

[ERROR]
GitHub self-host runner 'github-self-hosted_ami-0f12d300b01df6d27_2714-4523689996_arm64c' failed with connecting to GitHub.
Please cancel workflow and wait until infrastructure team investigation.

aqua-ci · 2023-03-26T08:48:21Z

[ERROR]
GitHub self-host runner 'github-self-hosted_ami-0ffd52e93f1a0370b_2714-4523689996_arm64c' failed with connecting to GitHub.
Please cancel workflow and wait until infrastructure team investigation.

aqua-ci · 2023-03-26T08:48:25Z

[ERROR]
GitHub self-host runner 'github-self-hosted_ami-0f14a28ff0b2d6279_2714-4523689996_x64' failed with connecting to GitHub.
Please cancel workflow and wait until infrastructure team investigation.

aqua-ci · 2023-03-26T08:48:25Z

[ERROR]
GitHub self-host runner 'github-self-hosted_ami-0f12d300b01df6d27_2714-4523689996_arm64' failed with connecting to GitHub.
Please cancel workflow and wait until infrastructure team investigation.

aqua-ci · 2023-03-26T08:48:29Z

[ERROR]
GitHub self-host runner 'github-self-hosted_ami-0d40904002284d8de_2714-4523689996_arm64c' failed with connecting to GitHub.
Please cancel workflow and wait until infrastructure team investigation.

aqua-ci · 2023-03-26T08:48:29Z

[ERROR]
GitHub self-host runner 'github-self-hosted_ami-0f14a28ff0b2d6279_2714-4523689996_x64c' failed with connecting to GitHub.
Please cancel workflow and wait until infrastructure team investigation.

aqua-ci · 2023-03-26T08:48:33Z

[ERROR]
GitHub self-host runner 'github-self-hosted_ami-0f23165db12015479_2714-4523689996_x64' failed with connecting to GitHub.
Please cancel workflow and wait until infrastructure team investigation.

aqua-ci · 2023-03-26T08:48:51Z

[ERROR]
GitHub self-host runner 'github-self-hosted_ami-0f23165db12015479_2714-4523689996_x64c' failed with connecting to GitHub.
Please cancel workflow and wait until infrastructure team investigation.

yanivagman

Replied to few comments.
I will do a more thorough review tomorrow.

yanivagman · 2023-03-28T15:21:27Z

pkg/ebpf/c/common/consts.h

@@ -18,6 +18,7 @@
 #define SEND_META_SIZE      28

 #define MAX_MEM_DUMP_SIZE   127
+#define MAX_NUM_MODS        600       // number of iterations - value that the verifier was seen to cope with - the higher, the better.


This macro name is not clear enough - does MODS imply modules?
Also - why should we put this under common? isn't it only used for your event?

Changed it to MAX_NUM_MODULES.

Where do you recommend to put it?

yanivagman · 2023-03-28T15:26:55Z

pkg/ebpf/c/maps.h

+#define BPF_MAP_QUEUE(_name, _type, _value_type, _max_entries)                                     \
+    struct {                                                                                       \
+        __uint(type, _type);                                                                       \
+        __uint(max_entries, _max_entries);                                                         \
+        __type(value, _value_type);                                                                \
+    } _name SEC(".maps");
+


key size must be zero - https://docs.kernel.org/bpf/map_queue_stack.html

yanivagman · 2023-03-28T15:31:17Z

pkg/ebpf/c/tracee.bpf.c

+// If we won't create these maps, the userspace needs to be kernel version aware as well,
+// otherwise it'll try to interact with the maps and report an error


Where does userspace try to access them?

yanivagman · 2023-03-29T08:36:01Z

pkg/ebpf/c/maps.h

+#define BPF_MAP_QUEUE(_name, _type, _value_type, _max_entries)                                     \
+    struct {                                                                                       \
+        __uint(type, _type);                                                                       \
+        __uint(max_entries, _max_entries);                                                         \
+        __type(value, _value_type);                                                                \
+    } _name SEC(".maps");
+


No, this struct should be defined without key_type, since key is not part of the Queue concept by definition.
This macro should be kept, but let's remove the need to pass the BPF_MAP_TYPE_QUEUE argument since it will always be the same

pkg/ebpf/c/common/consts.h

yanivagman · 2023-03-29T08:40:35Z

pkg/ebpf/c/tracee.bpf.c

+// If we won't create these maps, the userspace needs to be kernel version aware as well,
+// otherwise it'll try to interact with the maps and report an error


Of course you need to create the maps if we access them from userspace, what I don't understand is why do we need to add a comment about that? What is special here comparing to other hash maps defined in the code? How does it relate to the kernel version? hash maps were available in older kernels as well (unlike queue maps)

pkg/ebpf/c/tracee.bpf.c

pkg/events/derive/hidden_kernel_module.go

yanivagman

LGTM.
Nice work!

OriGlassman force-pushed the lkm_finder branch 2 times, most recently from c95f795 to 9962514 Compare February 13, 2023 16:25

yanivagman linked an issue Feb 15, 2023 that may be closed by this pull request

Add hidden linux kernel rootkit detection #2643

Closed

rafaeldtinoco suggested changes Feb 16, 2023

View reviewed changes

OriGlassman force-pushed the lkm_finder branch 2 times, most recently from f332f93 to bae4d9b Compare February 16, 2023 15:30

OriGlassman requested a review from rafaeldtinoco February 16, 2023 15:39

rafaeldtinoco added this to the v0.12.0 milestone Feb 17, 2023

rafaeldtinoco added area/ebpf area/events kind/feature labels Feb 17, 2023

rafaeldtinoco assigned OriGlassman Feb 17, 2023

roikol suggested changes Feb 19, 2023

View reviewed changes

OriGlassman force-pushed the lkm_finder branch 4 times, most recently from 107756a to 9c9bd2c Compare February 19, 2023 17:31

OriGlassman requested review from roikol and removed request for rafaeldtinoco February 20, 2023 13:06

OriGlassman force-pushed the lkm_finder branch from 9c9bd2c to 6b575de Compare February 20, 2023 14:24

rafaeldtinoco self-requested a review February 20, 2023 14:59

OriGlassman force-pushed the lkm_finder branch 2 times, most recently from 9c040bb to 4b59e14 Compare February 20, 2023 16:17

rafaeldtinoco added the milestone/v0.12.0 label Feb 21, 2023

OriGlassman force-pushed the lkm_finder branch 4 times, most recently from e1dcf3b to 6b3665a Compare February 23, 2023 14:16

OriGlassman force-pushed the lkm_finder branch 3 times, most recently from 6a7af17 to ac57319 Compare March 21, 2023 15:25

github-actions bot removed the area/testing label Mar 21, 2023

OriGlassman force-pushed the lkm_finder branch 3 times, most recently from cbabf3e to 600a867 Compare March 22, 2023 09:11

OriGlassman requested a review from yanivagman March 22, 2023 09:38

yanivagman requested changes Mar 23, 2023

View reviewed changes

OriGlassman force-pushed the lkm_finder branch 2 times, most recently from 9721e3f to 5402447 Compare March 26, 2023 08:38

github-actions bot added the kind/documentation label Mar 26, 2023

OriGlassman requested a review from yanivagman March 26, 2023 08:40

yanivagman reviewed Mar 28, 2023

View reviewed changes

OriGlassman force-pushed the lkm_finder branch from 5402447 to 0bbec67 Compare March 28, 2023 16:49

yanivagman requested changes Mar 29, 2023

View reviewed changes

OriGlassman force-pushed the lkm_finder branch from cdd1515 to e326ae1 Compare March 30, 2023 11:49

Add hidden linux kernel module event

799f77a

OriGlassman force-pushed the lkm_finder branch from e326ae1 to 799f77a Compare March 30, 2023 11:57

yanivagman approved these changes Mar 30, 2023

View reviewed changes

rafaeldtinoco approved these changes Mar 30, 2023

View reviewed changes

rafaeldtinoco merged commit b5b6a1c into aquasecurity:main Mar 30, 2023

		// If we won't create these maps, the userspace needs to be kernel version aware as well,
		// otherwise it'll try to interact with the maps and report an error

Add hidden linux kernel module event #2714

Add hidden linux kernel module event #2714

Uh oh!

Conversation

Uh oh!

1. Explain what the PR does

2. Explain how to test it

3. Other comments

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!