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

Options to deal with open file descriptors #4845

Closed
smitsohu opened this issue Jan 11, 2022 · 7 comments
Closed

Options to deal with open file descriptors #4845

smitsohu opened this issue Jan 11, 2022 · 7 comments
Labels
enhancement New feature request

Comments

@smitsohu
Copy link
Collaborator

Currently Firejail sandboxes inherit all open file descriptors from their parent process.

As far as standard streams (stdin, stdout, stderr) are concerned this is probably desired behaviour. On the other hand, passing non-standard file descriptors (> 2) into the sandbox is only rarely necessary in my experience.

IMHO Firejail should provide a mechanism to prevent accidental leakage of open file descriptors, either by default with a way to opt-out, or alternatively as an opt-in. I would like to know if there is general interest in options like

inheritfd (don't close one or more file descriptors > 2)
discard-stdin (-> /dev/null)
discard-stdout (-> /dev/null)
discard-stderr (-> /dev/null)

@smitsohu smitsohu added the enhancement New feature request label Jan 11, 2022
@glitsj16
Copy link
Collaborator

I'd say any prevention of accidental leakage is a welcome enhancement and the proposed options sound great.

On the other hand, passing non-standard file descriptors (> 2) into the sandbox is only rarely necessary in my experience.

Just out of curiosity, could you provide an example of such a case?

@rusty-snake
Copy link
Collaborator

Is there anything bad expect that you can read/write on that fd?

Just out of curiosity, could you provide an example of such a case?

firejail --noprofile --shell=none --private-bin=gnome-2048 /proc/self/fd/5 -l /usr/bin 5</usr/bin/ls

@reinerh
Copy link
Collaborator

reinerh commented Jan 11, 2022

Is there anything bad expect that you can read/write on that fd?

Open FDs are bad in simple chroots, as you can use them to escape with fchdir(fd) (if fd is outside). There is probably other bad stuff you can do with open FDs to get information from outside the jail.

Not sure how firejail behaves with --chroot. It's probably best to close everything.

@rusty-snake
Copy link
Collaborator

Yeah, if the open fd refers to a directory you can fchdir, openat, execveat, ... which is close to full filesystem access. But if it refers to a file? Is there something like get_basedirfd_of_fd? If anyone knows more, I'm interested.

@smitsohu
Copy link
Collaborator Author

smitsohu commented Jan 11, 2022

Is there anything bad expect that you can read/write on that fd?

It depends on the file descriptor. As a rule of thumb everything that can be done with a file descriptor outside the sandbox can be done also inside the sandbox. This includes reading/writing to files or interprocess communication.

Many file descriptors (those that are obtained by actually opening a file) are bound to a mount point, and inherited file descriptors necessarily refer to mount points outside the sandbox. So a worst case scenario would be a file descriptor that allows to bypass a blacklist or read-only directive, or if it refers to a directory to fchdir out of the sandbox entirely.

Pythons subprocess module by the way closes all non-standard file descriptors by default, or at least that is my understanding. If this is ok for a general purpose Python module, I think it should be okay'ish for Firejail as well.

Is there something like get_basedirfd_of_fd

Hopefully not!
If it exists, most if not all of the common sandboxing tools will have a problem, because, as you say, that would be equal to full unrestricted filesystem access.

@netblue30
Copy link
Owner

MHO Firejail should provide a mechanism to prevent accidental leakage of open file descriptors, either by default with a way to opt-out

It is rarely used, so we should prevent it by default + some command line option to opt-out.

smitsohu added a commit to smitsohu/firejail that referenced this issue Jan 14, 2022
smitsohu added a commit to smitsohu/firejail that referenced this issue Jan 14, 2022
smitsohu added a commit to smitsohu/firejail that referenced this issue Jan 14, 2022
smitsohu added a commit to smitsohu/firejail that referenced this issue Jan 14, 2022
smitsohu added a commit to smitsohu/firejail that referenced this issue Jan 14, 2022
smitsohu added a commit to smitsohu/firejail that referenced this issue Jan 14, 2022
smitsohu added a commit to smitsohu/firejail that referenced this issue Jan 14, 2022
netblue30 added a commit that referenced this issue Jan 16, 2022
smitsohu added a commit to smitsohu/firejail that referenced this issue Jan 16, 2022
@smitsohu
Copy link
Collaborator Author

smitsohu commented Jan 17, 2022

On the other hand, passing non-standard file descriptors (> 2) into the sandbox is only rarely necessary in my experience.

Just out of curiosity, could you provide an example of such a case?

I used man -wK 1 'file descriptor' for a first impression. There are only a few interesting hits, like for example in the git, git-remote-fd, gpg, openssl or tar man pages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request
Projects
None yet
Development

No branches or pull requests

5 participants