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

tetragon: Add support to use security_ functions in killer #2002

Merged
merged 8 commits into from
Jan 29, 2024
Merged
2 changes: 1 addition & 1 deletion bpf/process/types/basic.h
Original file line number Diff line number Diff line change
Expand Up @@ -2084,7 +2084,7 @@ struct {
__uint(value_size, sizeof(__u64) * PERF_MAX_STACK_DEPTH);
} stack_trace_map SEC(".maps");

#ifdef GENERIC_TRACEPOINT
#if defined GENERIC_TRACEPOINT || defined GENERIC_KPROBE
static inline __attribute__((always_inline)) void
do_action_notify_killer(int error, int signal)
{
Expand Down
3 changes: 2 additions & 1 deletion contrib/tester-progs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ PROGS = sigkill-tester \
threads-exit \
killer-tester \
drop-privileges \
getcpu
getcpu \
direct-write-tester

# For now killer-tester is compiled to 32-bit only on x86_64 as we want
# to test 32-bit binaries and system calls compatibility layer.
Expand Down
43 changes: 43 additions & 0 deletions contrib/tester-progs/direct-write-tester.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
// Copyright Authors of Tetragon

#include <fcntl.h>
#include <sys/syscall.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <stdio.h>

#define BLOCKSIZE 4096
#define O_DIRECT 00040000

int main(int argc, char **argv)
{
char *avd = "NEW_MESSAGE\n";
void *buffer;
int fd;

if (argc != 2)
exit(-1);

fd = open(argv[1], O_WRONLY | O_DIRECT | O_CREAT, S_IRWXU | S_IRWXG);
if (fd == -1) {
perror("open");
exit(-1);
}

posix_memalign(&buffer, BLOCKSIZE, BLOCKSIZE);
memset(buffer, 0, BLOCKSIZE);
memcpy(buffer, avd, strlen(avd) + 1);

ssize_t ret = pwrite(fd, buffer, BLOCKSIZE, 0);
if (ret == -1) {
perror("write");
exit(-1);
}

close(fd);
free(buffer);
return 0;
}
4 changes: 2 additions & 2 deletions docs/content/en/docs/concepts/tracing-policy/selectors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ the `killer` program and attach it to specified syscalls.
```yaml
spec:
killers:
- syscalls:
- calls:
- "list:dups"
```

Expand Down Expand Up @@ -1047,7 +1047,7 @@ spec:
- "sys_dup"
- "sys_dup2"
killers:
- syscalls:
- calls:
- "list:dups"
tracepoints:
- subsystem: "raw_syscalls"
Expand Down
2 changes: 1 addition & 1 deletion examples/tracingpolicy/killer.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ spec:
- "sys_dup2"
- "__ia32_sys_dup"
killers:
- syscalls:
- calls:
- "list:dups"
tracepoints:
- subsystem: "raw_syscalls"
Expand Down
9 changes: 9 additions & 0 deletions pkg/arch/arch.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,12 @@ func CutSyscallPrefix(symbol string) (string, bool) {
}
return symbol, false
}

func HasSyscallPrefix(symbol string) bool {
for _, prefix := range supportedArchPrefix {
if strings.HasPrefix(symbol, prefix) {
return true
}
}
return false
}
5 changes: 4 additions & 1 deletion pkg/ftrace/ftrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ func ReadAvailFuncs(pattern string) ([]string, error) {
var r *regexp.Regexp

if pattern != "" {
r, _ = regexp.Compile(pattern)
r, err = regexp.Compile(pattern)
if err != nil {
return nil, err
}
}

final := []string{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ spec:
description: A killer spec.
items:
properties:
syscalls:
description: syscalls where killer is executed in
calls:
description: Calls where killer is executed in
items:
type: string
type: array
required:
- syscalls
- calls
type: object
type: array
kprobes:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ spec:
description: A killer spec.
items:
properties:
syscalls:
description: syscalls where killer is executed in
calls:
description: Calls where killer is executed in
items:
type: string
type: array
required:
- syscalls
- calls
type: object
type: array
kprobes:
Expand Down
4 changes: 2 additions & 2 deletions pkg/k8s/apis/cilium.io/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,6 @@ type PodInfoList struct {
}

type KillerSpec struct {
// syscalls where killer is executed in
Syscalls []string `json:"syscalls"`
// Calls where killer is executed in
Calls []string `json:"calls"`
}
4 changes: 2 additions & 2 deletions pkg/k8s/apis/cilium.io/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions pkg/sensors/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,15 @@ func loadInstance(bpfDir, mapDir string, load *program.Program, version, verbose
version = kernels.FixKernelVersion(version)
probe, ok := registeredProbeLoad[load.Type]
if ok {
logger.GetLogger().WithField("Program", load.Name).WithField("Type", load.Type).Info("Loading registered BPF probe")
logger.GetLogger().WithField("Program", load.Name).
WithField("Type", load.Type).
WithField("Attach", load.Attach).
Info("Loading registered BPF probe")
} else {
logger.GetLogger().WithField("Program", load.Name).WithField("Type", load.Type).Info("Loading BPF program")
logger.GetLogger().WithField("Program", load.Name).
WithField("Type", load.Type).
WithField("Attach", load.Attach).
Info("Loading BPF program")
}

switch load.Type {
Expand Down
6 changes: 6 additions & 0 deletions pkg/sensors/tracing/generickprobe.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,9 @@ func createMultiKprobeSensor(sensorPath string, multiIDs []idtable.EntryID) ([]*
maps = append(maps, socktrack)
}

killerDataMap := program.MapBuilderPin(killerDataMapName, killerDataMapName, load)
maps = append(maps, killerDataMap)

filterMap.SetMaxEntries(len(multiIDs))
configMap.SetMaxEntries(len(multiIDs))

Expand Down Expand Up @@ -858,6 +861,9 @@ func createKprobeSensorFromEntry(kprobeEntry *genericKprobe, sensorPath string,
maps = append(maps, socktrack)
}

killerDataMap := program.MapBuilderPin(killerDataMapName, killerDataMapName, load)
maps = append(maps, killerDataMap)

if kprobeEntry.loadArgs.retprobe {
pinRetProg := sensors.PathJoin(pinPath, fmt.Sprintf("%s_ret_prog", kprobeEntry.funcName))
loadret := program.Builder(
Expand Down
42 changes: 30 additions & 12 deletions pkg/sensors/tracing/killer.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (kh *killerHandler) LoadProbe(args sensors.LoadProbeArgs) error {
}

// select proper override method based on configuration and spec options
func selectOverrideMethod(specOpts *specOptions) (OverrideMethod, error) {
func selectOverrideMethod(specOpts *specOptions, hasSyscall bool) (OverrideMethod, error) {
overrideMethod := specOpts.OverrideMethod
switch overrideMethod {
case OverrideMethodDefault:
Expand All @@ -125,7 +125,7 @@ func selectOverrideMethod(specOpts *specOptions) (OverrideMethod, error) {
return OverrideMethodInvalid, fmt.Errorf("option override return set, but it is not supported")
}
case OverrideMethodFmodRet:
if !bpf.HasModifyReturnSyscall() {
if !bpf.HasModifyReturn() || (hasSyscall && !bpf.HasModifyReturnSyscall()) {
return OverrideMethodInvalid, fmt.Errorf("option fmod_ret set, but it is not supported")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the hasNormal check is needed.

AFIACT, if HadModifyReturnSyscall() is true , HasModifyReturn() is (always) true.
So I think the following check:

if !bpf.HasModifyReturn() || (hasSyscall && !bpf.HasModifyReturnSyscall()) {
     return err
}

catches all the cases we care about, without the extra argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, I guess I wanted it to be more explicit.. I'll change that

}
}
Expand Down Expand Up @@ -158,9 +158,11 @@ func (kh *killerHandler) createKillerSensor(

killer := killers[0]

var hasSyscall bool

// get all the syscalls
for idx := range killer.Syscalls {
sym := killer.Syscalls[idx]
for idx := range killer.Calls {
sym := killer.Calls[idx]
if strings.HasPrefix(sym, "list:") {
listName := sym[len("list:"):]

Expand All @@ -169,18 +171,34 @@ func (kh *killerHandler) createKillerSensor(
return nil, fmt.Errorf("Error list '%s' not found", listName)
}

if !isSyscallListType(list.Type) {
return nil, fmt.Errorf("Error list '%s' is not syscall type", listName)
}
kh.syscallsSyms = append(kh.syscallsSyms, list.Values...)
continue
}

pfxSym, err := arch.AddSyscallPrefix(sym)
if err != nil {
return nil, err
kh.syscallsSyms = append(kh.syscallsSyms, sym)
}

var err error

// fix syscalls
for idx, sym := range kh.syscallsSyms {
isPrefix := arch.HasSyscallPrefix(sym)
isSyscall := strings.HasPrefix(sym, "sys_")
isSecurity := strings.HasPrefix(sym, "security_")

if !isSyscall && !isSecurity && !isPrefix {
return nil, fmt.Errorf("killer sensor requires either syscall or security_ functions")
}

if isSyscall {
sym, err = arch.AddSyscallPrefix(sym)
if err != nil {
return nil, err
}
kh.syscallsSyms[idx] = sym
}
kh.syscallsSyms = append(kh.syscallsSyms, pfxSym)

hasSyscall = hasSyscall || isSyscall || isPrefix
}

// register killer sensor
Expand All @@ -197,7 +215,7 @@ func (kh *killerHandler) createKillerSensor(
}

// select proper override method based on configuration and spec options
overrideMethod, err := selectOverrideMethod(specOpts)
overrideMethod, err := selectOverrideMethod(specOpts, hasSyscall)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sensors/tracing/killer_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (ksb *KillerSpecBuilder) Build() (*v1alpha1.TracingPolicy, error) {
Validated: false,
})
killers = append(killers, v1alpha1.KillerSpec{
Syscalls: []string{listName},
Calls: []string{listName},
})
}

Expand Down
Loading
Loading