Comment 102 for bug 1197395

Revision history for this message
In , Adam (adam-redhat-bugs) wrote :

(In reply to comment #29)
> I explicitly chose to bind this to loginuid rather than anything else.
> People shouldn't fool themselves in assuming that "su" would get them a
> completely new session that has all properties of the destination user
> rather than the source user. THis is technically not possible, and if it
> would be implemented would even kinda defeat the point of su.
>
> For example, people probably want DISPLAY= to be the old one so that their
> root program they started with su an run on X11. OTOH the home directory
> should probably be the one of the destination user afterwards, but the
> current directory the one of the source user. The audit trail should be the
> one of the old user, but $PATH of the new user. And so on and so on.
>
> So people want a weird mix of things from the old and the new user session,
> and even worse, people want different things from the old and from the new
> session. For example, PA now places its socket in XDG_RUNTIME_DIR, and so
> does dconf. But apparently this bug report is a complaint that dconf should
> be the one of the new user. THis would break audio of however, which
> probably should be the one of the original user.
>
> So, we should not pretend we could make "su" work properly for the general
> case, or that we could find a way that makes this work for everybody.
>
> When we decided to base our systemd XDG_RUNTIME_DIR logic on the audit login
> session id (if it is available) is the thinking that "su" should be minimal
> in its effect, and almost everything that is not explicitly detached from
> the old session should continue to be from the old session. i.e. the default
> should be not to detach rather than to detach.
>
> If you want a fully detached text session then use "ssh localhost" or so,
> which virtualizes everything.
>
> So really, I see little to fix here in systemd. I believe basing this on
> loginuid is the right thing. And if the trade-off is between breaking PA and
> breaking dconf my choice is to stick to what makes sense on logical level.

This statement is definitely right for "su". However for "su -" case I would expect that XDG_* directories are created with uid of destination user. To accomplish this I think two changes need to be done:

1. Change "session" part of /etc/pam.d/su (i.e. "su" variant) to

session optional pam_keyinit.so revoke
session required pam_limits.so
session required pam_unix.so

The main reason behind this is that "su" should just borrow uid and all environment variables (with some exceptions - $HOME, $SHELL, $USER, $LOGNAME, $PATH, and $IFS, not sure if others) should be preserved. This is also true for XDG_ variables. Another reason for this change is to avoid calling pam_systemd for non-login shell.

2. Change pam_systemd to create & set XDG_ directories with UID of target user, not of the loginuid user.

After those two changes, "su" will just borrow uid of new user but "su -" will create new session, including XDG_ directories.

Note there might be needed even more sophisticated pam.d/* change to keep "runuser", which is just variant of "su", in sync with "su" so /etc/pam.d/su might look like

...
session include runuser
...

and all changes will go into /etc/pam.d/runuser. Ditto for /etc/pam.d/su-l and /etc/pam.d/runuser-l.

Also please note there might be needed separate patch for dconf to fall back to ~/.cache when XDG_RUNTIME_DIR is not accessible, which should happen in "su" case.

What do you think about this proposal?