[systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]

[Dr. Werner Fink]

On Tue, Nov 26, 2013 at 02:39:49PM +0000, Colin Guthrie wrote:
> 'Twas brillig, and Dr. Werner Fink at 26/11/13 14:21 did gyre and gimble:
> > On Tue, Nov 26, 2013 at 10:41:36AM +0000, Colin Guthrie wrote:
> >> 'Twas brillig, and Martin Pitt at 26/11/13 06:19 did gyre and gimble:
> >>> Hey Lennart,
> >>>
> >>> Lennart Poettering [2013-11-26  5:12 +0100]:
> >>>> I implemented this now, using a different approach than Martin's
> >>>> original patch (i.e. I don't think it is a good idea to involve stat()
> >>>> here, instead let's just let logind pass all information to
> >>>> pam_systemd).
> >>>
> >>> Thanks!
> >>
> >> Indeed, thanks for this!
> >>
> >> If anyone backports this fix to v208 (i.e. pre sd-bus) please share it
> >> here. I'll likely do it just to have the "upstream-blessed" fix, but
> >> doubt I'll get around it it until later in the week.
> > 
> > I've backported it.
> 
> Can you link to it or attach it please?

Yep ...

> > But during tests I've found that it does not help
> > if the environment variable XDG_RUNTIME_DIR already exists before doing
> > su.  It will not unset but exported.
> 
> That's expected.
> 
> su does not do any env cleaning, su - does, sudo does, pkexec does.

I'm aware ;) neverthelesss ...

> 
> su's behaviour is to always not touch stuff and thus this is known and
> expected and has always been a problem.
> 
> Longer term we need to solve this more holistically (hence why I've
> tried to get information about "how things should be done" in the future
> to start helping lay the ground work for thise bits), but this is still
> the best fix we have just now.
> 
> For the specific case of su'ing to root, (which is the most common and
> potentially problematic one), I will probably use Colin W's most recent
> patch to have a static root runtime dir and for logind to set it. This
> should fix XDG_RUNTIME_DIR when su'ing to root. I'm not so worried about
> su'ing to other users (the damage that can be done is much more
> limited), but longer term we do need to address that nicely too IMO
> (which will likely need changes in su itself and a number of other places)

... I've a bug report here which indeed shows exactly the ``other'' problem.
If you do not like the enhancement in my backport you may replace

+        } else {
+                (void) unsetenv("XDG_RUNTIME_DIR");
+                r = pam_putenv(handle, "XDG_RUNTIME_DIR");
+                if (r != PAM_SUCCESS && r != PAM_BAD_ITEM) {
+                         pam_syslog(handle, LOG_ERR, "Failed to unset runtime dir.");
+                }

with `}'  ... otherwise both the pam_putenv() and unsetenv() are required
to make sure that XDG_RUNTIME_DIR is removed.

Werner

-- 
  "Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool." -- Edward Burr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1012-pam_systemd_do_override_XDG_RUNTIME_DIR_of_the_original_user.patch
Type: text/x-patch
Size: 7707 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20131126/25058f3f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20131126/25058f3f/attachment.pgp>