Re: [systemd-devel] Fix PAM module to not clobber XDG_RUNTIME_DIR with su
On Thu, 21.11.13 07:55, Martin Pitt (martin.p...@ubuntu.com) wrote: So, what's the intention here? That XDG_RUNTIME_DIR is entirely unset after su? That sounds kinda acceptable to me. Yes, for su - and pkexec. It might not work for su as that is likely configured to not run PAM, see above. That's what my patch is doing and what would prevent the damaging of runtime dirs. It's not what I consider ideal (I prefer Colin's approach of giving him the *correct* user's runtime dir), but if we can't have that, let's at least not pass the wrong one. Due to the lifecycle guarantees on XDG_RUNTIME_DIR we cannot hand out a correct (correct by what I assume you mean by correct) instance of it wihtout also setting up a full new session, and keeping it around and ref counting it. Anyway, as mentioned elsewhere, git now will not set XDG_RUNTIME_DIR in su instances, as per your original patch. Thanks for the patch, Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Fix PAM module to not clobber XDG_RUNTIME_DIR with su
On Thu, 14.11.13 07:45, Martin Pitt (martin.p...@ubuntu.com) wrote: Hello all, pam_systemd currently causes some havoc when you run programs or shells with su: it passes on the $XDG_RUNTIME_DIR from the original user session, so that programs like pulseaudio or dconf end up scribbling into the original user's runtime dir. This has been discussed at length at [1][2] and is leading people to consider workarounds like [3]. It seems Lennart is against giving the new user a new logind session and runtime dir; I think it would be right to give it a fresh (or an already existing one for the target user) runtime dir, but in either case passing it the original user's runtime dir is actively wrong and harmful. Well, that's quite arbitrary. What about dbus, X11, and so on, do you plan to turn that off for the new session too? If you leave access to X11 from the original session around, why isn't PA also left around? su is a hack, it is not clear what credentials it changes and which ones it doesn't. It's entirely random what people think su should do, and it's a security nightmare, as nobody knows the environment programs run in anymore, there's no chance to get this done correctly. Quit frankly, I am pretty sure the best approach is to simply prohibit running graphical applications from su sessions, it's never going to work. Letting other user access some (but not all) of a private user's bits and pieces is never going to work if those bits and pieces are nowadays a mix of dconf, X11, PA, dbus, security creds, keyrings, yadda yada... Until then I recommend applying this patch (or something equivalent) which at least stops destroying existing runtime dirs and makes it compliant to the spec [4]. With that, things like pulse, dconf, or dbus will still need to keep their internal fallback if there is no runtime dir, but that's a less pressing matter. So, what's the intention here? That XDG_RUNTIME_DIR is entirely unset after su? That sounds kinda acceptable to me. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Fix PAM module to not clobber XDG_RUNTIME_DIR with su
]] Lennart Poettering Well, that's quite arbitrary. What about dbus, X11, and so on, do you plan to turn that off for the new session too? Yes, please. In the following, I'm talking about «su -» not plain «su», which I think should go away since the semantics are woolen. su is a hack, it is not clear what credentials it changes and which ones it doesn't. It's entirely random what people think su should do, and it's a security nightmare, as nobody knows the environment programs run in anymore, there's no chance to get this done correctly. I don't see it as any more arbitrary than login or ssh. (ssh can transfer a bunch of credentials just fine, think Kerberos GSSAPI delegation or agent forwarding.) That we're not tracking loginuid across the network is just a limitation of the tools, there's nothing inherent which says that we should stop at a host boundary. Older, weaker protocols exist for tracking that, such as ident. Quit frankly, I am pretty sure the best approach is to simply prohibit running graphical applications from su sessions, it's never going to work. Letting other user access some (but not all) of a private user's bits and pieces is never going to work if those bits and pieces are nowadays a mix of dconf, X11, PA, dbus, security creds, keyrings, yadda yada... If you want to run graphical applications through su, you need to do xhost +SI:localuser:$newuser and set DISPLAY correctly. That's fine, and I think requiring people to do so is fine. Until then I recommend applying this patch (or something equivalent) which at least stops destroying existing runtime dirs and makes it compliant to the spec [4]. With that, things like pulse, dconf, or dbus will still need to keep their internal fallback if there is no runtime dir, but that's a less pressing matter. So, what's the intention here? That XDG_RUNTIME_DIR is entirely unset after su? That sounds kinda acceptable to me. Either unset or set to the new user's XDG_RUNTIME_DIR. The main point is «it should not be wrong» (which it is today). If we can make it point somewhere sensible that's a bonus, but not required. -- Tollef Fog Heen UNIX is user friendly, it's just picky about who its friends are ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Fix PAM module to not clobber XDG_RUNTIME_DIR with su
Hey Lennart, Lennart Poettering [2013-11-21 0:16 +0100]: On Thu, 14.11.13 07:45, Martin Pitt (martin.p...@ubuntu.com) wrote: Hello all, pam_systemd currently causes some havoc when you run programs or shells with su: it passes on the $XDG_RUNTIME_DIR from the original user session, so that programs like pulseaudio or dconf end up scribbling into the original user's runtime dir. This has been discussed at length at [1][2] and is leading people to consider workarounds like [3]. It seems Lennart is against giving the new user a new logind session and runtime dir; I think it would be right to give it a fresh (or an already existing one for the target user) runtime dir, but in either case passing it the original user's runtime dir is actively wrong and harmful. Well, that's quite arbitrary. What about dbus, X11, and so on, do you plan to turn that off for the new session too? At the moment, there is no dbus to turn off -- su - otheruser does not take the environment from the original session (if it does, it's misconfigured); and even if it does, it wouldn't work as otheruser cannot access the original user's session or user bus anyway. As to what happens to $DISPLAY, I have no strong opinion about it. But that's not the concern of libpam-systemd anyway. su is a hack, it is not clear what credentials it changes and which ones it doesn't. I think people keep confusing su and su - here. The manpage says -, -l, --login Provide an environment similar to what the user would expect had the user logged in directly. which shows the intention pretty well: It should be by and large similar to what I get when I log into a VT. It doesn't do that now as it doesn't get a seat and a runtime dir, but concerning credentials it should fully switch to otheruser's uid, groups, etc. It runs a full PAM stack after all, which should prepare the environment accordingly. This is similar to pkexec. su user is an entirely different matter, as you say. It should do little more than calling setgid()/setuid(), so there is no hope that this could ever do anything sensible for a program which depends on the environment. I don't remember ever having used it. Please let's keep it out of the discussion, since it isn't related to pam-systemd and as you say its semantics are pretty much broken anyway. Quit frankly, I am pretty sure the best approach is to simply prohibit running graphical applications from su sessions, it's never going to work. Fine for me. But trying to run them should properly fail then, not clobber the other user's home directory and then cause failures for the original user. Letting other user access some (but not all) of a private user's bits and pieces is never going to work if those bits and pieces are nowadays a mix of dconf, X11, PA, dbus, security creds, keyrings, yadda yada... Exactly my point! Hence, we must *not* pass another user's runtime dir in logind/the PAM module. So, what's the intention here? That XDG_RUNTIME_DIR is entirely unset after su? That sounds kinda acceptable to me. Yes, for su - and pkexec. It might not work for su as that is likely configured to not run PAM, see above. That's what my patch is doing and what would prevent the damaging of runtime dirs. It's not what I consider ideal (I prefer Colin's approach of giving him the *correct* user's runtime dir), but if we can't have that, let's at least not pass the wrong one. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Fix PAM module to not clobber XDG_RUNTIME_DIR with su
Hi Martin, Thanks for looking at this. 'Twas brillig, and Martin Pitt at 14/11/13 07:45 did gyre and gimble: pam_systemd currently causes some havoc when you run programs or shells with su: it passes on the $XDG_RUNTIME_DIR from the original user session, so that programs like pulseaudio or dconf end up scribbling into the original user's runtime dir. This has been discussed at length at [1][2] and is leading people to consider workarounds like [3]. It seems Lennart is against giving the new user a new logind session and runtime dir; I think it would be right to give it a fresh (or an already existing one for the target user) runtime dir, but in either case passing it the original user's runtime dir is actively wrong and harmful. Until then I recommend applying this patch (or something equivalent) which at least stops destroying existing runtime dirs and makes it compliant to the spec [4]. With that, things like pulse, dconf, or dbus will still need to keep their internal fallback if there is no runtime dir, but that's a less pressing matter. Thanks for considering, Martin [1] https://bugzilla.redhat.com/show_bug.cgi?id=753882 [2] https://launchpad.net/bugs/1197395 [3] http://lists.freedesktop.org/archives/pulseaudio-discuss/2013-November/019121.html [4] http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html I'm somewhat on the fence, but I think this patch is sensible in the short term at least. I do still think we need some kind of new su which is actually able to properly proxy graphics and sound (like SSH kinda does - at least for graphics), but this should prevent the nasty side effects in the short term. I've not considered any unwanted side effects this may cause so hopefully someone else can chime in accordingly. Your argument about it making it spec compliant seems rather compelling tho'. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Fix PAM module to not clobber XDG_RUNTIME_DIR with su
'Twas brillig, and Colin Guthrie at 14/11/13 09:48 did gyre and gimble: Hi Martin, Thanks for looking at this. 'Twas brillig, and Martin Pitt at 14/11/13 07:45 did gyre and gimble: pam_systemd currently causes some havoc when you run programs or shells with su: it passes on the $XDG_RUNTIME_DIR from the original user session, so that programs like pulseaudio or dconf end up scribbling into the original user's runtime dir. This has been discussed at length at [1][2] and is leading people to consider workarounds like [3]. It seems Lennart is against giving the new user a new logind session and runtime dir; I think it would be right to give it a fresh (or an already existing one for the target user) runtime dir, but in either case passing it the original user's runtime dir is actively wrong and harmful. Until then I recommend applying this patch (or something equivalent) which at least stops destroying existing runtime dirs and makes it compliant to the spec [4]. With that, things like pulse, dconf, or dbus will still need to keep their internal fallback if there is no runtime dir, but that's a less pressing matter. Thanks for considering, Martin [1] https://bugzilla.redhat.com/show_bug.cgi?id=753882 [2] https://launchpad.net/bugs/1197395 [3] http://lists.freedesktop.org/archives/pulseaudio-discuss/2013-November/019121.html [4] http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html I'm somewhat on the fence, but I think this patch is sensible in the short term at least. I do still think we need some kind of new su which is actually able to properly proxy graphics and sound (like SSH kinda does - at least for graphics), but this should prevent the nasty side effects in the short term. I've not considered any unwanted side effects this may cause so hopefully someone else can chime in accordingly. Your argument about it making it spec compliant seems rather compelling tho'. OK, I just tried this but I can't seem to make it work and prevent the XDG_* vars being set. I applied the attached variation to my 208 build and then ran pkexec /bin/bash which also suffers from the same problems. pkexec cleans out the environment quite well, but then pam_systemd re-injects these variables (I discussed this on IRC the other day with Colin Walters). I would have thought that this should have fixed things. I didn't get any joy from su or su - either (although I'd expect su to still have it set due to not cleaning the environment - is this a correct assumption?). The problem is that the pw_uid in this case is creating the session for my user, not the destination user (note the UID 603): Nov 14 10:25:45 jimmy pkexec[14287]: pam_systemd(polkit-1:session): Asking logind to create session: uid=603 pid=14287 service=polkit-1 type=unspecified class=background seat= vtnr=0 tty= display= remote=no remote_user= remote_host= Nov 14 10:25:45 jimmy pkexec[14287]: pam_systemd(polkit-1:session): Reply from logind: id=2 object_path=/org/freedesktop/login1/session/_32 runtime_path=/run/user/603 session_fd=10 seat=seat0 vtnr=1 Ditto for su: Nov 14 10:27:00 jimmy su[14355]: pam_systemd(su:session): Asking logind to create session: uid=603 pid=14355 service=su type=tty class=user seat=seat0 vtnr=1 tty=pts/2 display= remote=no remote_user=colin remote_host= Nov 14 10:27:00 jimmy su[14355]: pam_systemd(su:session): Reply from logind: id=2 object_path=/org/freedesktop/login1/session/_32 runtime_path=/run/user/603 session_fd=6 seat=seat0 vtnr=1 And su -: Nov 14 10:27:22 jimmy su[14419]: pam_systemd(su-l:session): Asking logind to create session: uid=603 pid=14419 service=su-l type=tty class=user seat=seat0 vtnr=1 tty=pts/2 display= remote=no remote_user=colin remote_host= Nov 14 10:27:22 jimmy su[14419]: pam_systemd(su-l:session): Reply from logind: id=2 object_path=/org/freedesktop/login1/session/_32 runtime_path=/run/user/603 session_fd=6 seat=seat0 vtnr=1 Thus the comparison of the dir owner and the uid succeeds. Am I missing something? Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ From b516dd4ea5187bfa9b5a237a0f50edc53f401468 Mon Sep 17 00:00:00 2001 From: Martin Pitt martinp...@gnome.org Date: Wed, 13 Nov 2013 13:02:28 +0100 Subject: [PATCH] pam: Check $XDG_RUNTIME_DIR owner http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html requires that $XDG_RUNTIME_DIR MUST be owned by the user, and he MUST be the only one having read and write access to it.. Don't set an existing $XDG_RUNTIME_DIR in the PAM module if it isn't owned by the session user. Otherwise su sessions get a runtime dir from a different user which leads to either permission errors or scribbling over the other user's files.
