[systemd-devel] Fix PAM module to not clobber XDG_RUNTIME_DIR with su

[Colin Guthrie]

'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/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0510-pam-Check-XDG_RUNTIME_DIR-owner.patch
Type: text/x-patch
Size: 2592 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20131114/76bd97da/attachment.bin>