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

fish doesn't run in bubblewrap jail #9605

Open
e00E opened this issue Feb 21, 2023 · 5 comments
Open

fish doesn't run in bubblewrap jail #9605

e00E opened this issue Feb 21, 2023 · 5 comments
Milestone

Comments

@e00E
Copy link

e00E commented Feb 21, 2023

I use bubblewrap to run sandboxes/containers. Bubblewrap has an option --new-session documented in the man page as follows:

Create a new terminal session for the sandbox (calls setsid()). This disconnects the
sandbox from the controlling terminal which means the sandbox can't for instance inject
input into the terminal.

Note: In a general sandbox, if you don't use --new-session, it is recommended to use
seccomp to disallow the TIOCSTI ioctl, otherwise the application can feed keyboard
input to the terminal.

Since I care about security I want to use this option. With this option, fish does not run in the sandbox. Fish outputs the following and exists with code 1:

warning: No TTY for interactive shell (tcgetpgrp failed)
setpgid: Inappropriate ioctl for device

A minimal reproduction is running bwrap --new-session --dev-bind / / fish .

I understand that some features of fish might be limited with this bubblewrap option, but it would still be useful to run fish. I do not know enough about the details of linux terminals to know how reasonable this is. bash does run in the sandbox after printing a warning about "inappropriate ioctl for device".

fish: 3.6.0
terminal: alacritty
OS: arch linux

@faho
Copy link
Member

faho commented Feb 21, 2023

Yeah, no, fish needs control of the terminal.

So the solution here is simple: Don't do that.

warning: No TTY for interactive shell (tcgetpgrp failed)

What this means is that fish can't find out which process group is in control of the terminal. It needs that to be able to acquire the terminal for itself, so that it can set terminal modes and hand off control to other processes.

E.g. fish needs to be able to turn off terminal echoing so that it can do syntax highlighting and autosuggestions, and it needs to be able to allow other processes to do the same thing.

Without this, fish would be broken. It won't work.

@faho faho added the question label Feb 21, 2023
@e00E
Copy link
Author

e00E commented Feb 21, 2023

Thanks for the answer. I don't understand then how it can be possible that in the same sandbox, if I run tmux first, I can run fish and fish does have autosuggestions etc. It seems like on a technical level it is possible but I need some kind of wrapper program. If tmux can do it, couldn't fish natively do whatever tmux does?

@faho
Copy link
Member

faho commented Feb 21, 2023

Fish is a shell, which requires a bunch more things from the terminal (when it comes to things like job control). Tmux, in contrast, is basically its own terminal.

Fish also does some more complicated drawing and cursor movement, so it requires more than e.g. bash (which almost just prints the text).

What happens here is fish asks the terminal for its controlling process group, and the answer it gets is "this isn't a terminal" - it explicitly gets the "ENOTTY" error.

It's possible this logic could be improved, but tbh I don't believe this is an area we really want to touch. I also don't really see what this option is supposed to protect against - your shell is going to run arbitrary commands, injecting things into the terminal is the least of your worries. It's also not great even with bash - which will warn you that it can't enable job control. (edit: zsh will also not have job control available, it just won't even tell you)

The gist here is: This is a use-case I do not believe we want to support.

@faho faho closed this as completed Feb 21, 2023
@faho faho removed the question label Feb 21, 2023
@faho faho added this to the will-not-implement milestone Feb 21, 2023
@e00E
Copy link
Author

e00E commented Feb 21, 2023

I understand your decision. Let me expand on the following though:

I also don't really see what this option is supposed to protect against - your shell is going to run arbitrary commands, injecting things into the terminal is the least of your worries.

Bubblewrap is a flexible sandbox. For example, I can run arbitrary commands inside of it without fear of these commands touching the rest of my system. Programs in the sandbox cannot break out. This includes a shell running in the sandbox.

The --new-session switch protects against the aforementioned TIOCSTI attack. With this attack, when running an interactive shell inside a sandbox, sandbox escape is possible by putting characters into the outer terminal, which is not part of the sandbox. A malicious program in the sandbox could put curl ... | sudo ... into the input buffer and without the switch it might get executed after the sandbox ends. This is a real, practical attack. This attack is why sudo and su have the --pty switch.

@ridiculousfish
Copy link
Member

ridiculousfish commented Feb 22, 2023

I dug into this a bit, using the repro case of bwrap --new-session --dev-bind / / ./fish. Thank you for providing that command, it made it much easier to debug.

I was able to reproduce your error. The basic issue is that fish checks if stdin is a tty via isatty; after confirming it is fish attempts to activate job control via the tc{get,set}pgrp family of functions and here receives ENOTTY: stdin is not a tty. This is seemingly absurd: it's no fair that isatty reports a tty and then later we get an ENOTTY error.

It seems that bubblewrap hopes to allow interactive usages (you get a tty) but without job control bits (no tty ownership); this is technically possible but would be a new mode for fish.

Confirming what faho wrote, I tried with bash:

bash: cannot set terminal process group (-1): Inappropriate ioctl for device
bash: no job control in this shell

Also confirming faho, zsh "works" but ignores errors. For example yes & should be immediately backgrounded, attempt to print to the tty, get SIGTTOU, and be stopped. That's normal Unix/Linux. But in this bubblewrap world, zsh fails to background yes but runs it anyways in the foreground. That's pretty bad.

To sum up, bubblewrap in this mode seems to present a tty without foreground process group management. This is very unusual. We might be able to support it cheaply by simply disabling job control at startup if tcgetpgrp or whatever fails. I need to think about it.

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

4 participants