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

Don't call setgroups unconditionally in mainrelay #1508

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

networkException
Copy link

This pull request moves the call to setgroups from the beginning of the drop_priviliges function to branch in which setuid is actually called. This still fulfills the intention of acbf7e1, initially introducting the call to setgroups:

Fix related to POS36-C and rpmlint error "missing-call-to-setgroups-before-setuid".

As per this intention is is not required to call setgroups otherwise, reducing the more exotic (as in not part of POSIX and considered priviliged by systemd) system calls coturn needs to make at startup.

This patch moves the call to `setgroups` from the beginning of the
`drop_priviliges` function to branch in which `setuid` is actually
called. This still fulfills the intention of
acbf7e1, initially introducting
the call to `setgroups`:

> Fix related to POS36-C and rpmlint error
> "missing-call-to-setgroups-before-setuid".

As per this intention is is not required to call `setgroups`
otherwise, reducing the more exotic (as in not part of POSIX and
considered priviliged by systemd) system calls coturn needs to make
at startup.
@eakraly
Copy link
Collaborator

eakraly commented May 30, 2024

Thank you @networkException for the contribution?
Is there any way we can test it?

@networkException
Copy link
Author

You can run

systemd-run -t -p "SystemCallFilter=~@privileged" turnserver

and see the process get killed without this patch (journalctl -t kernel -f for the error message)

I don't have a setup to test if rpmlint accepts this sadly

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

Successfully merging this pull request may close these issues.

2 participants