Skip to content

feat: extend string data filtering to LSM related events #4590

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rscampos
Copy link
Collaborator

@rscampos rscampos commented Feb 14, 2025

1. Explain what the PR does

1591fd0 chore: install deps
3eae8ff fix: increase timeout for go test
8e8ec5f test: external triggers for integration
6e2f4f1 feat(ebpf): extend string data filtering for LSM events
5a22b36 feat: allow different field names

3eae8ff fix: increase timeout for go test

- Since we have added integration tests and the default Go test
timeout is 10 minutes, we need to increase it otherwise it get panic.

8e8ec5f test: external triggers for integration

- Add external scripts to be triggered in order to test data filter
related to events that uses LSM.

6e2f4f1 feat(ebpf): extend string data filtering for LSM events

- Only for LSM related events.

5a22b36 feat: allow different field names

- Allow any field name in the in-kernel string filter.
- Currently, only one string-type field name is supported.
- Future support for multiple field names is planned.
- Start with LSM related events.

2. Explain how to test it

3. Other comments

This PR focuses only on LSM hooks and the related tests. Some tests were added to the integration test suite with external C program triggers.

part of #4432

Event String Name Trigger
✅ security_bprm_check pathname 5
✅ security_file_open pathname 1 (already present)
✅ security_inode_unlink pathname 3
✅ security_sb_mount path 6
✅ security_bpf_map map_name 9
✅ security_kernel_read_file pathname 4
✅ security_inode_mknod file_name 7
✅ security_kernel_post_read_file pathname 4
✅ security_inode_symlink linkpath 3
✅ security_mmap_file pathname 2 (already present)
✅ security_file_mprotect pathname 5
✅ security_inode_rename old_path 3
✅ security_bpf_prog name 9
✅ security_path_notify pathname 8
✅ shared_object_loaded pathname 5
Trigger Name
1 comm: event: data: trace event security_file_open set in multiple policies using multiple filter types
2 comm: event: data: trace event security_mmap_file using multiple filter types
3 event: data: trace event security_inode_symlink, security_inode_rename and security_inode_unlink using data filter
4 event: data: trace event security_kernel_read_file and security_kernel_post_read_file using data filter
5 comm: event: data: trace event shared_object_loaded, security_file_mprotect and security_bprm_check using data filter
6 event: data: trace event security_sb_mount using data filter
7 event: data: trace event security_inode_mknod using data filter
8 event: data: trace event security_path_notify using data filter
9 event: data: trace event security_bpf_prog and security_bpf_map using data filter

@rscampos rscampos marked this pull request as draft February 14, 2025 18:57
@rscampos rscampos force-pushed the data_filter_in_kernel_phase2_lsm branch 5 times, most recently from d9b223e to 4b175f1 Compare February 17, 2025 22:02
@rscampos rscampos force-pushed the data_filter_in_kernel_phase2_lsm branch from cacfb19 to da57dc6 Compare February 18, 2025 10:24
@rscampos rscampos changed the title [WIP] feat: extend string data filtering to LSM related events feat: extend string data filtering to LSM related events Feb 18, 2025
@rscampos rscampos requested a review from geyslan February 18, 2025 10:24
@rscampos rscampos added this to the v0.24.0 milestone Feb 18, 2025
@rscampos rscampos marked this pull request as ready for review February 18, 2025 10:24
@rscampos rscampos force-pushed the data_filter_in_kernel_phase2_lsm branch 2 times, most recently from 1ebaa4e to d79bf0d Compare February 25, 2025 12:48
Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

LGTM. Put some words.

@rscampos rscampos force-pushed the data_filter_in_kernel_phase2_lsm branch 5 times, most recently from 8e812f8 to 28517c5 Compare February 28, 2025 14:49
Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

LGTM. Put some doubts.

@rscampos rscampos force-pushed the data_filter_in_kernel_phase2_lsm branch from 28517c5 to 650b6b9 Compare March 5, 2025 21:42
Copy link
Collaborator

@NDStrahilevitz NDStrahilevitz left a comment

Choose a reason for hiding this comment

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

I think the PR would benefit from integrating the kernel filtering setting into the event definition. On the rest I trust @geyslan's review.

@@ -61,6 +72,35 @@ func NewDataFilter() *DataFilter {
}
}

// list of events and field names allowed to have in-kernel filter
var allowedKernelField = map[events.ID]string{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this fit in the event definition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point @NDStrahilevitz ...I've created this just to have a simple way to enable data filtering for events that have string filters in this phase. The next phase (phase 3) will implement filtering for other types of events. So, the idea is to remove this temporary table and use the event definition as you mentioned. However, to reach this step, I'll need to design a solution to accommodate all types of string filters.

rscampos added 2 commits March 6, 2025 13:34
- Allow any field name in the in-kernel string filter.
- Currently, only one string-type field name is supported.
- Future support for multiple field names is planned.
- Start with LSM related events.
@rscampos rscampos force-pushed the data_filter_in_kernel_phase2_lsm branch from 650b6b9 to cdc67a6 Compare March 6, 2025 19:34
rscampos added 3 commits March 6, 2025 14:13
- Add external scripts to be triggered in order to test data filter
related to events that uses LSM.
- Since we have added integration tests and the default Go test
timeout is 10 minutes, we need to increase it otherwise it get panic.
@rscampos rscampos force-pushed the data_filter_in_kernel_phase2_lsm branch from cdc67a6 to 1591fd0 Compare March 6, 2025 20:13
@geyslan geyslan removed this from the v0.24.0 milestone Mar 31, 2025
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.

3 participants