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

profiles: move blacklist of /etc/profile.d & blacklist /etc/profile #5167

Merged
merged 3 commits into from
May 31, 2022

Conversation

kmk3
Copy link
Collaborator

@kmk3 kmk3 commented May 30, 2022

disable-common.inc: move blacklist of /etc/profile.d

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

Cc: @hknaack (from #5159).

kmk3 added 3 commits May 30, 2022 14:25
This amends commit b6b3f3b ("kate.profile: allow common development
file access", 2022-05-28) / PR netblue30#5159.

See etc/templates/profile.template.
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.
Since /etc/profile.d is already being blacklisted.
@kmk3 kmk3 requested review from glitsj16 and rusty-snake May 30, 2022 17:52
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

Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

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

LGTM

  • Does anyone know why we blacklist them?
  • Should we add more shell startup files like /etc/bashrc, /etc/zpfoile, ...?

@kmk3
Copy link
Collaborator Author

kmk3 commented May 30, 2022

@rusty-snake left a comment:

LGTM

  • Does anyone know why we blacklist them?

Not exactly sure; it was added on commit 5db7520 ("profile work", 2015-09-22)
by @netblue30.

Maybe it's done as an extra layer of defense, as a program overwriting these
files is potentially as bad as it overwriting ~/.bashrc.

  • Should we add more shell startup files like /etc/bashrc, /etc/zpfoile,
    ...?

I think that would make sense.

Note that /etc/profile is listed on etc/ids.config, along with other such
paths:

### shells global ###
# all
/etc/dircolors
/etc/environment
/etc/profile
/etc/profile.d
/etc/shells
/etc/skel
# bash
/etc/bash_completion*
/etc/bash.bashrc
/etc/bashrc
# fish
/etc/fish
# ksh
/etc/ksh.kshrc
# tcsh
/etc/complete.tcsh
/etc/csh.cshrc
/etc/csh.login
/etc/csh.logout
# zsh
/etc/zlogin
/etc/zlogout
/etc/zprofile
/etc/zshenv
/etc/zshrc

@netblue30
Copy link
Owner

Short attention span here, doesn't go as far back as 2015! So let's move them!

@netblue30 netblue30 merged commit 6b248aa into netblue30:master May 31, 2022
@kmk3 kmk3 deleted the mv-sh-profile-blacklist branch June 2, 2022 03:08
kmk3 added a commit to kmk3/firejail that referenced this pull request Jun 2, 2022
Since /etc/profile is present, add the other shell-related paths in /etc
that are listed on ids.config.

Suggestion by @rusty-snake[1].

Relates to netblue30#5167 netblue30#5170.

[1] netblue30#5167 (review)
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.

4 participants