[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

mod_auth_pam extended group auth patch



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 guardiani us
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 eatworms swmed edu 
+             */
+            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 samba org> 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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]