Hello,
Please see attached for a patch to mod_auth_pam that
adds extended group auth support to mod_auth_pam.
In other words, this patch allows you to auth by ANY
group a user is a member of, not just their primary
group.
I wrote this patch because I needed extended group
auth functionality to seemlessly integrate my
Subversion server with my W2K PDC using winbind.
My network policy states that any user who is a
member of the "staging" windows group should have
login access to the Subversion server. The user's
primary group is the "Domain Users" group by default,
so I couldn't use the stock mod_auth_pam code as
I needed to auth by an extended group - "staging".
I noticed that Samba didn't have any trouble auth'ing
by extended groups, so I set out to port the Samba
/etc/group auth code to mod_auth_pam. This patch is
the result of that. However, note that I found a bug
in the Samba 3.0.21c code, so it's a little different
than that code. I plan to submit a bug fix to the
samba project shortly if the bug still exists in their
source (I wrote this patch over a month ago, so I'm
not sure about the current state of things).
If you'd like to compare this patch to the samba
code, take a look at the validate_group() function
in source/smbd/password.c
Anyway, this code has been stable for a month on my
production Subversion server and in daily use by 3
programmers, so "it works for me". Unfortunately, it
still has a bit of Samba cruft attached to it, like
safe_string.h and safe_strcpy_fn(). I simply do not
have the time to refactor this code and remove this
samba baggage.
I hope this is useful for someone. Is there a chance
it can make it into the next mod_auth_pam release?
--
Jesse Guardiani
Programmer/Sys Admin
jesse@xxxxxxxxxxxx
diff -ruN mod_auth_pam.orig/mod_auth_sys_group.c mod_auth_pam/mod_auth_sys_group.c
--- mod_auth_pam.orig/mod_auth_sys_group.c 2002-08-31 07:09:52.000000000 -0400
+++ mod_auth_pam/mod_auth_sys_group.c 2006-05-25 16:33:56.000000000 -0400
@@ -98,11 +98,19 @@
#include "http_log.h"
#include "http_protocol.h"
#include "http_request.h"
+#include "safe_string.h"
+#include <syslog.h>
#define VERSION "2.0-1.1"
+#define PSTRING_LEN 1024
+#define FSTRING_LEN 256
+
module auth_sys_group_module;
+typedef char pstring[PSTRING_LEN];
+typedef char fstring[FSTRING_LEN];
+
/*
* configuration directive handling
*/
@@ -159,6 +167,47 @@
{ NULL }
};
+/**
+ Safe string copy into a known length string. maxlength does not
+ include the terminating zero.
+**/
+
+char *safe_strcpy_fn(const char *fn, int line, char *dest,const char *src, size_t maxlength)
+{
+ setlogmask (LOG_UPTO (LOG_NOTICE));
+ openlog ("messages", LOG_CONS | LOG_PID | LOG_NDELAY, LOG_LOCAL1);
+
+ size_t len;
+
+ if (!dest) {
+ syslog (LOG_ERR,"ERROR: NULL dest in safe_strcpy, called from [%s][%d]\n", fn, line);
+ closelog ();
+ return NULL;
+ }
+
+ if (!src) {
+ *dest = 0;
+ return dest;
+ }
+
+ len = strnlen(src, maxlength+1);
+
+ if (len > maxlength) {
+ syslog (LOG_ERR,"ERROR: string overflow by %lu (%lu - %lu) in safe_strcpy [%.50s]\n",
+ (unsigned long)(len-maxlength), (unsigned long)len,
+ (unsigned long)maxlength, src);
+ closelog ();
+
+ len = maxlength;
+ }
+
+ memmove(dest, src, len);
+ dest[len] = 0;
+ return dest;
+}
+
+
+
/*
* These functions return 0 if client is OK, and proper error status
* if not... either AUTH_REQUIRED, if we made a check, and it failed, or
@@ -181,6 +230,8 @@
(auth_sys_group_dir_config*) ap_get_module_config(
r->per_dir_config, &auth_sys_group_module);
struct passwd *pwent;
+ struct passwd *pwent_tmp;
+ struct passwd pwent_struct;
/* check for allowed users/group */
const apr_array_header_t *reqs_arr = ap_requires (r);
@@ -192,7 +243,10 @@
/* retrieve user info from passwd and use that info instead of
* server supplied info */
- pwent = getpwnam(r->user);
+ pwent_tmp = getpwnam(r->user);
+ memcpy((char *)&pwent_struct, pwent_tmp, sizeof(struct passwd));
+ pwent = &pwent_struct;
+
if(pwent == NULL)
return DECLINED;
@@ -214,27 +268,72 @@
type = ap_getword(r->pool, (const char**)&line, ' ');
if(!strcmp(type, "group") && (r->user)) {
- struct group *grent;
- char **members;
+ struct group *grent;
+ char **members;
+
+ while (*line) {
+ char* groupname = ap_getword_conf(r->pool, (const char**)&line);
+
+ if((grent = getgrnam(groupname)) && grent->gr_mem) {
+ members = grent->gr_mem;
- while (*line) {
- char* groupname = ap_getword_conf(r->pool, (const char**)&line);
-
- if((grent = getgrnam(groupname)) && grent->gr_mem) {
- members = grent->gr_mem;
-
- /* maybe its the primary group? saves the comparisons */
- if(pwent->pw_gid == grent->gr_gid)
- return OK;
-
- while (*members) {
- if (strcmp (*members, pwent->pw_name) == 0)
- return OK;
-
- members ++;
- }
- }
- }
+ /* maybe its the primary group? saves the comparisons */
+ if(pwent->pw_gid == grent->gr_gid)
+ return OK;
+
+
+ struct group *gptr;
+ setgrent();
+ while ((gptr = (struct group *)getgrent())) {
+ if (gptr->gr_gid == grent->gr_gid)
+ break;
+ }
+
+ /*
+ * As user_ok can recurse doing a getgrent(), we must
+ * copy the member list into a pstring on the stack before
+ * use. Bug pointed out by leon@xxxxxxxxxxxxxxxxxxx
+ */
+ if (gptr) {
+ pstring member_list;
+ char *member;
+ size_t copied_len = 0;
+ int i;
+
+ *member_list = '\0';
+ member = member_list;
+
+ for(i = 0; gptr->gr_mem && gptr->gr_mem[i]; i++) {
+ size_t member_len = strlen(gptr->gr_mem[i]) + 1;
+ if( copied_len + member_len < sizeof(pstring)) {
+ safe_strcpy(member, gptr->gr_mem[i], sizeof(pstring) - copied_len - 1);
+ copied_len += member_len;
+ member += member_len;
+ } else {
+ *member = '\0';
+ }
+ }
+
+ endgrent();
+ member = member_list;
+
+ while (*member) {
+ static fstring name;
+ fstrcpy(name,member);
+ struct passwd *pwent2;
+ pwent2 = getpwnam(name);
+
+ if (pwent2 != NULL && pwent2->pw_uid == pwent->pw_uid) {
+ endgrent();
+ return OK;
+ }
+ member += strlen(member) + 1;
+ }
+ } else {
+ endgrent();
+ }
+ }
+ }
} /* end if group */
} /* method mask */
}
diff -ruN mod_auth_pam.orig/safe_string.h mod_auth_pam/safe_string.h
--- mod_auth_pam.orig/safe_string.h 1969-12-31 19:00:00.000000000 -0500
+++ mod_auth_pam/safe_string.h 2006-03-27 19:43:15.000000000 -0500
@@ -0,0 +1,228 @@
+/*
+ Unix SMB/CIFS implementation.
+ Safe string handling routines.
+ Copyright (C) Andrew Tridgell 1994-1998
+ Copyright (C) Andrew Bartlett <abartlet@xxxxxxxxx> 2003
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+#ifndef _SAFE_STRING_H
+#define _SAFE_STRING_H
+
+#ifndef _SPLINT_ /* http://www.splint.org */
+
+/* Some macros to ensure people don't use buffer overflow vulnerable string
+ functions. */
+
+#ifdef bcopy
+#undef bcopy
+#endif /* bcopy */
+#define bcopy(src,dest,size) __ERROR__XX__NEVER_USE_BCOPY___;
+
+#ifdef strcpy
+#undef strcpy
+#endif /* strcpy */
+#define strcpy(dest,src) __ERROR__XX__NEVER_USE_STRCPY___;
+
+#ifdef strcat
+#undef strcat
+#endif /* strcat */
+#define strcat(dest,src) __ERROR__XX__NEVER_USE_STRCAT___;
+
+#ifdef sprintf
+#undef sprintf
+#endif /* sprintf */
+#define sprintf __ERROR__XX__NEVER_USE_SPRINTF__;
+
+/*
+ * strcasecmp/strncasecmp aren't an error, but it means you're not thinking about
+ * multibyte. Don't use them. JRA.
+ */
+#ifdef strcasecmp
+#undef strcasecmp
+#endif
+#define strcasecmp __ERROR__XX__NEVER_USE_STRCASECMP__;
+
+#ifdef strncasecmp
+#undef strncasecmp
+#endif
+#define strncasecmp __ERROR__XX__NEVER_USE_STRNCASECMP__;
+
+#endif /* !_SPLINT_ */
+
+#ifdef DEVELOPER
+#define SAFE_STRING_FUNCTION_NAME FUNCTION_MACRO
+#define SAFE_STRING_LINE __LINE__
+#else
+#define SAFE_STRING_FUNCTION_NAME ("")
+#define SAFE_STRING_LINE (0)
+#endif
+
+/* We need a number of different prototypes for our
+ non-existant fuctions */
+char * __unsafe_string_function_usage_here__(void);
+
+size_t __unsafe_string_function_usage_here_size_t__(void);
+
+size_t __unsafe_string_function_usage_here_char__(void);
+
+#ifdef HAVE_COMPILER_WILL_OPTIMIZE_OUT_FNS
+
+/* if the compiler will optimize out function calls, then use this to tell if we are
+ have the correct types (this works only where sizeof() returns the size of the buffer, not
+ the size of the pointer). */
+
+#define CHECK_STRING_SIZE(d, len) (sizeof(d) != (len) && sizeof(d) != sizeof(char *))
+
+#define fstrterminate(d) (CHECK_STRING_SIZE(d, sizeof(fstring)) \
+ ? __unsafe_string_function_usage_here_char__() \
+ : (((d)[sizeof(fstring)-1]) = '\0'))
+#define pstrterminate(d) (CHECK_STRING_SIZE(d, sizeof(pstring)) \
+ ? __unsafe_string_function_usage_here_char__() \
+ : (((d)[sizeof(pstring)-1]) = '\0'))
+
+#define wpstrcpy(d,s) ((sizeof(d) != sizeof(wpstring) && sizeof(d) != sizeof(smb_ucs2_t *)) \
+ ? __unsafe_string_function_usage_here__() \
+ : safe_strcpy_w((d),(s),sizeof(wpstring)))
+#define wpstrcat(d,s) ((sizeof(d) != sizeof(wpstring) && sizeof(d) != sizeof(smb_ucs2_t *)) \
+ ? __unsafe_string_function_usage_here__() \
+ : safe_strcat_w((d),(s),sizeof(wpstring)))
+#define wfstrcpy(d,s) ((sizeof(d) != sizeof(wfstring) && sizeof(d) != sizeof(smb_ucs2_t *)) \
+ ? __unsafe_string_function_usage_here__() \
+ : safe_strcpy_w((d),(s),sizeof(wfstring)))
+#define wfstrcat(d,s) ((sizeof(d) != sizeof(wfstring) && sizeof(d) != sizeof(smb_ucs2_t *)) \
+ ? __unsafe_string_function_usage_here__() \
+ : safe_strcat_w((d),(s),sizeof(wfstring)))
+
+#define push_pstring_base(dest, src, pstring_base) \
+ (CHECK_STRING_SIZE(pstring_base, sizeof(pstring)) \
+ ? __unsafe_string_function_usage_here_size_t__() \
+ : push_ascii(dest, src, sizeof(pstring)-PTR_DIFF(dest,pstring_base)-1, STR_TERMINATE))
+
+#else /* HAVE_COMPILER_WILL_OPTIMIZE_OUT_FNS */
+
+#define fstrterminate(d) (((d)[sizeof(fstring)-1]) = '\0')
+#define pstrterminate(d) (((d)[sizeof(pstring)-1]) = '\0')
+
+#define wpstrcpy(d,s) safe_strcpy_w((d),(s),sizeof(wpstring))
+#define wpstrcat(d,s) safe_strcat_w((d),(s),sizeof(wpstring))
+#define wfstrcpy(d,s) safe_strcpy_w((d),(s),sizeof(wfstring))
+#define wfstrcat(d,s) safe_strcat_w((d),(s),sizeof(wfstring))
+
+#define push_pstring_base(dest, src, pstring_base) \
+ push_ascii(dest, src, sizeof(pstring)-PTR_DIFF(dest,pstring_base)-1, STR_TERMINATE)
+
+#endif /* HAVE_COMPILER_WILL_OPTIMIZE_OUT_FNS */
+
+#define safe_strcpy_base(dest, src, base, size) \
+ safe_strcpy(dest, src, size-PTR_DIFF(dest,base)-1)
+
+/* String copy functions - macro hell below adds 'type checking' (limited,
+ but the best we can do in C) and may tag with function name/number to
+ record the last 'clobber region' on that string */
+
+#define pstrcpy(d,s) safe_strcpy((d), (s),sizeof(pstring)-1)
+#define pstrcat(d,s) safe_strcat((d), (s),sizeof(pstring)-1)
+#define fstrcpy(d,s) safe_strcpy((d),(s),sizeof(fstring)-1)
+#define fstrcat(d,s) safe_strcat((d),(s),sizeof(fstring)-1)
+#define nstrcpy(d,s) safe_strcpy((d), (s),sizeof(nstring)-1)
+#define unstrcpy(d,s) safe_strcpy((d), (s),sizeof(unstring)-1)
+
+/* the addition of the DEVELOPER checks in safe_strcpy means we must
+ * update a lot of code. To make this a little easier here are some
+ * functions that provide the lengths with less pain */
+#define pstrcpy_base(dest, src, pstring_base) \
+ safe_strcpy(dest, src, sizeof(pstring)-PTR_DIFF(dest,pstring_base)-1)
+
+
+/* Inside the _fn variants of these is a call to clobber_region(), -
+ * which might destroy the stack on a buggy function. We help the
+ * debugging process by putting the function and line who last caused
+ * a clobbering into a static buffer. If the program crashes at
+ * address 0xf1f1f1f1 then this function is probably, but not
+ * necessarily, to blame. */
+
+/* overmalloc_safe_strcpy: DEPRECATED! Used when you know the
+ * destination buffer is longer than maxlength, but you don't know how
+ * long. This is not a good situation, because we can't do the normal
+ * sanity checks. Don't use in new code! */
+
+#define overmalloc_safe_strcpy(dest,src,maxlength) safe_strcpy_fn(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE,dest,src,maxlength)
+#define safe_strcpy(dest,src,maxlength) safe_strcpy_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE,dest,src,maxlength)
+#define safe_strcat(dest,src,maxlength) safe_strcat_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE,dest,src,maxlength)
+#define push_string(base_ptr, dest, src, dest_len, flags) push_string_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, base_ptr, dest, src, dest_len, flags)
+#define pull_string(base_ptr, dest, src, dest_len, src_len, flags) pull_string_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, base_ptr, dest, src, dest_len, src_len, flags)
+#define clistr_push(cli, dest, src, dest_len, flags) clistr_push_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, cli, dest, src, dest_len, flags)
+#define clistr_pull(cli, dest, src, dest_len, src_len, flags) clistr_pull_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, cli, dest, src, dest_len, src_len, flags)
+#define srvstr_push(base_ptr, dest, src, dest_len, flags) srvstr_push_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, base_ptr, dest, src, dest_len, flags)
+
+#define alpha_strcpy(dest,src,other_safe_chars,maxlength) alpha_strcpy_fn(SAFE_STRING_FUNCTION_NAME,SAFE_STRING_LINE,dest,src,other_safe_chars,maxlength)
+#define StrnCpy(dest,src,n) StrnCpy_fn(SAFE_STRING_FUNCTION_NAME,SAFE_STRING_LINE,dest,src,n)
+
+#ifdef HAVE_COMPILER_WILL_OPTIMIZE_OUT_FNS
+
+/* if the compiler will optimize out function calls, then use this to tell if we are
+ have the correct types (this works only where sizeof() returns the size of the buffer, not
+ the size of the pointer). */
+
+#define safe_strcpy_fn2(fn_name, fn_line, d, s, max_len) \
+ (CHECK_STRING_SIZE(d, max_len+1) \
+ ? __unsafe_string_function_usage_here__() \
+ : safe_strcpy_fn(fn_name, fn_line, (d), (s), (max_len)))
+
+#define safe_strcat_fn2(fn_name, fn_line, d, s, max_len) \
+ (CHECK_STRING_SIZE(d, max_len+1) \
+ ? __unsafe_string_function_usage_here__() \
+ : safe_strcat_fn(fn_name, fn_line, (d), (s), (max_len)))
+
+#define push_string_fn2(fn_name, fn_line, base_ptr, dest, src, dest_len, flags) \
+ (CHECK_STRING_SIZE(dest, dest_len) \
+ ? __unsafe_string_function_usage_here_size_t__() \
+ : push_string_fn(fn_name, fn_line, base_ptr, dest, src, dest_len, flags))
+
+#define pull_string_fn2(fn_name, fn_line, base_ptr, dest, src, dest_len, src_len, flags) \
+ (CHECK_STRING_SIZE(dest, dest_len) \
+ ? __unsafe_string_function_usage_here_size_t__() \
+ : pull_string_fn(fn_name, fn_line, base_ptr, dest, src, dest_len, src_len, flags))
+
+#define clistr_push_fn2(fn_name, fn_line, cli, dest, src, dest_len, flags) \
+ (CHECK_STRING_SIZE(dest, dest_len) \
+ ? __unsafe_string_function_usage_here_size_t__() \
+ : clistr_push_fn(fn_name, fn_line, cli, dest, src, dest_len, flags))
+
+#define clistr_pull_fn2(fn_name, fn_line, cli, dest, src, dest_len, srclen, flags) \
+ (CHECK_STRING_SIZE(dest, dest_len) \
+ ? __unsafe_string_function_usage_here_size_t__() \
+ : clistr_pull_fn(fn_name, fn_line, cli, dest, src, dest_len, srclen, flags))
+
+#define srvstr_push_fn2(fn_name, fn_line, base_ptr, dest, src, dest_len, flags) \
+ (CHECK_STRING_SIZE(dest, dest_len) \
+ ? __unsafe_string_function_usage_here_size_t__() \
+ : srvstr_push_fn(fn_name, fn_line, base_ptr, dest, src, dest_len, flags))
+
+#else
+
+#define safe_strcpy_fn2 safe_strcpy_fn
+#define safe_strcat_fn2 safe_strcat_fn
+#define push_string_fn2 push_string_fn
+#define pull_string_fn2 pull_string_fn
+#define clistr_push_fn2 clistr_push_fn
+#define clistr_pull_fn2 clistr_pull_fn
+#define srvstr_push_fn2 srvstr_push_fn
+
+#endif
+
+#endif
_______________________________________________
Pam-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/pam-list
