OpenRC sets its own PATH variable in /etc/init.d/functions.sh so that certain executables are available in openrc init scripts. However, the value of PATH set by OpenRC is exported and leaks into login managers like emptty. emptty maintainer decided to use the value of PATH set by /etc/pam.d/system-login instead of the PATH value set exported from OpenRC. It took months to fix emptty because of this issue. I think OpenRC shouldn't modify PATH value passed to programs executed by OpenRC. Reproducible: Always
Created attachment 739653 [details] 0001-do-not-export-path-to-the-environment.patch I don't know how to reproduce your bug, but this patch will make OpenRC not export PATH. Please apply it locally and let me know if it fixes your issue. Thanks, William
It turns out that not exporting PATH doesn't change a thing because init scripts get the value of PATH modified by OpenRC. The init scripts just pass its PATH value to child processes.
Perhaps, OpenRC shouldn't rely on modified PATH value?
Can you please explain how I can reproduce the issue you are seeing? If I didn't modify the path, I would have to add all of the binaries in /lib/rc/* to /usr/bin or /usr/sbin, but I hesitate to do that.
It is not unusual for a package to have private binaries. It was suggested that start-stop-daemon and supervise-daemon could remove @libexecdir/bin and @libexecdir/sbin from the path before they spawn the daemon if they don't already. I will take a look at doing this.
Install gui-apps/emptty-0.6.0::crocket-overlay. Log in. Check the value of $PATH. emptty used to just inherit PATH from OpenRC and ignore PATH from PAM. Emptty started honoring PATH value from pam after 0.6.0.
PATH from OpenRC screws applications. PATH from /etc/profile.env and PAM /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/opt/bin:/usr/lib/llvm/12/bin:/usr/lib/llvm/11/bin PATH from OpenRC /bin:/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin:/opt/bin:/usr/lib/llvm/12/bin:/usr/lib/llvm/11/bin
/etc/pam.d/system-login contains session required pam_env.so envfile=/etc/profile.env which resets PATH.
I don't really know much about pam, so I'll ask the question here. Gentoo uses different pam settings for start-stop-daemon and supervise-daemon than openrc's defaults. Would it work to use openrc's defaults or would this break things too?
I don't know what you mean. Can you explain more clearly?
Gentoo overwrites openrc's default pam setups for start-stop-daemon and supervise-daemon. OpenRC's defaults are [here](https://github.com/openrc/openrc/tree/master/pam,) However they are overwritten inside Gentoo by the ones [here](https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-apps/openrc/files/start-stop-daemon.pam) This file is copied to /etc/pam.d/start-stop-daemon and /etc/pam.d/supervise-daemon. Would it be better to not use the files from Gentoo and use the files at the first link? William
/etc/init.d/emptty uses supervise-daemon. OpenRC's pam file for supervise-daemon doesn't source /etc/profile.env or /etc/pam.d/system-login. Gentoo's OpenRC package doesn't have a pam file for supervise-daemon. Gentoo OpenRC package needs to acquire a pam file for supervise-daemon.
I do not really see a issue here. Even if openrc sets it's own $PATH, you can override it or reset to default in your init script if your init script needs it. On the top of that it's a package that is not present in ::gentoo but in your personal overlay, and as such, it's issues can hardly being a bug of gentoo. Moreover, if it's a replacement for CLI login manager, instead of taking over actual TTY via init scripts (like openvt can do) you should use /etc/inittab. OpenRC has no business to spawn agetty for you, which is why it does not do it. For me this is candidate for WONTFIX.
(In reply to Piotr Karbowski from comment #13) > Moreover, if it's a replacement for CLI login manager, instead of taking > over actual TTY via init scripts (like openvt can do) you should use > /etc/inittab. OpenRC has no business to spawn agetty for you, which is why > it does not do it. I think your knowledge on this topic might be a bit outdated. OpenRC can run without sysvinit/inittab via openrc-init. An init script to start agetty via supervise-daemon was added in 2017.
Gentoo does provide pam.d files for supervise-daemon and start-stop-daemon. Those pam files look like this. ``` account required pam_permit.so session include system-services ``` That's all they have. Should I change them, or is there something going on in /etc/pam.d/system-services or files it includes?
I think supervise-daemon and start-stop-daemon should have pam_env.so envfile=/etc/profile.env because applications launched by start-stop-daemon and supervise-daemon may rely on the correct value of PATH.
I don't think init script writers should be aware of "undocumented" idiosyncracies of OpenRC. That's why /etc/pam.d/start-stop-daemon and /etc/pam.d/supervise-daemon should have something like session required pam_env.so envfile=/etc/profile.env
@zlogene: @sam: I'm bringing both of you into this because this is possibly a fix to pambase or the gentoo-specific openrc pam files. Please let me know how I should fix this, or if it is a fix needed in /etc/pam.d/system-services or /etc/pam.d/system-auth. Thanks, William
(In reply to crocket from comment #17) > I don't think init script writers should be aware of "undocumented" > idiosyncracies of OpenRC. That's why /etc/pam.d/start-stop-daemon and > /etc/pam.d/supervise-daemon should have > > something like > > session required pam_env.so envfile=/etc/profile.env You want to change the interface that some init scripts depends on. The app-misc/conmux does alter $PATH in init script and depends on this behavior, which the change you propose would render not working.
Another thing that worries me, is that even if you try to sanitize environment via PAM, isn't it so that it will only ever happen when s-s-d switch user? So init scripts that do not drop user would skip this code path and keep the modified $PATH anyway. nginx is one random example that gets started as root, so it can open ports and log files before it fork workers and drop to unprivileged user. So we have really two things here -- whatever openrc should sanitize/reset $PATH or not and another if pam is the way to go with it.
The more I think about this, I tend to agree that PAM isn't the way to fix this. There are two paths I can think of to fix it. 1. In start-stop-daemon and supervise-daemon, remove the openrc directories from PATH, or 2. never put the openrc directories in PATH and use wrapper functions to call them.
(In reply to William Hubbs from comment #21) > The more I think about this, I tend to agree that PAM isn't the way to > fix this. > > There are two paths I can think of to fix it. > > 1. In start-stop-daemon and supervise-daemon, remove the openrc > directories from PATH, or > 2. never put the openrc directories in PATH and use wrapper functions > to call them. Note that (1) is already implemented. The OPs bug (filed upstream at emptty: https://github.com/tvrzna/emptty/issues/45) doesn't even involve leaking /lib/rc (see https://github.com/tvrzna/emptty/issues/45#issuecomment-917844690.) emptty should not leak /lib/rc because emptty uses supervise-daemon (which removes /lib/rc from PATH.) Instead it appears OP is unhappy that PATH is not set to what they want in /etc/profile.env. I think if we are strictly talking about openrc /lib/rc in $PATH then we could try doing (2) or just ensuring that initscripts use one of the openrc utilities to spawn their services.
Hi crocket, This bug lead to some vigorous debates, and while it DID point out some issues, they aren't entirely openrc's. Firstly, my opinion on expected behavior during init scripts running. - /etc/profile.env is ALREADY sourced (provides default PATH) - in the init environment, it's valid to add additional things in PATH, e.g. the /lib/rc/ - start-stop-daemon & supervise-daemon go to great lengths to remove those PATH modifications, directly in their codebase, for child processes. - start-stop-daemon & supervise-daemon ALSO end up invoking pam for the child process, to ensure limits are set on services, and append /etc/environment to the config, plus any tweaks applied by /etc/security/pam_env.conf Bug 1: bugs in openrc consumers Any daemons that start directly, without using either s-s-d or s-d end up with a polluted PATH, or correct limits. I consider those daemons init scripts to be buggy. Bug 2: cosmetic-only Somewhere in OpenRC's path mangling, the path has duplicate entries: /bin:/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/bin:/usr/sbin Not actually problematic, just cosmetic Bug 3: potential bug in openrc, but also bike-shed: During init scripts, should /usr/local/*bin be before /*bin and /usr/*bin? For a user's normal sessions, likely yes. For an init script, maybe? Maybe it should be configurable? Bug 4: display managers... Now, we have that emptty is a special case among daemons: it's a display manager and by DESIGN, expected to start user sessions. Should the environment of the display manager leak directly to the user session? I would expect NOT, because it might have sensitive information in it. You gave us this instruction for reproduction. "Install gui-apps/emptty-0.6.0::crocket-overlay. Log in. Check the value of $PATH" That didn't look at the value of PATH inside emptty, which you can do so with /proc/$(pidof emptty)/environ I think your packaging of emptty should follow OTHER display managers in Gentoo, and correctly use: pamd_mimic system-local-login ${PN} auth account password session rather than including upstream's pam file. If you did need more complex behavior, wrapping around system-local-login or system-login would be best, with substack, NOT include. See sddm after pam-1.4-substack.patch is applied as an example. Pam stacks: # Things that require password/auth module types system-local-login -> system-login -> system-auth # Things that don't need pam password or auth module types, but do want account & session module types. system-services -> system-services
The problem was 1. The order of PATH entries was wrong. /usr/bin was prioritized over /usr/sbin, and /bin over /sbin. /usr/bin was also prioritized over /usr/local/bin. I couldn't override /usr/bin/something with /usr/local/bin/something. 2. There were duplicate PATH entries. 3. emptty-0.6.0 overrides PATH value set by PAM with PATH value inherited from OpenRC. emptty maintainer made sure that PATH value set by PAM is prioritized over PATH value inherited from OpenRC. 4. The PATH value passed from supervise-daemon was /bin:/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin:/opt/bin:/usr/lib/llvm/12/bin which has the problems mentioned in 1 and 2. 5. In the future, PAM may not be used. Then, emptty needs to be fixed again.
Hi Robin, (In reply to Robin Johnson from comment #23) > Bug 1: bugs in openrc consumers > Any daemons that start directly, without using either s-s-d or s-d end up > with a polluted PATH, or correct limits. I consider those daemons init > scripts to be buggy. That seems fair. Still, the PATH handling of OpenRC does strike me as being somewhat wishy-washy in nature. I shall attempt to substantiate this view. > > Bug 2: cosmetic-only > Somewhere in OpenRC's path mangling, the path has duplicate entries: > /bin:/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/bin:/usr/sbin > Not actually problematic, just cosmetic There's something else about this that strikes me as interesting, which is that it assembles PATH in an unconventional order. Certainly, it isn't respecting the order expressed by the value of PATH at the time that openrc-run is executed. Whether it should or shouldn't is another matter. Yet, if it is the position of OpenRC that the PATH should be composed in any particular fashion, I find it odd that it bucks widely-observed conventions. To clarify, let's consider how baselayout orders the above-mentioned paths. $ grep ^PATH /etc/env.d/50baselayout PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/opt/bin" If we ignore /opt/bin for a moment, I would say that this is conventional. In particular, note that both "sbin" precedes "bin" and that the /usr prefixed variants come first. Now, let's see how systemd goes about it. According to systemd.exec(5): "[...] systemd uses a fixed value of /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin" in the system manager. When compiled for systems with "unmerged /usr/" (/bin is not a symlink to /usr/bin), s not a symlink to /usr/bin), ":/sbin:/bin" is appended." Not only is this clearly explained and easy to understand, it happens to agree exactly with 50baselayout, /opt/bin notwithstanding. Of course, systemd is not the only such example and that is why I say that it is conventional. Yet, by the time /lib/rc/sh/functions.sh is executed, PATH has already been mangled in the manner that you describe, even before any of the shell code that purports to "Make a sane PATH" kicks in. So, while it is true that the duplicate paths only amount to a cosmetic issue, I would say that this quasi-deterministic ordering is not merely a cosmetic issue. Indeed, I would say that OpenRC treats the value of PATH in a way that is: * partial in nature * undocumented * difficult to reason with It's partial because the ordering of well-known paths is not respected. It's undocumented in so far as openrc-run(8) does not specify the expected behaviour. It's difficult to reason with; even reading and understanding functions.sh is insufficient because there are other parts of OpenRC that affect the value of PATH and it's not immediately clear that there is a singular approach at play. > > Bug 3: potential bug in openrc, but also bike-shed: > During init scripts, should /usr/local/*bin be before /*bin and /usr/*bin? > For a user's normal sessions, likely yes. For an init script, maybe? > Maybe it should be configurable? Paintbrush duly in hand ... That's an interesting question. To have them be situated at the front would present an obvious risk that a user might inadvertently influence the behaviour of a packaged runscript. For that matter, one might ask, "should they even be present at all?" After all, no packaged runscript should expect to be finding executables in /usr/local/*sbin. Granted, those that are writing custom runscripts - especially ones that they have no intention of distributing - can reasonably expect to be able to execute things there. The thing is, nobody is stopping them from doing so. Irrespective of what OpenRC does, they need only begin their scripts with PATH=/usr/local/sbin:/usr/local/bin:$PATH, or to fully qualify the paths of their locally deployed executables. Regardless, I see no reason whatsoever that it should be configurable. As noted, PATH is already configurable in the sense that runscript authors are free to do as they please with it. If anything, I think that the behaviour of OpenRC should become simpler rather than more complicated. For example, if /usr/local/*bin are deemed to be problematic, one might take that sentiment to its logical conclusion, exercise the courage of one's convictions and just ensure that they are stripped out altogether. Otherwise, I would suggest that it makes sense to focus on better defining - and even cleaning up - the existing routines. On that note, I'm attaching a patch to functions.sh. While it's just a proof-of-concept, I think that it improves upon the existing code by being easier to digest and by making the behaviour more deterministic and easier to explain. Like systemd.exec, it begins with a well-defined, fixed value of PATH that is intended to be suited to a service manager (including /usr/local/*bin though it could be changed). This is named "standard_path". Unlike systemd.exec, it preserves the paths that were already defined by appending them in the order that they are found, except for those that are already defined by standard_path, thus dealing with most of the cosmetic issues. The _sanitize_path function is dropped - it didn't perform any sanitization anyway. By contrast, the patch introduces some degree of sanitization by disregarding empty and non-relative pathnames and makes the overall process safer by disabling pathname expansion. The patch can be considered incomplete in so far as it doesn't attempt to touch - or unify - any of the other parts of OpenRC that affect PATH.
Created attachment 743349 [details, diff] lib-rc-sh-functions.patch One possible approach for improving the PATH mangling code in /lib/rc/sh/functions.sh.
I think this is why an init script shouldn't run on a complex shell script system. s6 and runit wouldn't do this to me. GNU Shepherd init system is written in guile scheme. Thus, it manages to not touch PATH.
emptty now sets its own environment through PAM.
