> On Feb. 4, 2013, 3:53 p.m., Konstantinos Smanis wrote: > > We can do better than hardcoding a reasonable default. We can launch a > > login shell (1) and 'echo $PATH' the user's PATH. This has many advantages: > > > > 1. We don't miss paths (e.g. /usr/local/bin, /usr/local/sbin etc.). > > 2. We honor the precedence of paths as set in $PATH by the user. > > 3. We only use the user's PATH (DBus activation works for non-root users > > too). > > > > I am currently working on this approach for kcm-grub2 which also misbehaves > > when $PATH is not set. If you are interested, you may restrain from > > committing until I post a link to the commit. > > > > (1) A login shell is needed to properly source /etc/profile, ~/.profile > > and/or other shell-specific login scripts (such as ~/.bash_profile for > > Bash). > > Konstantinos Smanis wrote: > Here you are: > http://commits.kde.org/kcm-grub2/7c5beb979fdf9dd14abfffb0e24d4f69b11ca985 > > Thomas Lübking wrote: > Question (for the particular case and in general): > This is about a suid root:dbus call (thus env clearing for dbus system > activation) correct? > Ie. the called application is executed with root privs? > > In this case there should afaics rather not be _any_ PATH resolution at > all and checking the usual suspects is about the last acceptable process. > > Otherwise one could place a random binary "zic" or "hwclock" into the > $PATH and gain a root shell (or are there further protections against this?) > > Kevin Kofler wrote: > * This code (and any KAuth helper, actually) always runs as root. > * We're looking for 2 core system executables, the chances they are in > /usr/local are near zero, and I'd also share Thomas Lübking's security > concerns. (D-Bus clears the path for a reason.) > * Spawning a login shell looks like a really overengineered and ugly > solution, considering the above. > * kcm-grub2 is the last piece of software I'd model a KAuth helper on, > given that your KAuth actions are implemented in a totally insecure way. > (Last I checked, the process running as the user passes an arbitrary file to > the helper running as root, defeating the whole purpose of finegrained > PolicyKit permissions. Give a user that PolicyKit permission and you > essentially gave him root. Of course, it's not a problem if you stick to the > default auth_admin policy, but if some local admin tries to change it, ouch!) > > Kevin Kofler wrote: > Oh, and looking at your commit, what you did before was even worse, of > course. Passing PATH from the main application to the helper is totally > unacceptable for the same reason as passing an arbitrary file is, see my > previous comment and Thomas Lübking's comment.
@Thomas: Afaik modifying the root's $PATH requires root privileges. It is defined in /etc/profile and /root/.profile usually. Not messing with root's $PATH is definitely a precaution but I can't see how one could take advantage of it. @Kevin: * Afaik not all helpers run as root, you can drop privileges in the .service/.conf files to the desired user (i.e. apache). * I already commented on D-Bus precautions. * Sure, this is just a suggestion, you are free to pass on it. Personally I find hardcoding stuff less elegant than overengineered solutions. * Going off-topic, kcm-grub2 is definitely a typical application that needs a KAuth helper. I agree that the implementation can be vastly improved but I can't see how this is related to the proposal above. * Lastly, completely unrelated as it may be, I can't but agree that the workaround used before my commit simply sucked. Feel free to enlighten me if you find a valid compromise scenario. - Konstantinos ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108711/#review26617 ----------------------------------------------------------- On Feb. 2, 2013, 8:27 a.m., Kevin Kofler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/108711/ > ----------------------------------------------------------- > > (Updated Feb. 2, 2013, 8:27 a.m.) > > > Review request for kde-workspace, Christoph Feck and Oswald Buddenhagen. > > > Description > ------- > > Unfortunately, we cannot rely on the $PATH environment variable in KAuth > helpers, because D-Bus activation clears it. So we have to use a reasonable > default for the KStandardDirs::findExe search path, and actually use the > return value of KStandardDirs::findExe in the calls to KProcess::execute. > > This fixes things so hwclock and zic actually get found. See: > https://bugzilla.redhat.com/show_bug.cgi?id=906854 . This got noticed in > Fedora 18 because it does not always create /etc/localtime, so the fallback > code operating on /etc/localtime triggered an error. > > > Diffs > ----- > > kcontrol/dateandtime/helper.cpp 5a946d8 > > Diff: http://git.reviewboard.kde.org/r/108711/diff/ > > > Testing > ------- > > Builds against at least 4.10.0 and 4.9.5. > > Works at runtime on Fedora 18, see: > https://bugzilla.redhat.com/show_bug.cgi?id=906854#c12 (The reporter > encountered another issue, apparently because ktimezoned also misbehaves when > /etc/localtime is absent, but at least this particular issue is confirmed > fixed.) > > > Thanks, > > Kevin Kofler > >
