Add -uid and -uid flags to drop permissions #804
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.
|
So, I have no idea whether this is desired, or if there's a better way to do it, or what, but it doesn't look like ygg needs to have so many permissions once it's up and running. Might be worth looking into, or maybe not. |
| if syscall.Getuid() == 0 { | ||
| if args.rungid > 0 { | ||
| fmt.Println("Dropping gid to ", args.rungid) | ||
| syscall.Setgid(args.rungid) |
There was a problem hiding this comment.
You know this makes Yggdrasil no longer portable, right?
There was a problem hiding this comment.
Yeah, I see the circleci failure under the "other" OS (I assume that's windows?). I'm a bit surprised there's an OS that doesn't have UIDs, but just a little bit. Anyhow, I guess the fix for that would be to move the args struct and arg parser into a arg_windows.go and a arg_posix.go, and only have the gid/uid options in the args_posix side. Similarly define a drop_perm_unix.go that does the above, and a drop_perm_windows.go that's just a no-op. I can definitely do that, but before I do, is there interest in privilege dropping in yggdrasil? I'm happy to do the grunt work, but if it's something that's not seen as necessary anyhow, then I can just drop this.
There was a problem hiding this comment.
More importantly, syscall.Setgid (or setting UIDs) probably won't work in Go. Or to be exact, it will set the ID for whatever random thread the goroutine happened to be running on at the time, but the rest of the threads will be unaffected.
The language recently added https://golang.org/pkg/syscall/#AllThreadsSyscall which is meant to address issues like this, but it has some caveats associated with it.
There was a problem hiding this comment.
If I understand correctly, the AllThreadsSyscall function is only available on linux, so this feature wouldn't be cross-platform.
There was a problem hiding this comment.
POSIX setuid is required to set the uid for the entire process (ref and a stackoverflow). It definitely appears to be working under FreeBSD and Linux, but I haven't tried it under OSX. I'd expect it to work the same there, as OSX's POSIX layer appears to still be based on FreeBSD.
Or am I misunderstanding the problem? It looks like Linux is actually the weird one, because it does (in the kernel) implement per-thread uids, and sane programs depend on glibc or musl to provide the POSIX semantics that people tend to expect (more). If Go's "libc" really is just sending the raw syscalls to the kernel, then I can see how the AllThreadsSyscall would be needed to be run explicitly on LInux systems. Unfortunately I'm not actually a Go programmer, and I have no idea what AllThreadsSyscall's function signature means. So, I guess I'll drop this, unless somebody really wants to see it completed.
There was a problem hiding this comment.
Looking into it more, it appears that Setgid and Setuid were rewritten to use AllThreadsSyscall on linux (as long as cgo isn't being used), so this should be doing the right thing.
Some refactoring/cleanup is needed to avoid breaking things on platforms where syscall.Setuid/syscall.Setgid are undefined (if any... windows maybe?). That shouldn't be too difficult, I can probably take a look at in the next few days.
There was a problem hiding this comment.
If somebody else could figure out how to do the conditional compilation thing, that would be cool. I tried to split out main.go's argv into its own main_posix.go and main_other.go with // +build directives for per-OS compilation, and I just couldn't get the thing to compile. I don't know Go though, and its package system seems to be pretty unique, so I'm sure I was just doing something really basic wrong.
|
Related #802 |
|
Closing as the approach in #927 is a bit more portable. |
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.