golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.98k stars 17.53k forks source link

syscall: Setuid/Setgid doesn't apply to all threads on Linux #1435

Closed gopherbot closed 3 years ago

gopherbot commented 13 years ago

by ziutek@Lnet.pl:

What steps will reproduce the problem?

Compile attached test code. Run it as root like this:

# GOMAXPROCS=4 ./test 65534 65534

and note output:

gorutine 1: uid=0 euid=0 gid=0 egid=0
gorutine 2: uid=0 euid=0 gid=0 egid=0
gorutine 3: uid=0 euid=0 gid=0 egid=0
gorutine 4: uid=0 euid=0 gid=0 egid=0
gorutine 5: uid=0 euid=0 gid=0 egid=0
gorutine 6: uid=0 euid=0 gid=0 egid=0
gorutine 7: uid=0 euid=0 gid=0 egid=0
gorutine 8: uid=0 euid=0 gid=0 egid=0
gorutine 9: uid=0 euid=0 gid=0 egid=0
gorutine 0: uid=65534 euid=65534 gid=65534 egid=65534
gorutine 1: uid=0 euid=0 gid=0 egid=0
gorutine 2: uid=0 euid=0 gid=0 egid=0
gorutine 3: uid=0 euid=0 gid=0 egid=0
gorutine 4: uid=0 euid=0 gid=0 egid=0
gorutine 5: uid=0 euid=0 gid=0 egid=0
gorutine 6: uid=0 euid=0 gid=0 egid=0
gorutine 7: uid=0 euid=0 gid=0 egid=0
gorutine 8: uid=0 euid=0 gid=0 egid=0
gorutine 9: uid=0 euid=0 gid=0 egid=0
gorutine 0: uid=65534 euid=65534 gid=65534 egid=65534

Use ps -efL during test execution and note output:

UID        PID  PPID   LWP  C NLWP STIME TTY          TIME CMD
nobody   26088 25928 26088  0   10 11:56 pts/1    00:00:00 ./test 65534 65534
root     26088 25928 26089  0   10 11:56 pts/1    00:00:00 ./test 65534 65534
root     26088 25928 26090  0   10 11:56 pts/1    00:00:00 ./test 65534 65534
root     26088 25928 26091  0   10 11:56 pts/1    00:00:00 ./test 65534 65534
root     26088 25928 26092  0   10 11:56 pts/1    00:00:00 ./test 65534 65534
root     26088 25928 26093  0   10 11:56 pts/1    00:00:00 ./test 65534 65534
root     26088 25928 26094  0   10 11:56 pts/1    00:00:00 ./test 65534 65534
root     26088 25928 26095  0   10 11:56 pts/1    00:00:00 ./test 65534 65534
root     26088 25928 26096  0   10 11:56 pts/1    00:00:00 ./test 65534 65534
root     26088 25928 26097  0   10 11:56 pts/1    00:00:00 ./test 65534 65534

What is the expected output?

All threads must have the same UID/GID: (65534, nobody user in my system).

Which compiler are you using (5g, 6g, 8g, gccgo)?

I tested this with 6g and 8g.

Which operating system are you using?

Linux (Debian 6.0 SID on i386, Ubuntu 10.10 on amd64)

Which revision are you using?  (hg identify)

d8ba80011a98 release/release.2011-01-20

Please provide any additional information below.

http://groups.google.com/group/golang-nuts/browse_thread/thread/59597aafdd84a0e

Attachments:

  1. test.go (1067 bytes)
alberts commented 10 years ago

Comment 52:

@iant I'm assuming this approach could fix Unshare too?
https://golang.org/issue/1954
ianlancetaylor commented 10 years ago

Comment 53:

Calling the C library version of unshare wouldn't make any difference.  The unshare
function in the GNU/Linux libc only applies to the current thread.  Since goroutines
aren't attached to a specific thread, there is no meaningful way to call unshare from a
goroutine.  We would have to introduce a mechanism to call unshare from every thread,
which is more or less what we are trying to avoid, and which is in any case impossible
if cgo code has created new threads.
I think the only way to use unshare from a Go program is to use runtime.LockOSThread. 
Or to arrange to call it just before a fork.  I don't think there is any reasonable way
to unshare across the whole process, and the same is true of C code.
rsc commented 10 years ago

Comment 54:

How do you force the use of cgo for any program that uses syscall.Setuid? The only way I
know how to do this would be to force the use of cgo for any program that uses syscall,
but that's untenable.
What we do in the runtime is check to see if cgo is already being used for some other
reason, and if so we use it for things like pthread_create but otherwise fall back to
our own implementation. I think we'd have to do something similar to play nicely with
cgo here.
ianlancetaylor commented 10 years ago

Comment 55:

Argh, you're right.
yonderblue commented 9 years ago

Does setreuid apply to all threads? Would this be a work around when trying to use it to temporarily go to root?

ianlancetaylor commented 9 years ago

All the related system calls apply only to a single thread. The full set is setuid setgid setgroups setreuid setregid setresuid setresgid (and the xxx32 variants were applicable).

labkode commented 9 years ago

Any updates on this ?

Thanks

bradfitz commented 9 years ago

No.

mbenkmann commented 8 years ago

I would like to add that the suggested workaround of using capabilities does not work. In order to be useful for security, capabilities need to be dropped by the process after performing the privileged function. However just as you cannot setuid() other threads than your own, you cannot drop capabilities of other threads than your own. So the other threads created by the Go runtime keep the elevated privileges granted by the capabilities.

This is a big problem. E.g. the capability CAP_SYS_CHROOT is worthless if you cannot drop it, since that capability can be used to break out of chroots.

So even with the best effort using SystemD and file capabilities it is currently not possible to create a Go program that performs some privileged task and then drops ALL of its privileges. This is embarassing.

DemiMarie commented 8 years ago

A better solution is for Go to provide a way to serve on an already-open socket FD. A launcher (written in C) could then open the socket, drop privileges, and exec() the Go program.

minux commented 8 years ago

Go could do that already.

Use https://golang.org/pkg/os/#NewFile and https://golang.org/pkg/net/#FileListener.

bgilmore commented 8 years ago

This could use a lot of tidying, but as a proof of concept: http://play.golang.org/p/dXBizm4xl3

When run as root, the program will open a privileged socket and re-exec itself as an unprivileged user passing down the opened FD.

% go build drop.go
% sudo ./drop
Dropping privileges...
Spawned process 30278, exiting
% curl 127.0.0.1
I am process 30278 running as 65534/65534
mholt commented 8 years ago

@bgilmore Thank you, that's just what I was about to start coding up. Really glad to see it works! (Can't believe I didn't think of this earlier...) Although it would be nice to do it in-process, this effectively solves the issue for me.

DemiMarie commented 8 years ago

This trick should be documented as the way to drop priviliges in Go. On Jan 21, 2016 11:46 PM, "Matt Holt" notifications@github.com wrote:

@bgilmore https://github.com/bgilmore Thank you, that's just what I was about to start coding up. Really glad to see it works! (Can't believe I didn't think of this earlier...) Although it would be nice to do it in-process, this effectively solves the issue for me.

— Reply to this email directly or view it on GitHub https://github.com/golang/go/issues/1435#issuecomment-173805699.

mbenkmann commented 8 years ago

Unfortunately, this does not work if you chroot, which is another one of the most important security measures together with dropping privileges. E.g. if you implement a service like Dropbox or Google Drive where naturally users have the ability to upload arbitrary files, it's a very good idea to chroot into a filesystem mounted no-exec and together with setuid this will protect you against so many common programming errors.

j7b commented 8 years ago

I'd suggest the GOOS=linux behavior for Setuid/Setgid is correct for all operating systems and should be extended to Chroot for all OS's. I don't think any user code should be able to change the process state of the runtime in such a way other user code might not be able to do something it was able to do before the process state of the runtime was changed. For example any package that relies on any kind of file access needs to assume the state of the runtime will be reasonably consistent and shouldn't have to assume something can change the effective root of the runtime at any time without notice. It should be pretty reasonable to assume a package can create a temporary file and write to it at any time without worrying some other user code could at any time change uid/gid and make it unwritable.

Using setuid/gid to "drop privileges" in a Go program would tend to give a false sense of security anyway, the obvious place to do it would be a main package's init() and that's the last init() to run if there's multiple, and would always lead to the case above where things were possible for an imported package's init() that aren't after main's init(). Setuid, Setgid and Chroot just introduce potential opaque, hard to debug chaos and provide little or no value.

HerrSpace commented 8 years ago

Setuid, Setgid and Chroot just introduce potential opaque, hard to debug chaos and provide little or no value.

I beg to differ about the "little or no value" part. I'd like to see something like openbsds pledge eventually, though I realize that's probably not something that will happen anytime soon.

tejasmanohar commented 7 years ago

I think Go shouldn't schedule goroutines from a locked thread on another thread. goroutines are often used for control flow, and introducing a goroutine wouldn't be a "breaking change" in many cases in code you depend on... so it's hard to use Go when you really need a thread to be locked (e.g. when managing network namespaces).

ivan4th commented 7 years ago

See #20676 and https://golang.org/cl/46033

tsuna commented 7 years ago

Not sure whether #20676 will be of much help here since the problem is that on Linux setuid / setgid only affect the current thread instead of the entire process. Usually the goal of these calls is to drop/change privileges for the entire process, not just temporarily affect the privileges of the current thread.

riking commented 6 years ago

One method of fixing this correctly would be a mirror to runtime.systemstack; e.g. runtime.everyM.

// Call Setuid for every OS thread in the process.
func Setuid(uid int) error {
    errCh := make(chan syscall.Errno)
    runtime.everyM(func() {
        _, _, err := syscall.Syscall(syscall.SYS_SETUID, uid, 0, 0)
        if err != nil {
            errCh <- err
        }
    })

    select {
    case err := <-errCh:
        return err
    default:
        return nil
    }
}

// and a Windows example...

// Call SetThreadToken for every OS thread of the process.
func SwitchToken(tokenHandle syscall.Handle) error {
    errCh := make(chan syscall.Errno)
    runtime.everyM(func() {
        r1, _, e1 := syscall.Syscall(procSetThreadToken.Addr(), 0, uintptr(tokenHandle), 0)
        if r1 == 0 {
            if e1 != 0 {
                errCh <- errnoErr(e1)
            } else {
                errCh <- syscall.EINVAL
            }
        }
    })
    select {
    case err := <-errCh:
        return err
    default:
        return nil
    }
}

The semantics of what happens when a machine thread is already in a syscall are currently unspecified, but sending a reserved signal like glibc does as a trigger to check for everyM tasks seems reasonable. Syscalls are already restarted on EINTR.

hgfischer commented 6 years ago

Will Setuid/Setgid someday be allowed again on Linux? I have a use-case for it and I don't even bother that it just works in the main thread.

bradfitz commented 6 years ago

@hgfischer, what do you mean by "again"? I don't think this ever worked in Go.

In any case, bumping to Go 1.11 because this didn't happen for Go 1.10.

ivan4th commented 6 years ago

@bradfitz to be more precise, it used to be enabled but didn't really work. It was then disabled in 343b4ba8c1ad8a29b6dd19cb101273b57a26c9b0

ianlancetaylor commented 6 years ago

@hgfischer If you just want to change the ID of one thread (but why?) you can use runtime.LockOSThread and syscall.Syscall(syscall.SYS_SETUID, uid, 0, 0).

hgfischer commented 6 years ago

@bradfitz Thanks. As @ivan4th mentioned, it used to do something, some time ago, but then it got disabled because of this issue.

@ianlancetaylor Thanks for the tips. I tried just syscall.Syscall() before but it was not working for some reason. Since you mentioned, I tried a bit more and voilá!

The reason is to have a binary with setuid bit, that can be called by an unpriviledged user. I don't need goroutines for this, and the tool will be short lived. My test is here. Could I still face the problems reported in this issue, in this use case?

ianlancetaylor commented 6 years ago

@hgfischer Yes. Go programs are always multi-threaded, goroutines can migrate between threads at any function call, and the old syscall.Setuid only changed one thread in the program.

nnnn20430 commented 6 years ago

Couldn't this be solved by adding setuid/setguid parameters to syscall.SysProcAttr, and then have it call setuid/setgid in a thread safe way right before exec() when starting a new process? So that it can at least set uid of a new process, and then you can just re-execute the program and have the original instance exit, successfully achieving privilege drop/escalation.

ianlancetaylor commented 6 years ago

@nnnn20430 There is no particular problem with starting another program using a new UID/GID. We already support that via SysProcAttr.Credential.

The problem is with programs that want to change their effective uid/gid while executing. This is a more or less reasonable thing for a Unix program to do, but it doesn't currently work for Go programs on GNU/Linux. That is what this issue is about.

nnnn20430 commented 6 years ago

@ianlancetaylor ah oops, i just looked at surface level entries in the type didn't see setuid and assumed it to not be there, i was just thinking "can't this be solved like this?", and just skimmed over the type to confirm if it was there or not before commenting, sorry that was dumb....

PS: basically it was late and i wanted to comment before i forget and go to sleep comment

PPS: again it was late... now i see this was even said here before... https://github.com/golang/go/issues/1435#issuecomment-66054177

ianlancetaylor commented 6 years ago

I want to note that I think that any fix for this issue has to be quite different in pure Go programs and in programs that use cgo. In cgo-using programs I think we need to somehow use the glibc signal handler for SIGSETXID (SIGRTMIN + 1) and use the glibc call for setuid, setgid, setresgid, setgroups, setegid, setregid, setresuid, seteuid, setreuid, etc.

In pure Go programs we might be able to get away with queuing up a list of changes for each thread, and having the thread apply them every time it acquires a lock. My thinking here is that it's OK if the thread only reflects the UID change after there is some happens-before relationship between the UID call and the thread execution, and that should be mediated by a lock.

ironsteel commented 5 years ago

So is it safe to call glibc's version of setuid (which implies using CGO) just to setup the UID for all OS threads within the process? I've tested that and it seems to work but don't know if it will have any impact on the correctness of goroutine scheduling?

ianlancetaylor commented 5 years ago

@ironsteel I can't think of a reason that that wouldn't work, but I also wouldn't be surprised if there were cases where it failed. One particular case to consider is if a new thread is created during the setuid call.

larytet commented 5 years ago

The page is the top result in the google search. Thus I put an arguably verbose example of fork/exec here.

If do not like syscall.Syscall(syscall.SYS_SETUID, uintptr(uid), 0, 0) you can do this:

// Read environment variable USER, fork the process
func SwitchUser(daemon bool) (*user.User, error) {
    userCurrent, err := user.Current()
    if err != nil {
        return nil, fmt.Errorf("Failed to get current user %v", err)
    }

    userRequired := userCurrent
    if v := os.Getenv("USER"); v != "" {        // Rely on the environment
        userRequired, err = user.Lookup(v)  // and make it a truly a single liner in the calling code
        if err != nil {
            return userCurrent, fmt.Errorf("Failed to lookup '%s' %v", v, err)
        }
    }

    if userRequired.Uid == userCurrent.Uid {
        return userCurrent, nil
    }

    if uid := os.Getuid(); uid != 0 { // Straight from the exec unitest 
        return userCurrent, fmt.Errorf("I need root credentials, got %v instead", uid)
    }

    executable, err := filepath.Abs(filepath.Dir(os.Args[0]))  // Who am I? What is me?
    if err != nil {
        return userCurrent, fmt.Errorf("Failed to get path args[0]=%s %v", os.Args[0], err)
    }
    executable = path.Join(executable, path.Base(os.Args[0]))

    cmd := exec.Command(executable)
    gid, _ := strconv.Atoi(userRequired.Gid)    // this is the magic part 
    uid, _ := strconv.Atoi(userRequired.Uid)    // run the same executable 
    cmd.SysProcAttr = &syscall.SysProcAttr{  // with a different user
        Credential: &syscall.Credential{Uid: uint32(uid), Gid: uint32(gid)},
    }
    cmd.Stdout = os.Stdout  // you probably want to see the child's logs 
    cmd.Stderr = os.Stderr   // in the parent's stdout
    cmdRun := cmd.Run
    if daemon {
        cmdRun = cmd.Start
    }
    if err := cmdRun(); err != nil {
                // Or exec failed or the child failed 
        return userCurrent, fmt.Errorf("Failed to run %s as user %s %v", executable, userRequired.Name, err)
    } else {
                // Or daemon is true, or the child did Exit(0)
        return userRequired, nil
    }
}
gopherbot commented 4 years ago

Change https://golang.org/cl/210639 mentions this issue: syscall: Support POSIX semantics for Linux syscalls.

forsyth commented 4 years ago
> // To listen on port 80 we need root privileges
ls, err := net.Listen("tcp", "127.0.0.1:80")
if err != nil {
    log.Exitln("Can't listen:", err)
}

You could instead do what I do, specifically to avoid having a Go program run even briefly as root, but still able to listen on 80 and 443: use CAP_NET_BIND_SERVICE=+eip. I use a command "bless", attached, to set the attribute each time the file is updated from my build server. The executable name is fixed, so bless itself can be setuid, but you could add an argument instead. bless.c.txt

The < 1024 restriction is, of course, long obsolete. It wasn't even all that sensible when BSD had it for their timesharing systems "to make them more secure".

jedisct1 commented 4 years ago

@forsyth Unfortunately, this is a linux-only thing.

forsyth commented 4 years ago

I hadn't noticed this item was so old and undoubtedly the original user's need has long since gone away. Sadly, the "privileged port" stuff, let alone root itself hasn't gone away in the 8 years since.

ibukanov commented 4 years ago

An alternative to CAP_NET_BIND_SERVICE=+eip is to give CAP_NET_BIND_SERVICE as ambient capability without marking the exe as such. This way only invocations via the service manager will be allowed to bind to the privileged port, not an arbitrary invocation. Systemd allows that with one line in the service file via AmbientCapabilities=. Or one can use setpriv utility.

tv42 commented 4 years ago

If you're using systemd, you can just let it pass an open listening fd in as it executes your app. Also nicely limits what ports your app can use, with complete admin control.

Maybe these low port workarounds should be marked as off-topic. They're not helping solve the actual problem; try reimplementing samba or opensshd in Go and you'll hit this limit pretty quickly.

AndrewGMorgan commented 4 years ago

New to this issue tracker. Happy to be assigned this issue. I have a pending change to fix it here:

https://go-review.googlesource.com/c/go/+/210639

AndrewGMorgan commented 4 years ago

On the subject of capabilities and Go... I'd like to point out that the latest libcap-2.29 sources include a full port of libcap to Go ( see the official sources for libcap and release notes here: https://sites.google.com/site/fullycapable/ )

Using cgo and a C library libpsx for now, capability raising and lowering of all sets including the ambient one are fully supported in the Go package.

The native (CGO_ENABLED=0) build of the Go package requires the above https://go-review.googlesource.com/c/go/+/210639 patch to run. The libcap source tree includes a small webserver example:

https://git.kernel.org/pub/scm/libs/libcap/libcap.git/tree/go/web.go

-- update:

to add Go module build support, I've moved the web.go sources to:

https://git.kernel.org/pub/scm/libs/libcap/libcap.git/tree/goapps/web

AndrewGMorgan commented 4 years ago

I don't seem to be able to edit the tags associated with this bug. I'm not sure help is wanted for the code, but would appreciate any real world testing if folk want to stress test the proposed patch...

AndrewGMorgan commented 4 years ago

FYI As a follow up to https://github.com/golang/go/issues/1435#issuecomment-569543665 the latest two releases of libcap also contain Go module buildable versions of the libcap/cap and libcap/psx packages. I've done a write up of the web.go example (which I moved around to make it buildable with the module system) here:

https://sites.google.com/site/fullycapable/getting-started-with-go/building-go-programs-that-manipulate-capabilities

This doesn't address the native Go build issue pending a solution in the form of https://go-review.googlesource.com/c/go/+/210639 patch, but it does allow general support for mirroring system calls in a combined Go/CGo runtime.

AndrewGMorgan commented 4 years ago

The modified instructions over that write up for validating and getting started exploring the above patch are to do this to build the web binary:

$ cd bar
$ git clone https://go.googlesource.com/go
$ cd go
$ git fetch https://go.googlesource.com/go refs/changes/39/210639/38 && git checkout -b change-210639 FETCH_HEAD
$ cd src
$ ./make.bash
$ cd ../..
$ mkdir foo
$ cd foo
$ wget https://git.kernel.org/pub/scm/libs/libcap/libcap.git/plain/goapps/web/web.go
$ ../go/bin/go mod init web
$ CGO_ENABLED=0 ../go/bin/go build -tags allthreadssyscall web.go
$ ldd web
        not a dynamic executable

You can follow the rest of https://sites.google.com/site/fullycapable/getting-started-with-go/building-go-programs-that-manipulate-capabilities for how to use it.

AndrewGMorgan commented 4 years ago

I realize that the above is only just on topic for the initial subject of this bug ;) Here is an explanation of how to use the same packages to address the reason this bug was filed (currently via cgo, while the AllThreadsSyscall patch is still pending):

https://sites.google.com/site/fullycapable/getting-started-with-go/using-go-to-set-uid-and-gids

AndrewGMorgan commented 3 years ago

Note, one build target linux-ppc64 appears to fail the added tests for this feature. Resolving that is the subject of #42178.

makew0rld commented 3 years ago

Happy to see this is fixed, but when will this be "backported", if at all? What Go version will this fix occur in?

AndrewGMorgan commented 3 years ago

This change is currently included in the 1.16 development branch. No currently released version of Go contains it.

AndrewGMorgan commented 3 years ago

Noting known outstanding bugs with this support in go1.16beta1: