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

Only the first syscall group is being executed when there is multiple syscall groups #40

Open
criyle opened this issue Dec 6, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@criyle
Copy link

criyle commented Dec 6, 2024

For example, if the policy have multiple syscall groups like

	policy := &Policy{
		DefaultAction: ActionKillProcess,
		Syscalls: []SyscallGroup{
			{
				Names:  []string{"execve", "fork"},
				Action: ActionAllow,
			},
			{
				Names:  []string{"clone", "listen"},
				Action: ActionAllow,
			},
		},
	}

The expected behaviour is to execute the default action only when all syscall group not matches, but the current behaviour is to execute the default action if the first syscall group not matches. In this case, for syscall clone in the second group, it suppose to be allowed, but the result is kill.

This behaviour is able to reproduce by adding test case in filter_test.go L190 with

		{
			SeccompData{NR: 56 /* clone */, Arch: uint32(arch.X86_64.ID)},
			ActionAllow,
		},
--- FAIL: TestPolicyAssembleWhitelist (0.00s)
    filter_test.go:185: Expected allow, but got kill_process for test case 2 with seccomp_data=seccomp.SeccompData{NR:56, Arch:0xc000003e, InstructionPointer:0x0, Args:[6]uint64{0x0, 0x0, 0x0, 0x0, 0x0, 0x0}}
FAIL
FAIL    github.com/elastic/go-seccomp-bpf       0.179s
FAIL

I think the root cause may be the default action is assembled to every group: https://github.com/elastic/go-seccomp-bpf/blob/main/filter.go#L223

@andrewkroh andrewkroh added the bug Something isn't working label Dec 6, 2024
andrewkroh added a commit to andrewkroh/go-seccomp-bpf that referenced this issue Dec 6, 2024
Add failing test case that demonstrates the bug. The
expected behavior is to allow clone, but the default
action of kill process is used instead.

The log output is:

    filter_test.go:185: Expected allow, but got kill_process for test case 2 with
    seccomp_data=seccomp.SeccompData{NR:56, Arch:0xc000003e, InstructionPointer:0x0,
    Args:[6]uint64{0x0, 0x0, 0x0, 0x0, 0x0, 0x0}}

Relates: elastic#40
@andrewkroh
Copy link
Member

This appears to be a bug. It looks like since #28, if the any SyscallGroup condition does not match that it executes the default action instead of continuing to the next group. That code is here:

go-seccomp-bpf/filter.go

Lines 364 to 367 in 644ae6c

syscall.Assemble(&p, action)
}
p.Ret(defaultAction)

@andrewkroh
Copy link
Member

From your suggested test modification, I can see the bad logic in the instructions.

go test -run TestPolicyAssembleWhitelist -dump .
0: ld [4]
1: jneq #3221225534,9
2: ld [0]
3: jlt #1073741824,1
4: ret #327718
5: jeq #59,2
6: jeq #57,1
7: ret #2147483648 #!! syscall group action
8: ret #2147418112 #!! default_action (BUG HERE - never fallsthrough to next syscall group)
9: jeq #56,2
10: jeq #50,1
11: ret #2147483648 #!! syscall group action
12: ret #2147418112 #!! default_action

@criyle
Copy link
Author

criyle commented Dec 6, 2024

Thank you for helping but I think there might be some small mistake in the comment, and I think it should be like

9: jeq #56,2 # jump +2 to 12 to syscall group action
10: jeq #50,1
11: ret #2147483648 #!! default action: kill_process 0x80000000
12: ret #2147418112 #!! syscall group action: allow 0x7fff0000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants