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

disable-shell.inc #3411

Merged
merged 4 commits into from
Jun 4, 2020
Merged

Conversation

rusty-snake
Copy link
Collaborator

No description provided.

@rusty-snake
Copy link
Collaborator Author

Initially I wanted to add it to disable-interpreters.inc, but to many program require allow-shell.inc. I think it is better to add it as an own disable-*.inc

Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Should we document somewhere that including disable-shell.inc will break join?

@rusty-snake
Copy link
Collaborator Author

Should we document somewhere that including disable-shell.inc will break join?

👍, we should also note that private-bin w/o $SHELL breaks join too.

@reinerh reinerh mentioned this pull request May 14, 2020
… private-bin line without bash/sh except profiles with redirect
profiles.
@@ -19,6 +19,7 @@ include disable-exec.inc
include disable-interpreters.inc
include disable-passwdmgr.inc
include disable-programs.inc
include disable-shell.inc
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aria2 supports on-download-complete=foo and we don't know what a user might put in a shell script. Perhaps we can add a comment, but the safest would be to not include disable-shell.inc here I guess. What do you think?

@@ -16,6 +16,7 @@ include disable-exec.inc
include disable-interpreters.inc
include disable-passwdmgr.inc
include disable-programs.inc
include disable-shell.inc
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More a FYI, this package has been removed from AUR a while back, due to incompatibilities with pacman. So perhaps we can drop this profile. But the include would be fine regardless.

@Fred-Barclay
Copy link
Collaborator

@rusty-snake how does this compare to shell none?

@rusty-snake
Copy link
Collaborator Author

disable-shell: blacklists shell inside the sandbox.
shell none: make firejail searching for the program in $PATH and exec* it direct by full path instead of /bin/bash -c firefox.

firejail will fail to start a program in a sandbox for profiles with disable-shell but w/o shell none.

@rusty-snake rusty-snake merged commit 2c914c7 into netblue30:master Jun 4, 2020
@rusty-snake rusty-snake deleted the disable-shell.inc branch June 4, 2020 11:55
@matu3ba matu3ba mentioned this pull request Oct 7, 2021
kmk3 added a commit to kmk3/firejail that referenced this pull request May 30, 2022
To disable-shell.inc.

Interactive shells can be executed from certain development-related
programs (such as IDEs) and the shells themselves are not blocked by
default, but this shell startup directory currently is.  To avoid
running a shell without access to potentially needed startup files, only
blacklist /etc/profile.d when interactive shells are also blocked.

Note that /etc/profile.d should only be of concern to interactive
shells, so a profile that includes both disable-shell.inc and
allow-bin-sh.inc (which likely means that it needs access to only
non-interactive shells) should not be affected by the blacklisting.

Relates to netblue30#3411 netblue30#5159.
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

Successfully merging this pull request may close these issues.

3 participants