Bug 45670 - pkexec doesn't invoke PAM session close hooks
Summary: pkexec doesn't invoke PAM session close hooks
Status: NEEDINFO
Alias: None
Product: PolicyKit
Classification: Unclassified
Component: daemon (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: David Zeuthen (not reading bugmail)
QA Contact: David Zeuthen (not reading bugmail)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-06 00:48 UTC by Michael Biebl
Modified: 2013-03-07 18:39 UTC (History)
6 users (show)

See Also:
i915 platform:
i915 features:


Attachments
strace of logind (179.02 KB, text/plain)
2012-02-08 23:21 UTC, Michael Biebl
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Biebl 2012-02-06 00:48:52 UTC
When using latest Git head, the sudo command no longer works. After typing my password I'm always thrown back to the console as the regular user and I can't execute any progams as root.

Doing a git bisect, the first faulty commit points at 9b221b63e5cc62439b32bb487775856a78c6015a
Comment 1 Lennart Poettering 2012-02-06 15:19:02 UTC
I am pretty sure your sudo is just broken, and doesn't waipid() before closing the PAM session. That was fixed in sudo quite some time ago iirc.
Comment 2 Michael Biebl 2012-02-06 15:26:56 UTC
(In reply to comment #1)
> I am pretty sure your sudo is just broken, and doesn't waipid() before closing
> the PAM session. That was fixed in sudo quite some time ago iirc.

just for documentation:
The said fix has landed in sudo 1.7.4 and the Debian package is 1.8.3p2-1, so it looks like a different issue.
Comment 3 Michael Biebl 2012-02-08 16:31:46 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > I am pretty sure your sudo is just broken, and doesn't waipid() before closing
> > the PAM session. That was fixed in sudo quite some time ago iirc.
> 
> just for documentation:
> The said fix has landed in sudo 1.7.4 and the Debian package is 1.8.3p2-1, so
> it looks like a different issue.

I've also compared the sudo sources from rawhide (where Lennart reported it as working) and Debian unstable and didn't find any relevant differences. So I'm inclined to *not* blame it on sudo.

As a workaround, I've removed pam_systemd from sudo's pam session configuration.
Comment 4 Michael Biebl 2012-02-08 23:21:00 UTC
Created attachment 56794 [details]
strace of logind
Comment 5 Michael Biebl 2012-02-08 23:22:17 UTC
<mezcalero> mbiebl: so, the problem goes something like this i think:
<mezcalero> mbiebl: on fedora, when the PAM session is opened, the calling process gets returned that fifo fd that logind uses to check when a session goes away
<mezcalero> mbiebl: then, when the session is destructed this fd is closed and logind knows
<mezcalero> mbiebl: now, when you invoke the PAM session hooks from a process that is already in a logind session
<mezcalero> mbiebl: we will pass it an fd to the same fifo, again
<mezcalero> mbiebl: that means that if you login on a getty, and then do sudo
<mezcalero> mbiebl: you'll have to session fds, to the same fifo
<mezcalero> mbiebl: only if both are closed the kernel will tell logind that the fifo is now closed
<mezcalero> mbiebl: now, if i run things here, then everything works fine like this
<mezcalero> mbiebl: and even though the sudo fd ends up being immediately closed simply because the original fd is still open everything works fine
<mezcalero> now, of course sudo should be fixed not to close the our logind fd in the parent
<mezcalero> but it appears as if your original login process also closes the login fd
Comment 6 Cristian Rodríguez 2012-02-13 13:29:31 UTC
(In reply to comment #5)
> <mezcalero> mbiebl: so, the problem goes something like this i think:
> <mezcalero> mbiebl: on fedora, when the PAM session is opened, the calling process gets returned that fifo fd that logind uses to check when a session goes
> away
> <mezcalero> mbiebl: then, when the session is destructed this fd is closed and logind knows
> <mezcalero> mbiebl: now, when you invoke the PAM session hooks from a process that is already in a logind session
> <mezcalero> mbiebl: we will pass it an fd to the same fifo, again
> <mezcalero> mbiebl: that means that if you login on a getty, and then do sudo
> <mezcalero> mbiebl: you'll have to session fds, to the same fifo
> <mezcalero> mbiebl: only if both are closed the kernel will tell logind that the fifo is now closed
> <mezcalero> mbiebl: now, if i run things here, then everything works fine like this
> <mezcalero> mbiebl: and even though the sudo fd ends up being immediately closed simply because the original fd is still open everything works fine
> <mezcalero> now, of course sudo should be fixed not to close the our logind fd in the parent
> <mezcalero> but it appears as if your original login process also closes the login fd

We can only reproduce this issue with KDM ... using xdm or plain old console makes the issue go away.
Comment 7 Sam Morris 2012-04-05 03:48:13 UTC
This bug is now 100% reproducible once a Debian system is upgraded to systemd 44; logging in via both GDM and login.
Comment 8 Lennart Poettering 2012-04-05 05:18:05 UTC
Sorry, can't do anything about this, since I don't run Debian. This really needs somebody using Debian to debug.
Comment 9 Michael Biebl 2012-04-05 09:20:16 UTC
(In reply to comment #7)
> This bug is now 100% reproducible once a Debian system is upgraded to systemd
> 44; logging in via both GDM and login.

Unfortunately it's not that simple. I'm running an up-to-date Debian sid system with systemd 44-1 and can't reproduce the issue anymore (whereas I could it some time ago).

It's still unclear to me under which circumstances the bug occurs (if it's architecture/kernel version/ etc related)
Comment 10 Michael Biebl 2012-04-05 09:22:45 UTC
(In reply to comment #8)
> Sorry, can't do anything about this, since I don't run Debian. This really
> needs somebody using Debian to debug.

Apparently this is also seen on non-Debian system, as e.g we have reports from openSUSE users.

Frederic suspected a kernel bug, but I dunno if he found out more in that direction.
Comment 11 Frederic Crozat 2012-04-05 09:24:32 UTC
nothing conclusive : I'm unable to reproduce this bug in KVM and some folks still have it (and apparently, even for them, it is intermittent) : https://bugzilla.novell.com/show_bug.cgi?id=746704
Comment 12 Sam Morris 2012-04-05 09:41:48 UTC
(In reply to comment #9)
> (In reply to comment #7)
> > This bug is now 100% reproducible once a Debian system is upgraded to systemd
> > 44; logging in via both GDM and login.
> 
> Unfortunately it's not that simple. I'm running an up-to-date Debian sid system
> with systemd 44-1 and can't reproduce the issue anymore (whereas I could it
> some time ago).
> 
> It's still unclear to me under which circumstances the bug occurs (if it's
> architecture/kernel version/ etc related)

Ah, I see. Both of the systems I hit this are VirtualBox virtual machines, running the Debian 3.2.0-2-amd64 kernel (version 3.2.12-1 at the present moment). For me, sudo stopped working immediately as soon as I upgraded the systemd Debian packages to version 44-1. Admittedly a few other packages were upgraded at the same time, but I think the systemd packages were the only ones on both systems that looked like they could have broken sudo in this way.

As a workaround, removing pam_systemd.so from /etc/pam.d/common-session-noninteractive restores sudo to working order.
Comment 13 Michael Biebl 2012-04-09 13:14:15 UTC
Seems the reason why I couldn't reproduce the problem anymore is that I've added pam_loginuid.so to my pam config locally.

As soon as I comment out those lines again from the login/gdm pam config, I can reproduce the sudo bug. This also explains, why this problem doesn't happen on fedora.
Comment 14 Frederic Crozat 2012-05-03 05:18:43 UTC
I tried to backport commit 75c8e3cffd7da8eede614cf61384957af2c82a29 to v44 and things got worse:
- sudo and su-l (and su) pam config all contains a call to pam_systemd in the session section
- the following test scenario is causing session leader to be terminated by logind:
login as user
su -
logout from root
sudo -s
=> terminate user session completely.
Comment 15 Frederic Crozat 2012-05-04 08:48:04 UTC
and after debugging further, SUSE bug "sudo get broken under systemd" seems to be related with kdm autologin configuration which doesn't include pam_loginuid.so, resulting into a /proc/self/sessionid equal to (uint32_t) -1 which is always the same and is screwing pam_systemd a lot..
Comment 16 Frederic Crozat 2012-05-22 06:15:20 UTC
second possible way to get a broken state, by using screen or tmux:

-login
-start a screen session
-detach the session
-logout
-login again
-re-attach the session
- try to use sudo or su -l in the screen session

for our case, pam_systemd is called in both sudo and su-l pam configuration, and will cause the call to be killed (either immediatly with sudo or when exiting the su -l shell) by pam_systemd, because the audit session id in screen process is no longer valid and screws logind
Comment 17 Frederic Crozat 2012-05-25 05:08:34 UTC
sudo 1.4.5 seems to have fixed the screen / tmux issue, as least on openSUSE.
Comment 18 Fabio Marconi 2012-05-28 11:48:44 UTC
I've got something similar in Ubuntu after changing system language, with sudo and resume from suspend not accepting passwd.
login and tty working.
I fixed it creating ~/.pam_environment with the following content:

LC_NUMERIC=en_US.UTF-8
LC_TIME=en_US.UTF-8
LC_MONETARY=en_US.UTF-8
LC_PAPER=en_US.UTF-8
LC_NAME=en_US.UTF-8
LC_ADDRESS=en_US.UTF-8
LC_TELEPHONE=en_US.UTF-8
LC_MEASUREMENT=en_US.UTF-8
LC_IDENTIFICATION=en_US.UTF-8

for en-us, save-reboot and now it works
Can someone do this fast test ?
Comment 19 Lennart Poettering 2012-05-29 10:43:28 UTC
Fabioy Ubuntu is not using systemd, your bug must be somewhere else.

Michael, can you verify that sudo 1.4.5 fixes the problem on Debian, too?
Comment 20 Michael Biebl 2012-05-29 10:47:12 UTC
(In reply to comment #19)
> Michael, can you verify that sudo 1.4.5 fixes the problem on Debian, too?

I'm using sudo 1.8.3p2. Are you sure you meant 1.4.5?
Comment 21 Lennart Poettering 2012-05-29 10:53:17 UTC
(In reply to comment #20)
> (In reply to comment #19)
> > Michael, can you verify that sudo 1.4.5 fixes the problem on Debian, too?
> 
> I'm using sudo 1.8.3p2. Are you sure you meant 1.4.5?

Hmm, that's what Frederic mentioned in comment #16. Frederic, what is going on?
Comment 22 Frederic Crozat 2012-05-29 13:27:56 UTC
I should learn to type, I was meaning 1.8.5
Comment 23 Lennart Poettering 2012-07-20 14:01:23 UTC
Is this problem still remaining with recent sudo?
Comment 24 Frederic Crozat 2012-07-20 14:27:51 UTC
not with sudo 1.8.5. We have similar issues with pkexec thought..
Comment 25 Lennart Poettering 2012-07-20 22:08:42 UTC
OK, renaming bug accordingly. But my guess is that pkexec also incorrectly calls the PAM hooks, much like sudo initially did.
Comment 26 Michael Biebl 2012-07-21 06:09:02 UTC
(In reply to comment #23)
> Is this problem still remaining with recent sudo?

I'm using sudo 1.8.5p2-1 on Debian now and this version seems to work fine, indeed. Even the old "workaround" to add pam_loginuid  is no longer necessary and sudo works with and without pam_loginuid.

As for pkexec (0.105): I can indeed confirm the behaviour. 
If pam_systemd is enabled, but pam_loginuid not, I simply get a "Hangup" message
With both pam_systemd and pam_loginuid enabled it works fine.
Comment 27 Michael Biebl 2013-01-28 15:55:45 UTC
(In reply to comment #26)
> As for pkexec (0.105): I can indeed confirm the behaviour. 
> If pam_systemd is enabled, but pam_loginuid not, I simply get a "Hangup"
> message
> With both pam_systemd and pam_loginuid enabled it works fine.

Since this came up recently again, let me repeat that: To reproduce the bug, one simply needs to remove pam_loginuid from the PAM stack (for testing purposes I've done it for login, as I can quickly login/logout on the console).
Comment 28 Lennart Poettering 2013-03-05 03:03:29 UTC
So, as it turns out pkexec doesn't invoke the PAM session hooks at all, but it really should. Reassigning to pkexec.
Comment 29 Lennart Poettering 2013-03-05 03:06:47 UTC
Or actually, it does invoke pam_open_session(), but never invokes pam_close_session(), and that's what is broken.

It really needs to invoke pam_open_session() in the parent process before forking off the shell, and then pam_close_session() in the parent process after the shell died.
Comment 30 David Zeuthen (not reading bugmail) 2013-03-06 18:15:09 UTC
(In reply to comment #29)
> Or actually, it does invoke pam_open_session(), but never invokes
> pam_close_session(), and that's what is broken.
> 
> It really needs to invoke pam_open_session() in the parent process before
> forking off the shell, and then pam_close_session() in the parent process
> after the shell died.

Hmm, right now I think we just exec() - this change would include forking a child process and baby-sitting it. Right?

FWIW, I'm not opposed to doing that but I don't have a lot of bandwidth right now to work on it. I'd be happy to review a patch if someone wants to work on this before I get around to it...
Comment 31 Lennart Poettering 2013-03-07 11:14:30 UTC
(In reply to comment #30)
> (In reply to comment #29)
> > Or actually, it does invoke pam_open_session(), but never invokes
> > pam_close_session(), and that's what is broken.
> > 
> > It really needs to invoke pam_open_session() in the parent process before
> > forking off the shell, and then pam_close_session() in the parent process
> > after the shell died.
> 
> Hmm, right now I think we just exec() - this change would include forking a
> child process and baby-sitting it. Right?

Yeah, right now the code apparently just does pam_open_session() and then exec(). What it should do is pam_open_session(), followed by exec() in the child, and waitpid() in the parent. Afterwards the parent should invoke pam_close_session() and exit.
Comment 32 Lennart Poettering 2013-03-07 11:16:47 UTC
(In reply to comment #31)
> (In reply to comment #30)
> > (In reply to comment #29)
> > > Or actually, it does invoke pam_open_session(), but never invokes
> > > pam_close_session(), and that's what is broken.
> > > 
> > > It really needs to invoke pam_open_session() in the parent process before
> > > forking off the shell, and then pam_close_session() in the parent process
> > > after the shell died.
> > 
> > Hmm, right now I think we just exec() - this change would include forking a
> > child process and baby-sitting it. Right?
> 
> Yeah, right now the code apparently just does pam_open_session() and then
> exec(). What it should do is pam_open_session(), followed by exec() in the
> child, and waitpid() in the parent. Afterwards the parent should invoke
> pam_close_session() and exit.

Sorry, let's try this again:

Yeah, right now the code apparently just does pam_open_session() and then exec(). What it should do is pam_open_session(), followed by fork(), then exec() in the child, and waitpid() in the parent. Afterwards the parent should invoke pam_close_session() and exit.
Comment 33 David Zeuthen (not reading bugmail) 2013-03-07 18:03:31 UTC
(In reply to comment #32)
> Yeah, right now the code apparently just does pam_open_session() and then
> exec(). What it should do is pam_open_session(), followed by fork(), then
> exec() in the child, and waitpid() in the parent. Afterwards the parent
> should invoke pam_close_session() and exit.

Sounds good to me. This shouldn't be a very big change, I'll try to look into this in the weekend. Thanks.

Btw, is this what su(8) and sudo(8) is doing? I'm also idly wondering if this extra process in the process tree is going to confuse other software (say, the desktop shell, gnome-system-monitor and other top(1)-ish software walking the process tree). If so, my view is that it's probably their bug, yea?
Comment 34 Cristian Rodríguez 2013-03-07 18:39:38 UTC
(In reply to comment #33)

> Btw, is this what su(8) and sudo(8) is doing? 

Yes, this is what any software using pam sessions _must_ do.


> If so, my view is that it's probably their bug,
> yea?

Yes. if there is any tool that for some reason misbehave after pkexec makes correct use of the PAM API then it is bug somewhere else. ;-)


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.