You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2005/08/30 13:26:48 UTC

svn commit: r264759 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c

Author: colm
Date: Tue Aug 30 04:26:45 2005
New Revision: 264759

URL: http://svn.apache.org/viewcvs?rev=264759&view=rev
Log:

Fix PR36410; Change how the get_suexec_identity hook is handled by CGID.
Instead of using mod_userdir and mod_suexec specific hacks, we now run the hook
on the httpd side of the handler. 

If this is NULL, we pass on a magic empty_ugid constant, otherwise pass on the
real ugid.

On the cgid side of the equation, we add our own hook, with REALLY_FIRST, and
then order the hooks. This ensures that cgid's doer runs before any other
registered get_suexec_identity doers.

We use cgid's request config to store the ugid. If ugid == empty_ugid, we DON'T
call ap_os_create_privileged_process, because our doer would return the magic
empty_ugid constant. Having the doer return NULL is no good, because then
userdir and mod_suexec's doers would be called. Instead, we call plain old
apr_proc_create(). 


Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/generators/mod_cgid.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?rev=264759&r1=264758&r2=264759&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Tue Aug 30 04:26:45 2005
@@ -2,6 +2,9 @@
 Changes with Apache 2.3.0
   [Remove entries to the current 2.0 and 2.2 section below, when backported]
 
+  *) mod_cgid: run the get_suexec_identity hook within the request-handler 
+     instead of within cgid. PR36410. [Colm MacCarthaigh]
+
   *) Correct mod_cgid's argv[0] so that the full path can be delved by the
      invoked cgi application, to conform to the behavior of mod_cgi.
      [Pradeep Kumar S <pradeep.smani gmail.com>]

Modified: httpd/httpd/trunk/modules/generators/mod_cgid.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/generators/mod_cgid.c?rev=264759&r1=264758&r2=264759&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/generators/mod_cgid.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_cgid.c Tue Aug 30 04:26:45 2005
@@ -91,10 +91,20 @@
 static apr_pool_t *root_pool = NULL;
 static const char *sockname;
 static pid_t parent_pid;
+static ap_unix_identity_t empty_ugid = { -1, -1, -1 };
 
 /* Read and discard the data in the brigade produced by a CGI script */
 static void discard_script_output(apr_bucket_brigade *bb);
 
+/* This doer will only ever be called when we are sure that we have
+ * a valid ugid.
+ */
+static ap_unix_identity_t *cgid_suexec_id_doer(const request_rec *r)
+{
+    return (ap_unix_identity_t *) 
+                        ap_get_module_config(r->request_config, &cgid_module);
+}
+
 /* KLUDGE --- for back-combatibility, we don't have to check ExecCGI
  * in ScriptAliased directories, which means we need to know if this 
  * request came through ScriptAlias or not... so the Alias module 
@@ -159,15 +169,12 @@
                             * wrong cgid socket use
                             */
     int core_module_index;
-    int have_suexec;
-    int suexec_module_index;
-    suexec_config_t suexec_cfg;
     int env_count;
+    ap_unix_identity_t ugid;
     apr_size_t filename_len;
     apr_size_t argv0_len;
     apr_size_t uri_len;
     apr_size_t args_len;
-    apr_size_t mod_userdir_user_len;
     int loglevel; /* to stuff in server_rec */
 } cgid_req_t;
 
@@ -328,10 +335,9 @@
                             cgid_req_t *req)
 { 
     int i; 
-    char *user;
     char **environ; 
-    core_dir_config *temp_core; 
-    void **dconf;
+    core_request_config *temp_core; 
+    void **rconf;
     apr_status_t stat;
 
     r->server = apr_pcalloc(r->pool, sizeof(server_rec)); 
@@ -348,17 +354,13 @@
     }
 
     /* handle module indexes and such */
-    dconf = (void **) apr_pcalloc(r->pool, sizeof(void *) * (total_modules + DYNAMIC_MODULE_LIMIT));
-
-    temp_core = (core_dir_config *)apr_palloc(r->pool, sizeof(core_module)); 
-    dconf[req->core_module_index] = (void *)temp_core;
-
-    if (req->have_suexec) {
-        dconf[req->suexec_module_index] = &req->suexec_cfg;
-    }
-
-    r->per_dir_config = (ap_conf_vector_t *)dconf; 
+    rconf = (void **) apr_pcalloc(r->pool, sizeof(void *) * (total_modules + DYNAMIC_MODULE_LIMIT));
 
+    temp_core = (core_request_config *)apr_palloc(r->pool, sizeof(core_module)); 
+    rconf[req->core_module_index] = (void *)temp_core;
+    r->request_config = (ap_conf_vector_t *)rconf; 
+    ap_set_module_config(r->request_config, &cgid_module, (void *)&req->ugid);
+    
     /* Read the filename, argv0, uri, and args */
     r->filename = apr_pcalloc(r->pool, req->filename_len + 1);
     *argv0 = apr_pcalloc(r->pool, req->argv0_len + 1);
@@ -391,19 +393,6 @@
     }
     *env = environ;
 
-    /* basic notes table to avoid segfaults */
-    r->notes = apr_table_make(r->pool, 1);
-
-    /* mod_userdir requires the mod_userdir_user note */
-    if (req->mod_userdir_user_len) {
-        user = apr_pcalloc(r->pool, req->mod_userdir_user_len + 1); /* last byte is '\0' */
-        stat = sock_read(fd, user, req->mod_userdir_user_len);
-        if (stat != APR_SUCCESS) {
-            return stat;
-        }
-        apr_table_set(r->notes, "mod_userdir_user", (const char *)user);
-    }
-
 #if 0
 #ifdef RLIMIT_CPU 
     sock_read(fd, &j, sizeof(int)); 
@@ -446,23 +435,20 @@
                              int req_type) 
 { 
     int i;
-    const char *user;
-    module *suexec_mod = ap_find_linked_module("mod_suexec.c");
     cgid_req_t req = {0};
-    suexec_config_t *suexec_cfg;
     apr_status_t stat;
+    ap_unix_identity_t * ugid = ap_run_get_suexec_identity(r);
 
+    if (ugid == NULL) {
+        req.ugid = empty_ugid;
+    } else {
+        memcpy(&req.ugid, ugid, sizeof(ap_unix_identity_t));
+    }
+    
     req.req_type = req_type;
     req.ppid = parent_pid;
     req.conn_id = r->connection->id;
     req.core_module_index = core_module.module_index;
-    if (suexec_mod) {
-        req.have_suexec = 1;
-        req.suexec_module_index = suexec_mod->module_index;
-        suexec_cfg = ap_get_module_config(r->per_dir_config,
-                                          suexec_mod);
-        req.suexec_cfg = *suexec_cfg;
-    }
     for (req.env_count = 0; env[req.env_count]; req.env_count++) {
         continue; 
     }
@@ -470,10 +456,6 @@
     req.argv0_len = strlen(argv0);
     req.uri_len = strlen(r->uri);
     req.args_len = r->args ? strlen(r->args) : 0;
-    user = (const char *)apr_table_get(r->notes, "mod_userdir_user");
-    if (user != NULL) {
-        req.mod_userdir_user_len = strlen(user);
-    }
     req.loglevel = r->server->loglevel;
 
     /* Write the request header */
@@ -506,13 +488,6 @@
         }
     }
 
-    /* send a minimal notes table */
-    if (user) {
-        if ((stat = sock_write(fd, user, req.mod_userdir_user_len)) != APR_SUCCESS) {
-            return stat;
-        }
-    }
-
 #if 0
 #ifdef RLIMIT_CPU 
     if (conf->limit_cpu) { 
@@ -593,6 +568,11 @@
 
     /* Close our copy of the listening sockets */
     ap_close_listeners();
+
+    /* cgid should use its own suexec doer */
+    ap_hook_get_suexec_identity(cgid_suexec_id_doer, NULL, NULL,
+                                APR_HOOK_REALLY_FIRST);
+    apr_hook_sort_all();
     
     if ((sd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
         ap_log_error(APLOG_MARK, APLOG_ERR, errno, main_server, 
@@ -753,10 +733,19 @@
             */
             close(sd2);
 
-            rc = ap_os_create_privileged_process(r, procnew, argv0, argv, 
-                                                 (const char * const *)env, 
-                                                 procattr, ptrans);
-
+            if (memcmp(&empty_ugid, &cgid_req.ugid, sizeof(empty_ugid))) {
+                /* We have a valid identity, and can be sure that 
+                 * cgid_suexec_id_doer will return a valid ugid 
+                 */
+                rc = ap_os_create_privileged_process(r, procnew, argv0, argv,
+                                                     (const char * const *)env,
+                                                     procattr, ptrans);
+            } else {
+                rc = apr_proc_create(procnew, argv0, argv, 
+                                     (const char * const *)env, 
+                                     procattr, ptrans);
+            }
+                
             if (rc != APR_SUCCESS) {
                 /* Bad things happened. Everyone should have cleaned up.
                  * ap_log_rerror() won't work because the header table used by