Menu

#88 initgroups call forgets earlier groups

pam_mount
closed-fixed
pam_mount (94)
5
2011-01-04
2010-12-17
No

Original report: SF #2906631 http://sf.net/support/tracker.php?aid=2906631
Affects: pam_mount >= 2.2, <= 2.7

pam_group (cred stage) may have set certain groups, and the initgroups call in pam_mount (session stage) renders this ineffective. Use a get-set-group call pair instead in pam_mount.

Discussion

  • Italo Valcy

    Italo Valcy - 2010-12-18

    Hi jengelh,

    Bellow is a patch that fix this issue. I didn't find a better way, so if you have any better idea, please let me know.

    I tested it with local groups (/etc/group), pam_group and both. It works fine in all situation.

    I hope this can help.

    ----------------------8<--------------------------
    From 480a012bbf226a673bec85b2396f4a8da8cd3d97 Mon Sep 17 00:00:00 2001
    From: Italo Valcy <italo@dcc.ufba.br>
    Date: Fri, 17 Dec 2010 13:13:44 -0300
    Subject: [PATCH] fix the load of groups in src/spawn.c

    - The use of initgroups() in src/spawn.c could make problems when use
    pam_groups.so early. Now it uses getgrouplist() and get/setgroup()

    Reference: http://sf.net/support/tracker.php?aid=3139147
    ---
    configure.ac | 2 +-
    src/spawn.c | 21 +++++++++++++++++++--
    2 files changed, 20 insertions(+), 3 deletions(-)

    diff --git a/configure.ac b/configure.ac
    index 0d921df..644cc37 100644
    --- a/configure.ac
    +++ b/configure.ac
    @@ -53,7 +53,7 @@ AC_CHECK_HEADERS([linux/fs.h linux/major.h dev/cgdvar.h dev/vndvar.h])
    AC_CHECK_HEADERS([sys/mdioctl.h sys/mount.h sys/statvfs.h])
    AC_CHECK_MEMBERS([struct loop_info64.lo_file_name], [], [],
    [#include <linux/loop.h>])
    -AC_CHECK_FUNCS([getmntent getmntinfo initgroups])
    +AC_CHECK_FUNCS([getmntent getmntinfo getgrouplist getgroups setgroups])
    AM_CONDITIONAL([HAVE_CGD], [test "x$ac_cv_header_dev_cgdvar_h" = "xyes"])
    AM_CONDITIONAL([HAVE_MDIO], [test "x$ac_cv_header_sys_mdioctl_h" = "xyes"])
    AM_CONDITIONAL([HAVE_VND], [test "x$ac_cv_header_dev_vndvar_h" = "xyes"])
    diff --git a/src/spawn.c b/src/spawn.c
    index 7c6ba1a..e008988 100644
    --- a/src/spawn.c
    +++ b/src/spawn.c
    @@ -110,8 +110,25 @@ static void set_myuid(void *data)
    l0g("could not get passwd entry for user %s\n", user);
    return;
    }
    -#ifdef HAVE_INITGROUPS
    - initgroups(real_user->pw_name, real_user->pw_gid);
    +#if defined(HAVE_GETGROUPLIST) && defined(HAVE_GETGROUPS) \ + && defined(HAVE_SETGROUPS)
    + int ngrps, maxgrps, tmp_ngrps;
    + gid_t *groups;
    + maxgrps = sysconf(_SC_NGROUPS_MAX);
    + if (maxgrps == -1) // value was indeterminate
    + maxgrps = 16394; // should be more than enough
    + groups = malloc(maxgrps * sizeof(gid_t));
    + if (groups) {
    + ngrps = maxgrps;
    + getgrouplist(user, real_user->pw_gid, groups, &ngrps);
    + tmp_ngrps = getgroups(maxgrps, &groups[ngrps]);
    + if (tmp_ngrps > 0)
    + ngrps += tmp_ngrps;
    + if (setgroups(ngrps, groups) == -1){
    + l0g("could not load groups for user %s\n", user);
    + }
    + free(groups);
    + }
    #endif
    if (setgid(real_user->pw_gid) == -1) {
    l0g("could not set gid to %ld\n",
    --
    1.7.2.3

    ----------------------8<--------------------------

     
  • Jan Engelhardt

    Jan Engelhardt - 2010-12-18

    Hm, that was fast. Cool.

     
  • Italo Valcy

    Italo Valcy - 2010-12-18

    Hi,

    We have one problem with that patch: if the user has some group in /etc/group, would say 'mygroup', and pam_group load the same group (mygroup), then we have that group will be listed twice in getgroups(). This is because the patch does not care about duplicated entries, and even setgroups() does not. But, I don't think this is a problem...

    Another issue is: when using pam-mount through SSH, it does not work... I mean: you log in as root and after call the command 'login', then pam_mount will fail with the same error of 'Permission deneid'. But this is subject to another bug report! :)

     
  • Nobody/Anonymous

    I was looking a little more in the problem and looks like the problem is not with pam-mount. Jengelh, I'll make some more tests and give you a feedback as soon as possible, ok?

     
  • Italo Valcy

    Italo Valcy - 2010-12-21

    Ops.. I forgot to login at the last comment... sorry. :)

     
  • Jan Engelhardt

    Jan Engelhardt - 2010-12-21
    • status: open --> pending-fixed
     
  • Jan Engelhardt

    Jan Engelhardt - 2010-12-21

    Added error checking and applied.

     
  • SourceForge Robot

    This Tracker item was closed automatically by the system. It was
    previously set to a Pending status, and the original submitter
    did not respond within 14 days (the time period specified by
    the administrator of this Tracker).

     
  • SourceForge Robot

    • status: pending-fixed --> closed-fixed
     

Log in to post a comment.