The way that pam_listfile currently checks whether or not a user is in
one of the groups listed is inefficient. It actually breaks in my case
where we have a large number of groups and members and LDAP is used for
name services ("ldap_result() failed: Size limit exceeded").
The problem is that it gets every possible group and checks whether or
not the user is a member, building a list of the ones that they are a
member of. It then compares that list to the groups listed in the file.
There is a lot of waste using this method.
Below is a patch that uses pam_modutil_user_in_group_nam_nam to just
check whether the user is a member of any of the groups listed in the
file. The patch also includes some indentation and block cleanup in the
attribute parsing to make it a little clearer which else is associated
with which if.
Thanks,
--
Heath Caldwell - hncaldwell@xxxxxxxxxxxxx
Operating Systems Analyst - California State Polytechnic University, Pomona
diff -ru Linux-PAM-1.0.4.orig/modules/pam_listfile/pam_listfile.c Linux-PAM-1.0.4/modules/pam_listfile/pam_listfile.c
--- Linux-PAM-1.0.4.orig/modules/pam_listfile/pam_listfile.c 2009-06-05 16:40:32.000000000 -0700
+++ Linux-PAM-1.0.4/modules/pam_listfile/pam_listfile.c 2009-06-05 16:42:34.000000000 -0700
@@ -82,7 +82,6 @@
/* Stuff for "extended" items */
struct passwd *userinfo;
struct group *grpinfo;
- char *itemlist[256]; /* Maximum of 256 items */
apply_type=APPLY_TYPE_NULL;
memset(apply_val,0,sizeof(apply_val));
@@ -127,7 +126,7 @@
if (!ifname)
return PAM_BUF_ERR;
strcpy(ifname,myval);
- } else if(!strcmp(mybuf,"item"))
+ } else if(!strcmp(mybuf,"item")) {
if(!strcmp(myval,"user"))
citem = PAM_USER;
else if(!strcmp(myval,"tty"))
@@ -145,7 +144,8 @@
extitem = EI_SHELL;
else
citem = 0;
- } else if(!strcmp(mybuf,"apply")) {
+ }
+ } else if(!strcmp(mybuf,"apply")) {
apply_type=APPLY_TYPE_NONE;
memset(apply_val,'\0',sizeof(apply_val));
if (myval[0]=='@') {
@@ -155,13 +155,13 @@
apply_type=APPLY_TYPE_USER;
strncpy(apply_val,myval,sizeof(apply_val)-1);
}
- } else if (!strcmp(mybuf,"quiet")) {
+ } else if (!strcmp(mybuf,"quiet")) {
quiet = 1;
- } else {
+ } else {
free(ifname);
pam_syslog(pamh,LOG_ERR, "Unknown option: %s",mybuf);
return onerr;
- }
+ }
}
if(!citem) {
@@ -264,30 +264,6 @@
if(extitem) {
switch(extitem) {
case EI_GROUP:
- userinfo = pam_modutil_getpwnam(pamh, citemp);
- if (userinfo == NULL) {
- pam_syslog(pamh,LOG_ERR, "getpwnam(%s) failed",
- citemp);
- free(ifname);
- return onerr;
- }
- grpinfo = pam_modutil_getgrgid(pamh, userinfo->pw_gid);
- if (grpinfo == NULL) {
- pam_syslog(pamh,LOG_ERR, "getgrgid(%d) failed",
- (int)userinfo->pw_gid);
- free(ifname);
- return onerr;
- }
- itemlist[0] = x_strdup(grpinfo->gr_name);
- setgrent();
- for (i=1; (i < (int)(sizeof(itemlist)/sizeof(itemlist[0])-1)) &&
- (grpinfo = getgrent()); ) {
- if (is_on_list(grpinfo->gr_mem,citemp)) {
- itemlist[i++] = x_strdup(grpinfo->gr_name);
- }
- }
- endgrent();
- itemlist[i] = NULL;
break;
case EI_SHELL:
/* Assume that we have already gotten PAM_USER in
@@ -358,13 +334,13 @@
continue;
if(aline[strlen(aline) - 1] == '\n')
aline[strlen(aline) - 1] = '\0';
- for(i=0;itemlist[i];)
- /* If any of the items match, strcmp() == 0, and we get out
- of this loop */
- retval = (strcmp(aline,itemlist[i++]) && retval);
+
+#ifdef DEBUG
+ pam_syslog(pamh,LOG_INFO,
+ "Checking if user is in group: user = %s, group = %s, retval = %d", citemp, aline, retval);
+#endif
+ retval = !pam_modutil_user_in_group_nam_nam(pamh,citemp,aline);
}
- for(i=0;itemlist[i];)
- free(itemlist[i++]);
} else {
while((fgets(aline,sizeof(aline),inf) != NULL)
&& retval) {
_______________________________________________
Pam-list mailing list
Pam-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/pam-list
