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

enforcer: track missed notifications #2994

Merged
merged 12 commits into from
Oct 16, 2024

Conversation

kkourt
Copy link
Contributor

@kkourt kkourt commented Oct 11, 2024

Builds on #2986

See commits for details.

enforcer: add  `tetragon_enforcer_missed_notifications_total` metric

@kkourt kkourt added the release-note/minor This PR introduces a minor user-visible change label Oct 11, 2024
@kkourt kkourt force-pushed the pr/kkourt/enforcer-missed-notifications branch 3 times, most recently from 07368f5 to 6fae399 Compare October 11, 2024 13:01
@kkourt kkourt force-pushed the pr/kkourt/syscall64-updates branch from 0f2406c to 67fd599 Compare October 11, 2024 13:11
Base automatically changed from pr/kkourt/syscall64-updates to main October 11, 2024 14:06
@kkourt kkourt force-pushed the pr/kkourt/enforcer-missed-notifications branch from 6fae399 to 3e413e7 Compare October 11, 2024 14:07
@kkourt kkourt marked this pull request as ready for review October 11, 2024 14:07
@kkourt kkourt requested review from a team and mtardy as code owners October 11, 2024 14:07
Copy link

netlify bot commented Oct 11, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 3e1e75d
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/670e35e70f8e830008c517b3
😎 Deploy Preview https://deploy-preview-2994--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kkourt
Copy link
Contributor Author

kkourt commented Oct 11, 2024

Moving to draft to figure out the CI failures.

@kkourt kkourt marked this pull request as draft October 11, 2024 15:00
@kkourt kkourt force-pushed the pr/kkourt/enforcer-missed-notifications branch 2 times, most recently from 3e1e75d to feee959 Compare October 15, 2024 09:34
@kkourt kkourt marked this pull request as ready for review October 15, 2024 12:07
@kkourt kkourt force-pushed the pr/kkourt/enforcer-missed-notifications branch from feee959 to b8f93d7 Compare October 15, 2024 14:24
Copy link
Contributor

@olsajiri olsajiri left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

pkg/sensors/sensors.go Show resolved Hide resolved
pkg/sensors/sensors.go Show resolved Hide resolved
DEPSDIR has a trailing slash so $(DEPSDIR)/foo.d does not match the
dependency file and the rule is ignored. Fix this.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Add an AddPostUnloadHook that allows multiple components to add a
PostUnloadHook to a sensor.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Add helpers for enforcer map. Currently, there is only one map but a
susbsequent commit will introduce another one. Have two calls: one for
the enforcer sensor, which is the owner of the map and one for the
kprobe sensors which are the users.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
In the previous patches, we split the enforcer map users into owners
(the enforcer sensor) and users (the tracing sensors). Given this
change, we need to load the enforcer sensor first. So ensure that this
happens.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
It will be used in subsequent patches.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This commit adds a enforcer map to track missed
notifications from the enforcer. Notifications are missed when the
enforcer bpf program did not run after the notification happened. This
can, for example, if there is a NotifyEnforcer action from the
raw_syscalls/sys_enter tracepoint without having the enforcer loaded
into this program.

For example:

```
apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: "kill-syscalls"
spec:
  enforcers:
  - calls:
    - "sys_swapoff"
    # - "sys_getcpu" <-- missing entry
  tracepoints:
  - subsystem: "raw_syscalls"
    event: "sys_enter"
    args:
    - index: 4
      type: "syscall64"
    selectors:
    - matchArgs:
      - index: 0
        operator: "InMap"
        values:
        - 309 # getcpu
        - 168 # swapoff
      matchActions:
      - action: "NotifyEnforcer"
        argError: -1
        argSig: 9
```

The current version of the code, checks the enforcer map when it updates
it for existing entries. Subsequent patches will improve this.

Executing two getcpu calls from the same thread when the above policy is
loaded, will result in the following entries:

```
bpftool map dump  pinned /sys/fs/bpf/tetragon/kill-syscalls/enforcer_missed_notifications
[{
        "key": {
            "act_info": {
                "func_id": 0,
                "arg": 309
            },
            "reason": 1
        },
        "value": 1
    }
]
```

Note that the syscall id is recorded so that we can know what was the
missed syscall.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This commit adds a metric for enforcer missed notifications.
The metric is implemented by reading the bpf map introduced in the
previous patch.

The metric is a counter with the system call name, the policy and the reason as labels.
tetragon_enforcer_missed_notifications_total{info="getcpu",policy="kill-syscalls",reason="overwritten"} 2

When the policy is unloaded, the metric is removed.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Previous commits introduced a bpf map to track missed enforcer
notifications, by checking the enforcer map for existing entries when
attempting to set a new one.

As an improvement, we want to introduce an action allowing to check for
enforcer missed notifications. The intention is to use this action when
exiting system calls.

This commit introduces the policy and proto changes.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This allows to specify an action (CleanupEnforcerNotification) that
checks whether the enforcer notification succeeded.

For example, the following policy:

```
apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: "kill-syscalls"
spec:
  enforcers:
  - calls:
    - "sys_swapoff"
    # - "sys_getcpu"
  tracepoints:
  - subsystem: "raw_syscalls"
    event: "sys_enter"
    args:
    - index: 4
      type: "syscall64"
    selectors:
    - matchArgs:
      - index: 0
        operator: "InMap"
        values:
        - 309 # getcpu
        - 168 # swapoff
      matchActions:
      - action: "NotifyEnforcer"
        argError: -1
        argSig: 9
  - subsystem: "raw_syscalls"
    event: "sys_exit"
    selectors:
    - matchActions:
      - action: "CleanupEnforcerNotification"
      - action: "NoPost"
```

Detects missing notifications by adding such a function in the
raw_syscalls/sys_exit tracepoint. This means that missed enforcements
can be detected even if there is no other notifcation for the same
thread.

The metric output in this case would be:
tetragon_enforcer_missed_notifications_total{info="getcpu",policy="kill-syscalls",reason="no_action"} 3

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
@kkourt kkourt force-pushed the pr/kkourt/enforcer-missed-notifications branch from b8f93d7 to 9c49fc4 Compare October 16, 2024 08:41
@kkourt kkourt merged commit 48e51a7 into main Oct 16, 2024
52 checks passed
@kkourt kkourt deleted the pr/kkourt/enforcer-missed-notifications branch October 16, 2024 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants