On Sat, 2002-05-25 at 20:17, Colm MacC�rthaigh wrote:
>
> Since there have been some changes to the affected source files
> and multiple problems presented themselves in unixd.c, my patches
> to make suexec + [ mod_include | mod_userdir | mod_cgid ] work
> were getting stale. So I've rediffed them against CVS.
>
> I also had a good look through all of the suexec bugs, I'm using
> the patches on a production system now with over 2000 shell users
> (redbrick.dcu.ie) and it's proving stable.
>
> Anyway, I think they fix these :
>
> PR 7810 - suexec + mod_userdir + mod_cgid needed fixing (also
> it's currently insecure by default, this really needs
> to be fixed)
> PR 7791 - malformed arguments array passed to suexec
> PR 8291 - mod_include + suexec "exec cmd" not working
> PR 9038 - really a duplicate of 7810
>
> Some notes:
>
> 1: http://redbrick.dcu.ie/~colmmacc/patches/mod_cgid.patch
> 2: http://redbrick.dcu.ie/~colmmacc/patches/unixd.patch
> 3: http://redbrick.dcu.ie/~colmmacc/patches/mod_include.patch
>
> patch 1 (mod_cgid.c) fixes 7810/9039/mod_cgid, it just works.
Patch 1 didn't work when I tried it. The problem was that it depends
on mod_userdir's get_suexec_id_doer() to create a proper suexec
config based on the userdir. But mod_userdir's get_suexec_id_doer()
function never is invoked, because mod_suexec's get_suexec_id_doer()
is called first.
I modified mod_userdir to insert its suexec identity function in
front of mod_suexec's. The suexec+cgid+userdir combination now works
properly in my test environment, but I'd like a second opinion before
I commit.
Thanks,
--Brian
Index: modules/generators/mod_cgid.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgid.c,v
retrieving revision 1.129
diff -u -r1.129 mod_cgid.c
--- modules/generators/mod_cgid.c 17 May 2002 11:33:09 -0000 1.129
+++ modules/generators/mod_cgid.c 26 May 2002 20:01:07 -0000
@@ -329,16 +329,8 @@
if (rc != sizeof(int)) {
return 1;
}
- rc = read(fd, &suexec_cfg->ugid.uid, sizeof(uid_t));
- if (rc != sizeof(uid_t)) {
- return 1;
- }
- rc = read(fd, &suexec_cfg->ugid.gid, sizeof(gid_t));
- if (rc != sizeof(gid_t)) {
- return 1;
- }
- rc = read(fd, &suexec_cfg->active, sizeof(int));
- if (rc != sizeof(int)) {
+ rc = read(fd, suexec_cfg, sizeof(*suexec_cfg));
+ if (rc != sizeof(*suexec_cfg)) {
return 1;
}
dconf[i] = (void *)suexec_cfg;
@@ -379,12 +371,20 @@
}
#endif
#endif
- /* For right now, just make the notes table. At some point we will need
- * to actually fill this out, but for now we just don't want suexec to
- * seg fault.
- */
+
+ /* basic notes table to avoid segfaults */
r->notes = apr_table_make(r->pool, 1);
+ /* mod_userdir requires the mod_userdir_user note */
+ rc = read(fd, &len, sizeof(len));
+ if ((rc == sizeof(len)) && len) {
+ data = apr_pcalloc(r->pool, len + 1); /* last byte is '\0' */
+ rc = read(fd, data, len);
+ if(rc != len) {
+ return 1;
+ }
+ apr_table_set(r->notes,"mod_userdir_user", data);
+ }
return 0;
}
@@ -441,9 +441,9 @@
suexec_mod);
write(fd, &suexec_mod->module_index, sizeof(int));
- write(fd, &suexec_cfg->ugid.uid, sizeof(uid_t));
- write(fd, &suexec_cfg->ugid.gid, sizeof(gid_t));
- write(fd, &suexec_cfg->active, sizeof(int));
+ write(fd, suexec_cfg, sizeof(*suexec_cfg));
+ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r,
+ "wrote suexec config %d bytes", sizeof(*suexec_cfg));
}
#if 0
@@ -483,6 +483,16 @@
}
#endif
#endif
+ /* send a minimal notes table */
+ data = (char *) apr_table_get(r->notes, "mod_userdir_user");
+ if(data != NULL) {
+ len = strlen(data);
+ write(fd, &len, sizeof(len));
+ write(fd, data, len);
+ } else {
+ len = 0;
+ write(fd, &len, sizeof(len));
+ }
}
static void daemon_signal_handler(int sig)
Index: modules/mappers/mod_userdir.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_userdir.c,v
retrieving revision 1.47
diff -u -r1.47 mod_userdir.c
--- modules/mappers/mod_userdir.c 7 May 2002 00:17:11 -0000 1.47
+++ modules/mappers/mod_userdir.c 26 May 2002 20:01:07 -0000
@@ -366,7 +366,7 @@
}
#ifdef HAVE_UNIX_SUEXEC
-static ap_unix_identity_t *get_suexec_id_doer(const request_rec *r)
+static ap_unix_identity_t *userdir_get_suexec_id_doer(const request_rec *r)
{
ap_unix_identity_t *ugid = NULL;
#if APR_HAS_USER
@@ -396,7 +396,7 @@
ap_hook_translate_name(translate_userdir,aszPre,NULL,APR_HOOK_MIDDLE);
#ifdef HAVE_UNIX_SUEXEC
- ap_hook_get_suexec_identity(get_suexec_id_doer,NULL,NULL,APR_HOOK_MIDDLE);
+ ap_hook_get_suexec_identity(userdir_get_suexec_id_doer,NULL,NULL,APR_HOOK_FIRST);
#endif
}
