Bug 789 - pam_setcred() not being called as root
Summary: pam_setcred() not being called as root
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: PAM support (show other bugs)
Version: 3.7.1p2
Hardware: ix86 Linux
: P2 normal
Assignee: OpenSSH Bugzilla mailing list
URL:
Keywords: patch
Depends on:
Blocks: 793
  Show dependency treegraph
 
Reported: 2004-01-15 10:02 AEDT by Egmont Koblinger
Modified: 2004-04-14 12:24 AEST (History)
0 users

See Also:


Attachments
this patch works for me (722 bytes, patch)
2004-01-15 23:29 AEDT, Egmont Koblinger
no flags Details | Diff
Only call pam_setcred a 2nd time when privsep is off (513 bytes, patch)
2004-01-29 21:20 AEDT, Darren Tucker
djm: ok+
Details | Diff
Only make setcred call for !use_privsep (non-interactive path). (514 bytes, patch)
2004-02-13 13:34 AEDT, Darren Tucker
no flags Details | Diff
Only make setcred call for !use_privsep (non-interactive path, corrected) (515 bytes, patch)
2004-02-13 13:41 AEDT, Darren Tucker
djm: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Egmont Koblinger 2004-01-15 10:02:38 AEDT
In openssh-3.7.1p2/auth-pam.c, line 589, where pam_setcred() is called, both
real and effective user ID's are already switched to normal user.
However, they should be root here.

This causes a problem when trying to use pam_group.so module. This module is
supposed to grant membership to some additional groups, however, as it fails
to do so, it reports an error to sshd and hence sshd refuses the login.

/etc/pam.d/ssh is a symlink to system-auth which is used by many utilities on
my system, none of them has problem with pam_group except openssh. If I remove
the "auth required /lib/security/pam_group.so" line then sshd works as I expect.
A debug printf put into pam_group also clearly shows that unlike openssh, other
system utilities (at least login, gdm, kdm) have uid=euid=0 here.

OpenSSH 3.7.1p2, Linux-PAM 0.77, quite up-to-date system in other respects
(e.g. gcc 3.3.2, glibc 2.3.2, Linux kernel 2.4.24).
And, of course, sshd_conf contains "UsePAM yes".
Comment 1 Darren Tucker 2004-01-15 21:58:02 AEDT
Try commenting out the second pam_setcred call (in session.c:do_exec_pty()) and
see if that makes the difference (and also check that the groups are in fact
correct).

Adding some debugging gives me these entries for pam_setcred:

debug1: do_pam_setcred called from do_setusercontext
debug3: do_pam_setcred called init 0 uid 0 euid 0 pid 5435
debug1: PAM: reinitializing credentials
debug1: do_pam_setcred called from do_exec_pty
debug3: do_pam_setcred called init 1 uid 500 euid 500 pid 5435

I'm wondering why the second call is there at all?
Comment 2 Egmont Koblinger 2004-01-15 23:28:23 AEDT
Well, in the mean time I analyzed and debugged the source, and I also found
that pam_setcred() is being called twice. I can't really see its purpose either
and probably it might have strange side effects (e.g. I guess group membership
would be granted twice if it worked -- I didn't test it, it's just a guess.)

Furthermore it's strange that the first time (when it is called as root)
"reinitializing credentials" is logged and the second time (when it's called
as user) log says "establishing credentials", it seems to me that the order is
somehow swapped.

Before reading your comment here I created a very similar patch (attached here),
it works for me, groups are okay (I'm granted membership to my standard groups
(listed in /etc/group, initialized by openssh with an initgroups() call) as
well as to the additional (terminal-dependant, time-dependant etc.) groups
that are listed in /etc/security/group.conf (initialized by pam_group).
Comment 3 Egmont Koblinger 2004-01-15 23:29:23 AEDT
Created attachment 530 [details]
this patch works for me
Comment 4 Darren Tucker 2004-01-15 23:40:53 AEDT
"Reinitialize" is misleading.  According to the Linux PAM man page, it means
"delete then initialize", and I'm guessing it's for dropping any creds the
parent process might have had.

As for the second call, since it comes directly after do_pam_set_tty() I'm
guessing it's for adding credentials associated with a specific tty (somewhat
like the tty ticket system employed by sudo, although I don't know of any PAM
examples).  That's just a guess, though.  I've been back 3 years in the CVS logs
and I'm still none the wiser. 
Comment 5 Darren Tucker 2004-01-15 23:48:50 AEDT
Hmm, since your patch works and you get the right creds, doesn't that mean you
had the right ones when the module was called the second time, it doesn't need
to do anything and is failing for no good reason?
Comment 6 Egmont Koblinger 2004-01-16 00:29:37 AEDT
No. Some debug stuff put into pam_group shows that it calls setgroups()
(actually I'm already member of the groups it wants to add, since it's
called for the second time, but this is not a problem), but being non-root
setgroups() returns -1 EPERM and this causes pam_group to return PAM_CRED_ERR.
I think it's okay for pam_group to return an error in this situation.
Comment 7 Darren Tucker 2004-01-16 00:42:28 AEDT
My point is why is it calling setgroups at all?  If you're already a member of
those groups (which you apparently are) it's not necessary.
Comment 8 Egmont Koblinger 2004-01-16 00:56:01 AEDT
If you want to assign 5 to the variable x, you write "x=5" instead of
"if (x!=5) x=5".
If you want to remove a file, you write "rm file" and let it perhaps return an
error, instead of writing "if this file is removable; then rm file; fi".

My point is that you cannot expect every pam module to first check whether the
thing it would like to do would have actually any effect, and then only do it
if it would. I perfectly agree with this pam module: if it wants you to add
group memberships, then it adds it. If it fails to do so, returns an error.
That's it. And I guess pam_group is just one module that happened to trigger
this behavior for me, most likely there are many other pam modules like this.

AFAIK pam is designed to run as root. Anything you want to do as normal user
should go out of pam into some .profile or similar. So calling a pam function
as non-root is IMHO definitely an openssh bug, PAM shouldn't expect and
workaround this situation.
Comment 9 Darren Tucker 2004-01-16 12:53:54 AEDT
Short answer: Set UsePrivilegeSeparation=no or change the PAM module.

Long answer:
I can't find any reference to PAM modules being guaranteed to run as root in
either the Open Group PAM RFC [1] or the Linux PAM documentation [2], so an
alternative viewpoint could be that pam_group is making unwarranted assumptions
about its environment, doing unnecessary things and failing because of it :-)

Assume for a moment that the 2nd call to pam_setcred is required in some
configurations and that it needs the tty set (I'm guessing Kerberos, but the
line predates my involvement in the project, so perhaps someone could help me
out here?).  The ssh2 protocol allows the client to request a pty at any time,
and when privsep is on, the main daemon is already running as the user at that
point, as you found.  Changing this means disabling privsep, for which there's a
run-time option.

Also note that due to the wide variety of PAM implementations and modules out
there, non-trivial changes to fix breakage on one platform/configuration usually
results in breakage on another.

I took a look at the pam_group code, and changing it seems to be a 3-line patch:
http://www.zip.com.au/~dtucker/patches/linux-pam-0.77-pam_group_noreset.patch

[1] http://www.opengroup.org/tech/rfc/mirror-rfc/rfc86.0.txt
[2] http://www.kernel.org/pub/linux/libs/pam/Linux-PAM-html/
Comment 10 Damien Miller 2004-01-16 13:08:51 AEDT
IIRC the second call to pam_setcred was to reinitialise supplemental groups
after our initgroups call. Given our privsep'd nature, I'm not sure it makes
sense any more, at lease for UsePrivilegeSeparation=yes

Perhaps the pam_setcred call should happen around the time the unpriv child is
forked and its credentials established? but who knows what else that would break
- the PAM stuff is so poorly specified and thus fragile. (That's why I recommend
it only as a last resort.)

What is wrong with simply handing groups via /etc/groups?
Comment 11 Darren Tucker 2004-01-16 13:24:24 AEDT
Reset-after-initgroups is the first call (in do_setusercontext), and it already
runs as root.

I can imagine there could be credentials that would not require root to obtain
but could need the tty (I'm thinking Kerberos tickets, but I don't know it that
well and could be wrong).

We could make that call to pam_setcred non-fatal if not called as root, or not
call it at all if privsep is on?
Comment 12 Darren Tucker 2004-01-29 21:20:04 AEDT
Created attachment 537 [details]
Only call pam_setcred a 2nd time when privsep is off

Perhaps we should do something like this?  It means that any modules that *do*
rely on the TTY can still run with privsep off.
Comment 13 Damien Miller 2004-02-06 09:57:05 AEDT
Comment on attachment 537 [details]
Only call pam_setcred a 2nd time when privsep is off

ok
Comment 14 Darren Tucker 2004-02-06 15:32:04 AEDT
Patch id 537 was just committed.  Thanks for the report.

 - (dtucker) [session.c] Bug #789: Do not call do_pam_setcred as a non-root
   user, since some modules might fail due to lack of privilege.  ok djm@
Comment 15 Egmont Koblinger 2004-02-13 09:22:18 AEDT
Your "official" committed patch works okay for interactive logins, however,
still runs into the same problem if I try to scp a file to the server.

My "this patch works for me" works in both cases.

Hence I'm reopening the bug.
Comment 16 Darren Tucker 2004-02-13 13:34:50 AEDT
Created attachment 546 [details]
Only make setcred call for !use_privsep (non-interactive path).

Looks like the same change is needed for the non-interactive path.

Since there is no PAM_TTY on this path, perhaps the entire #ifdef USE_PAM block
should be removed?
Comment 17 Darren Tucker 2004-02-13 13:41:15 AEDT
Created attachment 547 [details]
Only make setcred call for !use_privsep (non-interactive path, corrected)
Comment 18 Damien Miller 2004-02-23 15:37:59 AEDT
Comment on attachment 547 [details]
Only make setcred call for !use_privsep (non-interactive path, corrected)

OK - assuming that it works
Comment 19 Darren Tucker 2004-02-24 00:03:09 AEDT
Patch #547 committed (it tested OK with pam_group on my RH9 box).
Comment 20 Damien Miller 2004-04-14 12:24:20 AEST
Mass change of RESOLVED bugs to CLOSED