[revised] Add -uid and -uid flags to drop permissions #817
Conversation
yggdrasil seems to prefer to run as a privileged user, but it also seems to work fine if permissions are dropped after the socket initialization is performed. This adds -uid and -gid flags so that an instance run with root perms can drop them once it's ready.
|
The creation of the socket file is a filesystem permissions issue. It is at best very unconventional to solve this filesystem permissions issue the way this PR suggests: starting out as superuser and trusting ygg to drop to the permissions it wants. |
|
This isn't just about the socket file. If the process was started as root, then there's no reason to keep running as root after the point where elevated permissions may have been needed. A long standing issue in go prevented this until recently. |
|
Adding my own review to remind me to test this on macOS and FreeBSD. |
| logger.Infoln("Setting gid to:", args.rungid) | ||
| if err := setgid(args.rungid); err != nil { | ||
| logger.Errorln("Failed to set gid:", err) | ||
| return |
There was a problem hiding this comment.
instead of simply returning, you likely want to abort() here.
Not being able to change gid/uid (or egid/euid) is not a failure you should ignore and then simply continue running.
neilalexander
force-pushed
the
develop
branch
from
5a0470e to
8ce7c86
Compare
|
Closing in favour of #927. |
different from #817 in that it can resolve user names, automatically use user's primary gid & allows specifying gid in the same argument, with `:` eg `username:groupname`. feel free to criticize & suggest different argument name & description because i didn't put much of thought to that. --------- Co-authored-by: Neil <[email protected]> Co-authored-by: VNAT <[email protected]> Co-authored-by: Neil Alexander <[email protected]>
3 participants
This is built on #804 (thanks @tsuraan)
It's essentially identical to that PR, except it wraps the syscalls in a separate function, so we can return an error on platforms where this isn't supported (which seems to at least include windows).
I'm not sure exactly what build directives make the most sense here... at the moment, it only supports
linux,darwin,freebsd, andopenbsd, but maybe!windowsis more correct.We should probably think a bit about what behavior we want. If we always want to drop permissions, then we could maybe skip the
-uidand-gidargs, and hard code a specific user to switch to (nobodyor whatever makes the most sense).