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

Overzealous SECCOMP filters killing bots #32

Closed
rendello opened this issue Oct 1, 2024 · 2 comments
Closed

Overzealous SECCOMP filters killing bots #32

rendello opened this issue Oct 1, 2024 · 2 comments

Comments

@rendello
Copy link

rendello commented Oct 1, 2024

On Debian 12 arm64, the lobby bots' Python processes are killed without the following in their SystemCallFilter lists:

  • epoll_pwait
  • faccessat
  • fchown
  • readlinkat
  • rt_sigprocmask
  • unlinkat

Tested with Dunedan/lobby-bots@79f52f5.

I'd planned to just make a PR, but the syscalls required seem to change each time and I'd like to test further. For example, the moderation bot sometimes requires mkdirat, I assume this related to the database but need to confirm with strace.

I wish Linux had a higher-level API like OpenBSD's pledge and unveil. Unfortunately, SECCOMP forces whack-a-mole with syscalls, which leads me to this question:

The current layout of syscalls in the filter is like this, with each line being less than 100 chars and sorted alphabetically. Given that syscalls would have to be added for this PR and potentially in the future, might it be preferable to list them on their own lines to preserve git blames?

SystemCallFilter=access arch_prctl brk close connect epoll_create1 epoll_ctl epoll_pwait epoll_wait
SystemCallFilter=execve faccessat fchmod fcntl fdatasync futex getcwd getdents64 geteuid
SystemCallFilter=getpeername getpid getrandom getsockname getsockopt gettid ioctl lseek mmap
SystemCallFilter=mprotect mremap munmap newfstatat openat pread64 prlimit64 pwrite64 read readlink
SystemCallFilter=readlinkat recvfrom rseq rt_sigaction rt_sigprocmask sendto set_robust_list
SystemCallFilter=setsockopt set_tid_address socket socketpair sysinfo uname unlink write
SystemCallFilter= \
    access \
    arch_prctl \
    brk \
    close \
    connect \
    epoll_create1 \
    epoll_ctl \
    epoll_pwait \
    epoll_wait \
   ... etc. ...

I would defer to you on the formatting, @Dunedan.

And thanks for all the work on the lobby, I know I just came out of nowhere 😅 I'm __najimakimoda in game.

@Dunedan
Copy link
Collaborator

Dunedan commented Oct 1, 2024

Yes, the functionality introduced with 0ad/lobby-bots#45 does need additional syscalls. I have a branch with the necessary changes prepared already and plan to create a PR from it once I merged 0ad/lobby-bots#45. I plan to do it after the merge, as the branch refers to the changes in the lobby-bots repository and not only add the new syscalls, but enables the profanity monitoring as well.

So you don't have to create a PR for that. Good one noticing and being able to debug that though. 👍

Given that syscalls would have to be added for this PR and potentially in the future, might it be preferable to list them on their own lines to preserve git blames?

I'm not sure if the syntax you propose would work with systemd, however as the necessary syscalls shouldn't change frequently, I believe the current notation is fine. There is a trade-off between the readability of the systemd unit files and how granular being able to use git blame and I believe the current notation strikes a good balance between them.

And thanks for all the work on the lobby, I know I just came out of nowhere 😅 I'm __najimakimoda in game.

If you have questions or would like to discuss ideas with me, I'm available on IRC in #0ad-dev. 🙂

@rendello
Copy link
Author

rendello commented Oct 1, 2024

Alright, thanks

@rendello rendello closed this as completed Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants