FS#47884 - [filesystem] /etc/profile should check whether PATH is empty before overwriting it

Attached to Project: Arch Linux
Opened by Murukesh Mohanan (murukesh) - Monday, 25 January 2016, 03:11 GMT
Last edited by Sébastien Luttringer (seblu) - Sunday, 10 December 2017, 16:06 GMT
Task Type Bug Report
Category Packages: Core
Status Closed
Assigned To Sébastien Luttringer (seblu)
Architecture All
Severity Very Low
Priority Low
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Description:

/etc/profile contains the following lines:

# Set our default path
PATH="/usr/local/sbin:/usr/local/bin:/usr/bin"
export PATH

For anyone using pam_env (i.e., /etc/environment and ~/.pam_environment) to
manage their PATH, this is horrendous.

You should at least check if PATH is empty before overwriting it willy
nilly.

Additional info:
* package version(s): filesystem 2015.09-1
* config and/or log files etc.: /etc/profile

Steps to reproduce:

Enable pam_env in PAM configuration (which it is by default in
/etc/pam.d/system-login).

Set in /etc/environment:

PATH=/usr/local/sbin:/usr/local/bin:/usr/bin

and in ~/.pam_environment:

FOO=/some/dir
PATH DEFAULT=${PATH}:${FOO}/bin

Re-login (or SSH to localhost, or login in another TTY ...).

Run `echo $PATH` in a terminal.

Expected:

/usr/local/sbin:/usr/local/bin:/usr/bin:/some/dir/bin

Instead, I get:

/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl

(The latter paths are not relevant to this bug.)

Worse yet, if you do use .pam_environment in the manner shown (appending
to $PATH) without setting a full value before doing so, PATH is set to just
:/some/dir/bin, which just opens up a security hole. In practice, this
messes up SSh-based commands like ssh-copy-id or scp, which fail with
very mysterious errors (laptop being the Arch system):

~ scp foo laptop:
zsh:1: command not found: scp
lost connection

The reason no one noticed this might be because SSH has a default PATH.
According to /etc/ssh/sshd_config:

# This sshd was compiled with PATH=/usr/bin:/bin:/usr/sbin:/sbin

And that's what I get when I SSH after commenting out all mentions of PATH
in /etc/environment and ~/.pam_environment. So by default, even though
/etc/profile isn't sourced for non-login shells, I still get a PATH of
/usr/bin:/bin:/usr/sbin:/sbin when I run ssh host some-command.

At least the following would be good:

if [ -z "$PATH" ]; then
# Set our default path
PATH="/usr/local/sbin:/usr/local/bin:/usr/bin"
export PATH
fi

Ideally, the default PATH should be set in /etc/environment. I'm using zsh if
it matters, and I don't use or care about zshenv.
This task depends upon

Closed by  Sébastien Luttringer (seblu)
Sunday, 10 December 2017, 16:06 GMT
Reason for closing:  Implemented
Additional comments about closing:  filesystem 2017.10-2
Comment by Dave Reisner (falconindy) - Thursday, 28 January 2016, 21:01 GMT
You seem to assume that a shell would be started with an empty path if one didn't involve pam_environment.so to set it beforehand, but this simply isn't correct. Your suggestion would leave many users with a crippled PATH.
Comment by Murukesh Mohanan (murukesh) - Friday, 29 January 2016, 00:10 GMT
I didn't assume that as I clearly have pointed out a case where that isn't true: SSH.

That was the simplest test I could think of, but there could be other tests. Doesn't change my point that pam_env, which is explicitly meant for managing environment variables, should not be overridden by the *default* /etc/profile. The default setup does use pam_env, so it should leave environment variables to pam_env.

Another way could be to test if the given directories are already in PATH:

http://unix.stackexchange.com/a/4973/70524

http://unix.stackexchange.com/a/124447/70524
Comment by Dave Reisner (falconindy) - Friday, 29 January 2016, 16:32 GMT
Maybe the lesson to be learned is that you shouldn't manage your PATH with pam_environment and instead use something shell-based, but largely shell agnostic, like ~/.profile.
Comment by Murukesh Mohanan (murukesh) - Friday, 29 January 2016, 16:53 GMT
Thanks, but, no thanks. pam_env works reliably across distros unless the distro maintainers decide to mess up. Maybe the lesson to be learned is to use the best tool for the job and pray the maintainers don't decide to mess up. I really don't want start a "but $OTHER_DISTRO does X" discussion.

I don't know how you're missing it, but there are situations where .profile doesn't cut it. SSH is, again, the most obvious case. Unless you let SSH start an interactive shell, neither .profile nor /etc/profile is sourced.
So, if I have /some/path/bin, and I want it in my PATH, and I want it in my PATH no matter what so that I can run ssh laptop foo where foo is /some/path/foo, pam_env is the simplest option.

Yes, pam_env has deficiencies, most notably the absence of $HOME. People have worked around that [1], but if this change won't happen, that is pretty much beyond hope.

Unless you want me to go down the zshenv road, which is very shell-specific, and has led to enough complaints here already.

[1]: https://fedorahosted.org/linux-pam/ticket/24

Comment by Dave Reisner (falconindy) - Friday, 29 January 2016, 17:01 GMT
The most notable deficiency of pam_env is that it can't append to existing vars. That's what you're fighting against here.

> I really don't want start a "but $OTHER_DISTRO does X" discussion.
Discussing what other distros do would, in fact, provide useful data points.
Comment by Murukesh Mohanan (murukesh) - Friday, 29 January 2016, 18:48 GMT
pam_env can append to existing vars, in fact, that is precisely what the example in my bug report does. Admittedly, the documentation of pam_env is poorly written. This, FYI, is the syntax for appending:

FOO=/some/dir
PATH DEFAULT=${PATH}:${FOO}/bin

Note the DEFAULT=. That's what triggers expansion of variables (named using ${...}). I can do it as many times as I need:

PATH DEFAULT=${PATH}:/some/other/dir
PATH DEFAULT=${PATH}:/a/third/dir

Say you have these four lines in .pam_environment, but no initial value is set for PATH, and if you do: ssh localhost /usr/bin/env, you'll see:

PATH=:/some/dir/bin:/some/other/dir:/a/third/dir

As I said, ideally the PATH would be defined in /etc/environment. Then, I can append to it as I need, and that can be done in .pam_environment or .profile as needed. The problem with the current defaults is that any changes I do make to the PATH in .pam_environment are lost when /etc/profile gets sourced.

As for other distros:

1. Ubuntu uses /etc/environment. It works without problems. [1]
2. Debian uses /etc/profile, but there was a bug [2] filed long, *long* ago about using /etc/environment, without much activity. I'll need to poke it.
3. Fedora uses a function to add paths to PATH, like in one of the links I suggested in my first comment. The relevant bits in /etc/profile [3]:

pathmunge () {
case ":${PATH}:" in
*:"$1":*)
;;
*)
if [ "$2" = "after" ] ; then
PATH=$PATH:$1
else
PATH=$1:$PATH
fi
esac
}

...

# Path manipulation
if [ "$EUID" = "0" ]; then
pathmunge /usr/sbin
pathmunge /usr/local/sbin
else
pathmunge /usr/local/sbin after
pathmunge /usr/sbin after
fi

...

export PATH USER LOGNAME MAIL HOSTNAME HISTSIZE HISTCONTROL

...

unset -f pathmunge

However, apparently Fedora doesn't use pam_env otherwise, and I don't know where the default PATH is set. It isn't in /etc/environment on a live system. It is nice of them to not overwrite PATH, though.


[1]: http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/wily/pam/wily/view/head:/debian/libpam-modules.postinst#L23
[2]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=65611
[3]: https://git.fedorahosted.org/cgit/setup.git/tree/profile#n11
Comment by Murukesh Mohanan (murukesh) - Friday, 13 January 2017, 00:26 GMT
The change in priority is disappointing, but I suppose you can close this bug. I just delete Arch's profile and bashrc and copy over Ubuntu's now.
Comment by Sébastien Luttringer (seblu) - Sunday, 02 April 2017, 20:59 GMT
The change of priority is my personal organisation. Don't take it personally.

I finally looked into this BR and I implemented something like fedora does in our profile.
Comment by Murukesh Mohanan (murukesh) - Monday, 03 April 2017, 01:35 GMT Comment by Johannes Wienke (languitar) - Wednesday, 26 April 2017, 08:31 GMT
I also just stumbled across this issue and would be glad to see a solution.
Comment by Tad Fisher (tad) - Saturday, 29 April 2017, 16:19 GMT
A more "modern" approach could be to use systemd's environment.d generator: https://www.freedesktop.org/software/systemd/man/environment.d.html

It should be fairly simple and arguably more cohesive to port the parts of profile.d that set PATH to /usr/lib/environment.d/*.conf files. Then users could bypass the pam_environment/profile.d mess altogether by adding files in ~/.config/systemd/environment.d .
Comment by Sébastien Luttringer (seblu) - Wednesday, 30 August 2017, 23:52 GMT
Now in trunk. A test package is available here: http://pkgbuild.com/~seblu/filesystem-2017.08-0.3-x86_64.pkg.tar.xz.

Will look environment.d later, but that sound promising.
Comment by Peter Wu (Lekensteyn) - Monday, 30 October 2017, 00:02 GMT
Just a heads-up, this change (using the previous $PATH) has revealed an issue in Ninja/CMake/Clang which is caused by SDDM defaulting to /bin:/usr/bin:/usr/local/bin.

Here is a patch proposing to change the order of SDDM as a stop-gap measure, but perhaps it should be extended to respect the previous PATH?
https://github.com/sddm/sddm/pull/922
Comment by Murukesh Mohanan (murukesh) - Monday, 30 October 2017, 03:17 GMT
@Lekensteyn, yes, please extend it to keep an existing PATH if possible. An SSH like behavior (set to sane default if unset) would be nice, but overwriting the PATH without checking isn't good.
Comment by Eli Schwartz (eschwartz) - Monday, 30 October 2017, 03:34 GMT
This should be fixed by the filesystem package in [testing], have you actually tried it before power-posting?

EDIT: Sorry, misunderstood the comment.
Comment by Murukesh Mohanan (murukesh) - Wednesday, 01 November 2017, 06:59 GMT
@eschwartz OK... but Lekensteyn's comment (and my follow up) was about the fixed filesystem package revealing a related issue in SDDM.
Comment by Eli Schwartz (eschwartz) - Thursday, 09 November 2017, 23:12 GMT
This is sort of but not really related to  FS#55482  with a different cause:

@falconindy,

The current version of filesystem incorrectly orders /usr/bin before /usr/local/bin
I've determined that this is caused by /etc/login.defs which is part of the shadow package... perhaps it should set ENV_SUPATH and ENV_PATH to /usr/local/sbin:/usr/local/bin:/usr/bin which is the Arch defaults. Currently /etc/profile starts off with PATH initialized to /usr/bin and skips over that directory while appending all the rest.

@seblu,

I think appendpath should not be unset until after /etc/profile.d/*.sh is sourced, then other profiles can take advantage of this. e.g. perlbin.sh
Also, the systemd generator may be working for nspawn containers, but it does nothing for the host system, see above.

Is this how systemd is supposed to work?

Loading...