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.

Sign up for GitHub

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

Jump to bottom

Postfix operator for matchBinaries selector #2689

Merged
merged 4 commits into from
Aug 14, 2024
Merged

Postfix operator for matchBinaries selector #2689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2555a1d
bpf: Add postfix/notpostfix operator for matching binaries
anfedotoff Aug 14, 2024
6645d29
tetragon: Add Postfix and NotPostfix operators to matchBinaries selector
anfedotoff Aug 14, 2024
c4760fe
tetragon: Update tests for KprobeMatchBinaries
anfedotoff Aug 14, 2024
6cdfbd4
docs: Update docs for matchBinaries selector
anfedotoff Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions bpf/lib/process.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "bpf_event.h"
#include "bpf_helpers.h"
#include "bpf_cred.h"
#include "../process/string_maps.h"

/* Applying 'packed' attribute to structs causes clang to write to the
* members byte-by-byte, as offsets may not be aligned. This is bad for
Expand Down Expand Up @@ -274,6 +275,7 @@ struct msg_k8s {

struct heap_exe {
char buf[BINARY_PATH_MAX_LEN];
char end[STRING_POSTFIX_MAX_LENGTH];
__u32 len;
__u32 error;
}; // All fields aligned so no 'packed' attribute.
Expand Down Expand Up @@ -311,10 +313,16 @@ typedef __u64 mbset_t;
struct binary {
// length of the path stored in path, this should be < BINARY_PATH_MAX_LEN
// but can contain negative value in case of copy error.
// While s16 would be sufficient, 64 bits are handy for alignment.
__s64 path_length;
// While s16 would be sufficient, 32 bits are handy for alignment.
__s32 path_length;
// if end_r contains reversed path postfix
__u32 reversed;
anfedotoff marked this conversation as resolved.
Show resolved Hide resolved
// BINARY_PATH_MAX_LEN first bytes of the path
char path[BINARY_PATH_MAX_LEN];
// STRING_POSTFIX_MAX_LENGTH last bytes of the path
char end[STRING_POSTFIX_MAX_LENGTH];
// STRING_POSTFIX_MAX_LENGTH reversed last bytes of the path
char end_r[STRING_POSTFIX_MAX_LENGTH];
// matchBinary bitset for binary
kevsecurity marked this conversation as resolved.
Show resolved Hide resolved
mbset_t mb_bitset;
}; // All fields aligned so no 'packed' attribute
Expand Down
17 changes: 15 additions & 2 deletions bpf/process/bpf_execve_event.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ read_exe(struct task_struct *task, struct heap_exe *exe)
{
struct file *file = BPF_CORE_READ(task, mm, exe_file);
struct path *path = __builtin_preserve_access_index(&file->f_path);
__u64 offset = 0;
__u64 revlen = STRING_POSTFIX_MAX_LENGTH - 1;

// we need to walk the complete 4096 len dentry in order to have an accurate
// matching on the prefix operators, even if we only keep a subset of that
Expand All @@ -188,14 +190,20 @@ read_exe(struct task_struct *task, struct heap_exe *exe)
if (!buffer)
return 0;

if (exe->len > STRING_POSTFIX_MAX_LENGTH - 1)
offset = exe->len - (STRING_POSTFIX_MAX_LENGTH - 1);
else
revlen = exe->len;
// buffer used by d_path_local can contain up to MAX_BUF_LEN i.e. 4096 we
// only keep the first 255 chars for our needs (we sacrifice one char to the
// verifier for the > 0 check)
if (exe->len > 255)
exe->len = 255;
if (exe->len > BINARY_PATH_MAX_LEN - 1)
exe->len = BINARY_PATH_MAX_LEN - 1;
anfedotoff marked this conversation as resolved.
Show resolved Hide resolved
asm volatile("%[len] &= 0xff;\n"
: [len] "+r"(exe->len));
probe_read(exe->buf, exe->len, buffer);
if (revlen < STRING_POSTFIX_MAX_LENGTH)
probe_read(exe->end, revlen, (char *)(buffer + offset));

return exe->len;
}
Expand Down Expand Up @@ -378,6 +386,11 @@ execve_send(void *ctx)
curr->bin.path_length = probe_read(curr->bin.path, event->exe.len, event->exe.buf);
if (curr->bin.path_length == 0)
curr->bin.path_length = event->exe.len;
__u64 revlen = event->exe.len;

if (event->exe.len > STRING_POSTFIX_MAX_LENGTH - 1)
revlen = STRING_POSTFIX_MAX_LENGTH - 1;
Comment on lines +389 to +392
Copy link
Member

Choose a reason for hiding this comment

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

nit: you could also write revlen into the struct so that you don't need to recompute it again, not sure it's better than this but you could downsize again here and use 16/16 bytes, so that you get path_len and end_len/rev_len?

// if end_r contains reversed path postfix
__u32 reversed;

probe_read(curr->bin.end, revlen, event->exe.end);
}
#else
// reuse p->args first string that contains the filename, this can't be
Expand Down
48 changes: 40 additions & 8 deletions bpf/process/types/basic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,10 +827,7 @@ filter_char_buf_prefix(struct selector_arg_filter *filter, char *arg_str, uint a
return !!pass;
}

// Define a mask for the maximum path length on Linux.
#define PATH_MASK (4096 - 1)

FUNC_INLINE void copy_reverse(__u8 *dest, uint len, __u8 *src, uint offset)
FUNC_INLINE void __copy_reverse(__u8 *dest, uint len, __u8 *src, uint offset, uint mask)
{
uint i;

Expand All @@ -849,12 +846,25 @@ FUNC_INLINE void copy_reverse(__u8 *dest, uint len, __u8 *src, uint offset)
// Alternative (prettier) fixes resulted in a confused verifier
// unfortunately.
for (i = 0; i < (STRING_POSTFIX_MAX_MATCH_LENGTH - 1); i++) {
dest[i & STRING_POSTFIX_MAX_MASK] = src[(len + offset - 1 - i) & PATH_MASK];
dest[i & STRING_POSTFIX_MAX_MASK] = src[(len + offset - 1 - i) & mask];
if (len + offset == (i + 1))
return;
}
}

// Define a mask for the maximum path length on Linux.
#define PATH_MASK (4096 - 1)

FUNC_INLINE void copy_reverse(__u8 *dest, uint len, __u8 *src, uint offset)
{
__copy_reverse(dest, len, src, offset, PATH_MASK);
}

FUNC_INLINE void file_copy_reverse(__u8 *dest, uint len, __u8 *src, uint offset)
anfedotoff marked this conversation as resolved.
Show resolved Hide resolved
{
__copy_reverse(dest, len, src, offset, STRING_POSTFIX_MAX_LENGTH - 1);
}

FUNC_LOCAL long
filter_char_buf_postfix(struct selector_arg_filter *filter, char *arg_str, uint arg_len)
{
Expand Down Expand Up @@ -1554,7 +1564,10 @@ FUNC_INLINE int match_binaries(__u32 selidx)
__u8 *found_key;
#ifdef __LARGE_BPF_PROG
struct string_prefix_lpm_trie prefix_key;
long ret;
struct string_postfix_lpm_trie *postfix_key;
__u64 postfix_len = STRING_POSTFIX_MAX_MATCH_LENGTH - 1;

int zero = 0;
#endif /* __LARGE_BPF_PROG */

struct match_binaries_sel_opts *selector_options;
Expand Down Expand Up @@ -1602,11 +1615,30 @@ FUNC_INLINE int match_binaries(__u32 selidx)
// prepare the key on the stack to perform lookup in the LPM_TRIE
memset(&prefix_key, 0, sizeof(prefix_key));
prefix_key.prefixlen = current->bin.path_length * 8; // prefixlen is in bits
ret = probe_read(prefix_key.data, current->bin.path_length & (STRING_PREFIX_MAX_LENGTH - 1), current->bin.path);
if (ret < 0)
if (probe_read(prefix_key.data, current->bin.path_length & (STRING_PREFIX_MAX_LENGTH - 1), current->bin.path) < 0)
return 0;
found_key = map_lookup_elem(path_map, &prefix_key);
break;
case op_filter_str_postfix:
case op_filter_str_notpostfix:
path_map = map_lookup_elem(&string_postfix_maps, &selector_options->map_id);
if (!path_map)
return 0;
if (current->bin.path_length < STRING_POSTFIX_MAX_MATCH_LENGTH)
postfix_len = current->bin.path_length;
postfix_key = (struct string_postfix_lpm_trie *)map_lookup_elem(&string_postfix_maps_heap, &zero);
if (!postfix_key)
return 0;
postfix_key->prefixlen = postfix_len * 8; // prefixlen is in bits
if (!current->bin.reversed) {
file_copy_reverse((__u8 *)current->bin.end_r, postfix_len, (__u8 *)current->bin.end, current->bin.path_length - postfix_len);
current->bin.reversed = true;
}
if (postfix_len < STRING_POSTFIX_MAX_MATCH_LENGTH)
if (probe_read(postfix_key->data, postfix_len, current->bin.end_r) < 0)
return 0;
found_key = map_lookup_elem(path_map, postfix_key);
break;
#endif /* __LARGE_BPF_PROG */
default:
// should not happen
Expand Down
14 changes: 11 additions & 3 deletions docs/content/en/docs/concepts/tracing-policy/selectors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,17 @@ calls and kernel functions that are coming from `cat` or `tail`.
- "/usr/bin/tail"
```

Currently, only the `In` operator type is supported and the `values` field has
to be a map of `strings`. The default behaviour is `followForks: true`, so all
the child processes are followed. The current limitation is 4 values.
The available operators for `matchBinaries` are:
- `In`
- `NotIn`
- `Prefix`
- `NotPrefix`
- `Postfix`
- `NotPostfix`

The `values` field has to be a map of `strings`. The default behaviour
is `followForks: true`, so all the child processes are followed.
The current limitation is 4 values.
Copy link
Member

Choose a reason for hiding this comment

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

I think the limitation here is outdated but it's out of the scope of this PR


**Further examples**

Expand Down
8 changes: 8 additions & 0 deletions install/kubernetes/tetragon/crds-yaml/cilium.io_tracingpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -1065,6 +1067,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -1705,6 +1709,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -2282,6 +2288,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down
8 changes: 8 additions & 0 deletions install/kubernetes/tetragon/crds-yaml/cilium.io_tracingpoliciesnamespaced.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -1065,6 +1067,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -1705,6 +1709,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -2282,6 +2288,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down
7 changes: 6 additions & 1 deletion pkg/api/processapi/processapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ const (
MSG_COMMON_FLAG_USER_STACKTRACE = 0x4

BINARY_PATH_MAX_LEN = 256

STRING_POSTFIX_MAX_LENGTH = 128
)

type MsgExec struct {
Expand Down Expand Up @@ -139,8 +141,11 @@ type MsgCapabilities struct {
}

type Binary struct {
PathLength int64
PathLength int32
Reversed uint32
Path [BINARY_PATH_MAX_LEN]byte
End [STRING_POSTFIX_MAX_LENGTH]byte
End_r [STRING_POSTFIX_MAX_LENGTH]byte
MBSet uint64
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -1065,6 +1067,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -1705,6 +1709,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -2282,6 +2288,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down
8 changes: 8 additions & 0 deletions pkg/k8s/apis/cilium.io/client/crds/v1alpha1/cilium.io_tracingpoliciesnamespaced.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -1065,6 +1067,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -1705,6 +1709,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down Expand Up @@ -2282,6 +2288,8 @@ spec:
- NotIn
- Prefix
- NotPrefix
- Postfix
- NotPostfix
type: string
values:
description: Value to compare the argument against.
Expand Down
2 changes: 1 addition & 1 deletion pkg/k8s/apis/cilium.io/v1alpha1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type KProbeArg struct {
}

type BinarySelector struct {
// +kubebuilder:validation:Enum=In;NotIn;Prefix;NotPrefix
// +kubebuilder:validation:Enum=In;NotIn;Prefix;NotPrefix;Postfix;NotPostfix
// Filter operation.
Operator string `json:"operator"`
// Value to compare the argument against.
Expand Down
2 changes: 1 addition & 1 deletion pkg/k8s/apis/cilium.io/v1alpha1/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ package v1alpha1
// Used to determine if CRD needs to be updated in cluster
//
// Developers: Bump patch for each change in the CRD schema.
const CustomResourceDefinitionSchemaVersion = "1.2.3"
const CustomResourceDefinitionSchemaVersion = "1.2.4"
Loading
Loading