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

Support for running SFTP workers with specific UID/GID #614

Open
bruckins opened this issue Feb 6, 2025 · 1 comment
Open

Support for running SFTP workers with specific UID/GID #614

bruckins opened this issue Feb 6, 2025 · 1 comment

Comments

@bruckins
Copy link

bruckins commented Feb 6, 2025

Hi,

I am running an SFTP server as root, but I need to restrict users accessing the SFTP server to operate under a specific UID and GID.

Since unix.Setfsuid() and unix.Setfsgid() can be set at the thread level, I implemented this in my fork of the sftp package. To ensure each SFTP worker operates with the correct UID/GID, I modified the runWorker function in Serve() to:

  • Use runtime.LockOSThread() to bind the worker to a specific thread.
  • Set fsuid and fsgid at the beginning of each worker routine.
  • Unlock the thread when the worker exits.

I introduced a new option WithFileSystemUidGid(uid, gid), which allows users to define a specific UID and GID for file system access.

Code Changes (Inside Serve() in runWorker):

if svr.fsUID != -1 && svr.fsGID != -1 {
	runtime.LockOSThread()
	defer runtime.UnlockOSThread()

	if err := unix.Setfsuid(svr.fsUID); err != nil {
		fmt.Fprintf(svr.debugStream, "failed to setfsuid: %v\n", err)
		svr.conn.Close()
		return
	}
	if err := unix.Setfsgid(svr.fsGID); err != nil {
		fmt.Fprintf(svr.debugStream, "failed to setfsgid: %v\n", err)
		svr.conn.Close()
		return
	}
}

Is this the recommended approach for implementing per-worker UID/GID restriction in the sftp package? If there are better ways to achieve this functionality, I’d appreciate any guidance.

Thanks,
Bruckins

@puellanivis
Copy link
Collaborator

The recommended approach for implementing UID/GID restrictions is to do the same thing that the openssh SFTP server does: that is, have sshd handle inbound requests, do the necessary process of authenticating and authorizing the credentials, then fork+execing into the sftp-server binary. Generally, the only  safe way to handle per-user requests is to run a process per user. Otherwise, there will always be that root access sitting available on another goroutine, just waiting for a hacker to find a way to exploit it.

There are numerous security concerns when trying to manage the SSH credentials and assumption of user id. These concerns may seem trivial at the time, but it turns out that very minor things can be leveraged against you. Humans are just not used to the idea that if you don’t screw down the bolt on your Ikea bookcase to the precise torque specification, that your house becomes vulnerable to break ins. Remember: ⟨tapping the sign⟩ do not roll your own crypto (or security).

The SFTP library specifically places this sort of security as entirely outside of the scope of our library, and our example server specifically notes at the top: “Serves the whole filesystem visible to the user, and has a hard-coded username and password, so not for real use!”

Now, to the meat of your question: would I recommend this LockOSThread and Setfsuid. No. Golang uses pooled OS threads to run goroutines by design. Locking OS threads on a relatively permanent basis violates a core assumption of how Go works, and could have unexpected consequences on performance. Plus, if that locked OS thread spins off its own goroutines, now that goroutine has escaped your jail, and has free reign and free access. Having a security system that can be defeated by the difference of go doFunction() instead of doFunction() is generally not a good idea.

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