Reported by Dr. Carsten Benecke: "I guess that the forked child process that calls the sshpam_cleanup() function is forked before the parent calls do_pam_session() (which sets sshpam_session_open to true)." and Chris Jensen: "But when I exit the session, pam_sm_close_session gets called, but it only runs as the user that was logged in, so it doesn't have permission to unmount the directory." The PAM session modules are called in the forked child but the cleanup should be done as root in the parent. Will attach a patch shortly.
Created attachment 709 [details] Set session_open flag in parent
Comment on attachment 709 [details] Set session_open flag in parent Hmm, that doesn't appear to fix it for the privsep case. The privs are permanantly revoked in the parent too, so the cleanup needs to be in the *monitor*.
I'm not going to be able to look at this one until after 4.0.
I've been thinking about this. It's too late for 4.1p1, but I think the right way to fix this is to split up the do_pam_setcred() and do_pam_session() calls, and hook the do_pam_session calls into the login/logout recording in loginrec.c (to be called from the monitor). The existing loginmsg handling would allow any messages returned by PAM to be sent to the user. This would allow per-session login recording and would allow the pam_session_close to be called from the same process as the pam_session_open. It would mean that pam_session_open() would not be called be pam_setcred (since in privsep we switch to the real user very early). I'm not sure if that's going to be a problem or not. Comments (esp. from PAM folks) welcome.
I'm not sure if I understand your proposal well so I only make some remarks how from the PAM point of view it should be done. The pam_setcred(PAM_ESTABLISH_CRED) should be called before pam_open_session and it shouldn't be necessary to call it with PAM_REINITIALIZE_CRED after that, however it does no harm. What is important is to run pam_open_session as root and in the same process (or before forking the child) where will be the user shell executed. The pam_close_session call should be done in the same process. If your proposal stays within these limits it should be fine.
(In reply to comment #5) > I'm not sure if I understand your proposal well so I only make some remarks > how from the PAM point of view it should be done. Ah, but which PAM's view? :-) > The pam_setcred(PAM_ESTABLISH_CRED) should be called before pam_open_session I know that's what the LinuxPAM man page says, but the Sun PAM man page says the opposite: The pam_setcred() function is used to establish, modify, or delete user credentials. It is typically called after the user has been authenticated and after a session has been opened. See pam_authenticate(3PAM), pam_acct_mgmt(3PAM), and pam_open_session(3PAM). FWIW I think the Sun approach is saner since it means that pam_setcred can drop privs (or chroot, or whatever) and pam_open_session still has privs to write whatever records it needs to. [...] > The pam_close_session call should be done in the same process. The open+close in the same process is doable, however setcred in the same process (or the parent thereof) as the session is a problem. In the privsep case, what I'm proposing is roughly this: (monitor) forks slave (slave) setcred(ESTABLISH_CRED), setgid, setuid (slave) requests pty from master (monitor) allocates pty, calls pam_open_session (slave) forks shell process (shell) does stuff (shell) exits (slave) requests session close from master (monitor) calls pam_close_session [connection closes] (slave) setcred(DELETE_CRED) (slave) exits (monitor) exits So, the session open/close would happen purely in the monitor, and the establish/delete creds happen purely in the slave. It basically boils down to this: does pam_{open,close}_session represent an interactive (ie shell) session or an authentication session? I suspect the answer is "depends on the module" (which is one of the fundamental problem with PAM...).
I speak about the Linux PAM. This cannot be right in Linux PAM - you must do pam_open_session before forking the slave process. Otherwise for example the pam_limits module will completely fail to set the limits. Also about the pam_setcred call - some modules may need having credentials before opening a session - for example for mounting home directory.
I don't understand - surely the limits should be applied in the *child* process and not the parent process?
I am talking about your proposition in comment #6. It doesn't make sense because te pam_open_session + pam_close_session must be called by a parent process of the child from which the shell is executed but before forking the child so the child inherits limits and other things set by the session module. The reason why the pam_open_session cannot be called in slave is that the pam_close_session must be called in the same process as where the pam_open_session were. It also should be called with root privileges.
Created attachment 1143 [details] Call pam_session_open and pam_session_close in monitor when privsep=yes With the above details in mind, I think this patch or similar is about as close as we can get. I suspect this may break pam_chroot when privsep=yes, though (since the monitor will also be chrooted). I would like to think that we can rationalize the PAM calls in session.c, but touching them usually results in problems, so it would need to be done very carefully.
On the first look the current patch seems good, however it doesn't seem to solve the problem when privsep is disabled. Or am I wrong?
NO(In reply to comment #11) > On the first look the current patch seems good, however it doesn't seem > to solve the problem when privsep is disabled. Or am I wrong? No, you're right. It doesn't solve the issue when privsep=no. Fixing that means changing the way sshd works with PAM without privsep. We can do something similar to patch #1143 for that path, but that means moving the pam_setcred can pam_session_open calls to before the fork. This in turn means there will be no pty, and modules won't be able to interact with the user via the tty like they do now. I can't see a way to make both work simultaneously.
That wouldn't be a good idea, because even the session modules should be able to communicate with the user. I think the most correct but a little bit bloated approach would be to do another fork (in privsep mode it would be in slave before dropping privileges) which would be there regardless of privsep setting. That's how login, gdm and other such programs work.
(In reply to comment #13) > That wouldn't be a good idea, because even the session modules should > be able to communicate with the user. They can communicate but it's one-way only (through Buffer loginmsg). This is how it works with privsep=yes (but folks have the option of setting privsep=no if they need this functionality, which is one reason I didn't change it in the patch). > I think the most correct but a little bit bloated approach would be to > do another fork (in privsep mode it would be in slave before dropping > privileges) which would be there regardless of privsep setting. That's > how login, gdm and other such programs work. I was wondering what login did. That means that the pam_session_close gets called by a different pid to the pam_session_open right? (although a direct descendant of it).
*** Bug 1189 has been marked as a duplicate of this bug. ***
Hi Darren, as the pam code has changed a lot, I wonder if you could please summarize the stat of the art? Which process will call which pam function and how is this related to the privsep settings? Regards, CB
(In reply to comment #16) > as the pam code has changed a lot, I wonder if you could please > summarize the stat of the art? Changes since when? We've been trying for incremental improvements for a while... > Which process will call which pam function and how is this related to > the privsep settings? This patch is not in the main code yet, but with it applied and privsep=yes, the monitor will call both pam_session_open() and pam_session_close() and will, I believe, solve the problem you reported. With the patch applied and privsep=no, pam_session_open() will be called by the process that later exec's the user's shell (I'm not sure pam_session_close() is called at all because the "session open" flag isn't set in the parent... that's one of the next things to look at.) Confirmation of whether or not it actually fixes your problem would be appreciated.
(In reply to comment #17) > With the patch applied and privsep=no, pam_session_open() will be > called by the process that later exec's the user's shell (I'm not sure > pam_session_close() is called at all because the "session open" flag > isn't set in the parent... that's one of the next things to look at.) > > Confirmation of whether or not it actually fixes your problem would be > appreciated. > I have just applied the patch to my Solaris 8 systems where the symptom was that a root user (and only a root user) session would hang at logout. The patch solved this problem and has no immediate other side effects.
I've been bitten by this bug as well. (Want to do custom session teardown as root. Currently, only "UseLogin yes" gets me there, but of course that costs me X11 forwarding.) I've applied patch 1143 to an openssh_cvs tree and tested on a Debian/unstable Linux system. My observations: 1. pam_sm_{open,close}_session() are correctly invoked as root. (uid == gid == euid == egid == 0 for both) 2. Messages written to stdout/stderr in pam_sm_{open,close}_session() are not visible to the user logging in or out. (I don't know if this is by PAM's design or not.) 3. When running "sshd -de", messages written to stderr in pam_sm_{open,close}_session() are visible in the server's stderr log. Messages to stdout go nowhere that I can find.
(In reply to comment #19) > I've been bitten by this bug as well. (Want to do custom session > teardown as root. Currently, only "UseLogin yes" gets me there, but of > course that costs me X11 forwarding.) > > I've applied patch 1143 to an openssh_cvs tree and tested on a > Debian/unstable Linux system. My observations: Firstly, thanks for testing. > 1. pam_sm_{open,close}_session() are correctly invoked as root. Cool. > 2. Messages written to stdout/stderr in pam_sm_{open,close}_session() > are not visible to the user logging in or out. (I don't know if this > is by PAM's design or not.) PAM modules should not be writing to stdout or stderr. If they have something to say they should call the conversation function. (There's a simple example here: http://www.zip.com.au/~dtucker/patches/pam_echo.c, which is based on pam_echo from OpenPAM.)
The patch causes a regression with pam_krb5 module. See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=201341 As I said above I think that the only correct solution which would solve all cases (privsep yes/no, root/regular user) would be to add another fork before the setuid calls and shell process exec. login does this: 1. call pam_open_session 2. fork 3. parent waits for child, child impersonates user, execs shell 4. when child exits, parent calls pam_close_session
(In reply to comment #21) > The patch causes a regression with pam_krb5 module. > See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=201341 Thanks for giving it a spin in Fedora. Does this particular problem also occur with PrivSep=no? > As I said above I think that the only correct solution which would > solve all cases (privsep yes/no, root/regular user) would be to add > another fork before the setuid calls and shell process exec. Would there be any downside to setting KRB5CCNAME in the parent too? (since it causes a regression, I'm taking this bug out of the list for 4.4 pending further work.)
(In reply to comment #22) > (In reply to comment #21) > > The patch causes a regression with pam_krb5 module. > > See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=201341 > > Thanks for giving it a spin in Fedora. Does this particular problem > also occur with PrivSep=no? I don't think that this occurs with privsep disabled. > > As I said above I think that the only correct solution which would > > solve all cases (privsep yes/no, root/regular user) would be to add > > another fork before the setuid calls and shell process exec. > > Would there be any downside to setting KRB5CCNAME in the parent too? I don't know of any however note that with privsep disabled or when called as root the pam_session_close still won't be called correctly.
Created attachment 1216 [details] Improved pam-session patch I have moved the pam session calls to the sshd.c before the monitor forks the child. I also removed various pam calls from session.c because they would be duplicate to the calls in sshd.c. I tested this here in various root/non-root privsep/no-privsep combinations and it seems to work fine, pam_ calls are done as uid 0 and they are always called.
I just tested the new patch on Solaris 8 built with PAM and it works great - with or without PrivSep, as root or not. Any chance of getting this bug added to the list of bugs to be fixed for 4.6 (bug ID 1274)?
I neglected to run the regression tests before, which I have now done and which passed with no errors. So, any chance for 4.6? I will be happy to provide any additional details or run tests to assist this along. (In reply to comment #25) > I just tested the new patch on Solaris 8 built with PAM and it works > great - with or without PrivSep, as root or not. > > Any chance of getting this bug added to the list of bugs to be fixed > for 4.6 (bug ID 1274)? >
Patch #1216 looks reasonable, targeting 4.8. My concern is that the PAM coverage is a bit like a too-small blanket, and pulling it one way to cover a particular case uncovers another case that was previously working.
The patch is now included in Fedora and RHEL5 for some time without any bug reports caused by it so I think it should be safe to include it. But of course you're right that there is a potential for regressions especially when some configurations were adjusted for the current (wrong) implementation.
(In reply to comment #28) > The patch is now included in Fedora and RHEL5 for some time without any > bug reports caused by it so I think it should be safe to include it. That certainly helps, but the caveat is that LinuxPAM is one of three families of PAM implementations (that I know of), all of which have their various quirks (and also differences between platforms that nominally use the same code base).
I've compiled 4.7p1 and SNAP-20080122 and observed that the PAM session was not closed when I exited the session. I was looking for a solution and found this bug report. I've applied the patch provided above and after that the PAM session was closed correctly. The system was openSUSE 10.3 with PAM 0.99.8.1-15. UsePrivilegeSeparation was yes. User was root. I would like to know whether the inclusion of this patch is being considered or not. If yes, when will it be released? Thank you very much.
Created attachment 1472 [details] PAM session in monitor. Update patch to -current.
Patch has been applied, thanks. It is currently planned to be in 4.8. I'm leaving this bug open for the time being because of the history of collateral damage caused by PAM changes.
To comment #20: Modules do not seem to be able to do converse (in 5.0p1). pam_mount for example is affected by this (ideally it would just grab the authtoken from the auth stage but sadly enough openssh destroys the pam context and instead starts a new one for session stage).
(In reply to comment #33) > To comment #20: > Modules do not seem to be able to do converse (in 5.0p1). pam_mount for > example is affected by this (ideally it would just grab the authtoken > from the auth stage but sadly enough openssh destroys the pam context > and instead starts a new one for session stage). That's a separate issue (see bug #688), however I think it only applies for challenge-response type authentications. You can probably work around it by disabling ChallengeResponseAuthentication in sshd_config or using password authentication (as opposed to keyboard-interactive, with an OpenSSH client that's "ssh -o preferredauthentications=password server").
(from bug #926) >> Modules do not seem to be able to do converse (in 5.0p1). pam_mount for >> example is affected by this (ideally it would just grab the authtoken >> from the auth stage but sadly enough openssh destroys the pam context >> and instead starts a new one for session stage). > >That's a separate issue (see bug #688), however I think it only applies >for challenge-response type authentications. > >You can probably work around it by disabling >ChallengeResponseAuthentication in sshd_config or using password >authentication (as opposed to keyboard-interactive, with an OpenSSH >client that's "ssh -o preferredauthentications=password server"). Unfortunately, that does not do it either. CRA is set to no, PasswordAuthentication set to yes, no pubkey in ~/.ssh. According to sshd -ddd, it's still conversation error. pam_mount(pam_mount.c:518) error trying to retrieve authtok from auth code pam_mount(pam_mount.c:208) enter read_password debug3: PAM: sshpam_store_conv called with 1 messages pam_mount(pam_mount.c:176) conv->conv(...): Conversation error
pam_mount tries to ask a password from the user*, which puts it in challenge-response case of needing more than one interaction. Since session modules can't interact with the user other than to display messages, you'd also need to put it in the PAM accounting stack. This is what bug #688 is about, and isn't related to this one (pam_session_close behaviour). IMO this bug can be closed with the release of openssh-4.8p1. Darren, do you agree? Isn't this also incorrectly marked as blocking 5.1? * See http://www.google.com/codesearch?hl=en&q=show:9Q9GBXVnR7k:chgFMim3giQ:glu2lh4-EfU&sa=N&ct=rd&cs_p=http://open-systems.ufl.edu/mirrors/gentoo/distfiles/pam_mount-0.18.tar.bz2&cs_f=pam_mount-0.18/src/pam_mount.c&start=1#l195
(In reply to comment #36) > pam_mount tries to ask a password from the user*, which puts it in > challenge-response case of needing more than one interaction. Since > session modules can't interact with the user other than to display > messages, you'd also need to put it in the PAM accounting stack. This > is what bug #688 is about, and isn't related to this one > (pam_session_close behaviour). > > IMO this bug can be closed with the release of openssh-4.8p1. Darren, > do you agree? Isn't this also incorrectly marked as blocking 5.1? The thing is it (pam_mount) probably used to work with at least privsep=no, because the session wasn't opened until the pty had been allocated, thus the modules could interact using the tty conversation function. So, this is a regression (I what I was worried about in comment #27). Now that the session is opened in the monitor, the session modules can't interact with the user. On the flip side, the session close now runs with privilege. So, take the bug out of the 5.1 list, but I wouldn't close it yet.