Merge r551843, r551889 from trunk:

Add alternative fixes for CVE-2007-3304:

* configure.in: Check for getpgid.

* include/mpm_common.h (ap_mpm_safe_kill): New prototype.

* server/mpm_common.c (reclaim_one_pid): Ensure pid validity before
calling apr_proc_wait().
(ap_mpm_safe_kill): New function.

* server/mpm/prefork/prefork.c, server/mpm/worker/worker.c,
server/mpm/experimental/event/event.c: Use ap_mpm_safe_kill() on pids
from the scoreboard, throughout.

* include/ap_mmn.h: Minor bump.

* server/mpm_common.c: getpgid() returns a pid_t

Submitted by: jorton, jim
Reviewed by: jorton, jim, rpluem

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@556936 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index fd9542fb1098e7901a6807e3e78b2034967bc848..6a4b87bab563ef9aa8c7c3e5c7ddc67703f01cd3 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,11 @@ Changes with Apache 2.2.5
      Cache-Control header without any value. 
      [Niklas Edmundsson <nikke acc.umu.se>]
 
+  *) SECURITY: CVE-2007-3304 (cve.mitre.org)
+     prefork, worker, event MPMs: Ensure that the parent process cannot
+     be forced to kill processes outside its process group. 
+     [Joe Orton, Jim Jagielski]
+
   *) mod_cache: Do not set Date or Expires when they are missing from
      the original response or are invalid.  [Justin Erenkrantz]
 

diff --git a/configure.in b/configure.in
index 869ebd81cf09922e3244cd0865a51a8c304b49c3..5ab7133c22ec690a5a532f176d0402b0e62d8054 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -392,6 +392,7 @@ initgroups \
 bindprocessor \
 prctl \
 timegm \
+getpgid
 )
 
 dnl confirm that a void pointer is large enough to store a long integer

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 9d8d89cd2c32a66bb6bf41b4812c0b4cea59f356..95871f1b4b3a82010a39acb7e56af93046b2b96f 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -113,6 +113,8 @@
  * 20051115.3 (2.2.3)  Added server_scheme member to server_rec (minor)
  * 20051115.4 (2.2.4)  Added ap_get_server_banner() and
  *                         ap_get_server_description() (minor)
+ * 20051115.5 (2.2.5)  Added ap_mpm_safe_kill() (minor)
+ *
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503232UL /* "AP22" */
@@ -120,7 +122,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20051115
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 4                     /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 5                     /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

diff --git a/include/mpm_common.h b/include/mpm_common.h
index bc4480a792db6adf48f3d43d4090a47126381855..bacf76f256beb077138f52b17098f2d2ea1e2680 100644 (file)
--- a/include/mpm_common.h
+++ b/include/mpm_common.h
@@ -144,6 +144,19 @@ void ap_register_extra_mpm_process(pid_t pid);
 int ap_unregister_extra_mpm_process(pid_t pid);
 #endif
 
+/**
+ * Safely signal an MPM child process, if the process is in the
+ * current process group.  Otherwise fail.
+ * @param pid the process id of a child process to signal
+ * @param sig the signal number to send
+ * @return APR_SUCCESS if signal is sent, otherwise an error as per kill(3);
+ * APR_EINVAL is returned if passed either an invalid (< 1) pid, or if
+ * the pid is not in the current process group
+ */
+#ifdef AP_MPM_WANT_RECLAIM_CHILD_PROCESSES
+apr_status_t ap_mpm_safe_kill(pid_t pid, int sig);
+#endif
+
 /**
  * Determine if any child process has died.  If no child process died, then
  * this process sleeps for the amount of time specified by the MPM defined

diff --git a/server/mpm/experimental/event/event.c b/server/mpm/experimental/event/event.c
index e105c25e97d117c821cc0a015ef766ae8374c7fd..2a6913b3c3d518621f0661d7760864ba6455bcc4 100644 (file)
--- a/server/mpm/experimental/event/event.c
+++ b/server/mpm/experimental/event/event.c
@@ -1998,12 +1998,10 @@ int ap_mpm_run(apr_pool_t * _pconf, apr_pool_t * plog, server_rec * s)
 
             active_children = 0;
             for (index = 0; index < ap_daemons_limit; ++index) {
-                if (MPM_CHILD_PID(index) != 0) {
-                    if (kill(MPM_CHILD_PID(index), 0) == 0) {
-                            active_children = 1;
-                            /* Having just one child is enough to stay around */
-                            break;
-                    }
+                if (ap_mpm_safe_kill(MPM_CHILD_PID(index), 0) == APR_SUCCESS) {
+                    active_children = 1;
+                    /* Having just one child is enough to stay around */
+                    break;
                 }
             }
         } while (!shutdown_pending && active_children &&

diff --git a/server/mpm/prefork/prefork.c b/server/mpm/prefork/prefork.c
index ba50581b6a717180064d773f24134faaa7b176d9..d3c93a63f6854d159298a4e84e332fb63623820e 100644 (file)
--- a/server/mpm/prefork/prefork.c
+++ b/server/mpm/prefork/prefork.c
@@ -1127,7 +1127,7 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
         for (index = 0; index < ap_daemons_limit; ++index) {
             if (ap_scoreboard_image->servers[index][0].status != SERVER_DEAD) {
                 /* Ask each child to close its listeners. */
-                kill(MPM_CHILD_PID(index), AP_SIG_GRACEFUL);
+                ap_mpm_safe_kill(MPM_CHILD_PID(index), AP_SIG_GRACEFUL);
                 active_children++;
             }
         }
@@ -1165,12 +1165,10 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
 
             active_children = 0;
             for (index = 0; index < ap_daemons_limit; ++index) {
-                if (MPM_CHILD_PID(index) != 0) {
-                    if (kill(MPM_CHILD_PID(index), 0) == 0) {
-                            active_children = 1;
-                            /* Having just one child is enough to stay around */
-                            break;
-                    }
+                if (ap_mpm_safe_kill(MPM_CHILD_PID(index), 0) == APR_SUCCESS) {
+                    active_children = 1;
+                    /* Having just one child is enough to stay around */
+                    break;
                 }
             }
         } while (!shutdown_pending && active_children &&
@@ -1222,7 +1220,7 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
                  * piped loggers, etc. They almost certainly won't handle
                  * it gracefully.
                  */
-                kill(ap_scoreboard_image->parent[index].pid, AP_SIG_GRACEFUL);
+                ap_mpm_safe_kill(ap_scoreboard_image->parent[index].pid, AP_SIG_GRACEFUL);
             }
         }
     }

diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c
index a9d822f5411708bcab6f13c36f3529c39e556d92..b6b9447b70e07cbc9fd242d5aeea8479845fd9f8 100644 (file)
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -1813,12 +1813,10 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
 
             active_children = 0;
             for (index = 0; index < ap_daemons_limit; ++index) {
-                if (MPM_CHILD_PID(index) != 0) {
-                    if (kill(MPM_CHILD_PID(index), 0) == 0) {
-                            active_children = 1;
-                            /* Having just one child is enough to stay around */
-                            break;
-                    }
+                if (ap_mpm_safe_kill(MPM_CHILD_PID(index), 0) == APR_SUCCESS) {
+                    active_children = 1;
+                    /* Having just one child is enough to stay around */
+                    break;
                 }
             }
         } while (!shutdown_pending && active_children &&

diff --git a/server/mpm_common.c b/server/mpm_common.c
index c654cbfb2212ebea7d5f0eb871c055457e8f525a..f80637330f3d8a2f803665412facf821cff9f6d8 100644 (file)
--- a/server/mpm_common.c
+++ b/server/mpm_common.c
@@ -126,6 +126,11 @@ static int reclaim_one_pid(pid_t pid, action_t action)
     apr_proc_t proc;
     apr_status_t waitret;
 
+    /* Ensure pid sanity. */
+    if (pid < 1) {
+        return 1;
+    }        
+
     proc.pid = pid;
     waitret = apr_proc_wait(&proc, NULL, NULL, APR_NOWAIT);
     if (waitret != APR_CHILD_NOTDONE) {
@@ -305,6 +310,66 @@ void ap_relieve_child_processes(void)
         cur_extra = next;
     }
 }
+
+/* Before sending the signal to the pid this function verifies that
+ * the pid is a member of the current process group; either using
+ * apr_proc_wait(), where waitpid() guarantees to fail for non-child
+ * processes; or by using getpgid() directly, if available. */
+apr_status_t ap_mpm_safe_kill(pid_t pid, int sig)
+{
+#ifndef HAVE_GETPGID
+    apr_proc_t proc;
+    apr_status_t rv;
+    apr_exit_why_e why;
+    int status;
+
+    /* Ensure pid sanity */
+    if (pid < 1) {
+        return APR_EINVAL;
+    }
+
+    proc.pid = pid;
+    rv = apr_proc_wait(&proc, &status, &why, APR_NOWAIT);
+    if (rv == APR_CHILD_DONE) {
+#ifdef AP_MPM_WANT_PROCESS_CHILD_STATUS
+        /* The child already died - log the termination status if
+         * necessary: */
+        ap_process_child_status(&proc, why, status);
+#endif
+        return APR_EINVAL;
+    }
+    else if (rv != APR_CHILD_NOTDONE) {
+        /* The child is already dead and reaped, or was a bogus pid -
+         * log this either way. */
+        ap_log_error(APLOG_MARK, APLOG_NOTICE, rv, ap_server_conf,
+                     "cannot send signal %d to pid %ld (non-child or "
+                     "already dead)", sig, (long)pid);
+        return APR_EINVAL;
+    }
+#else
+    pid_t pg;
+
+    /* Ensure pid sanity. */
+    if (pid < 1) {
+        return APR_EINVAL;
+    }
+
+    pg = getpgid(pid);    
+    if (pg == -1) {
+        /* Process already dead... */
+        return errno;
+    }
+
+    if (pg != getpgrp()) {
+        ap_log_error(APLOG_MARK, APLOG_ALERT, 0, ap_server_conf,
+                     "refusing to send signal %d to pid %ld outside "
+                     "process group", sig, (long)pid);
+        return APR_EINVAL;
+    }
+#endif        
+
+    return kill(pid, sig) ? errno : APR_SUCCESS;
+}
 #endif /* AP_MPM_WANT_RECLAIM_CHILD_PROCESSES */
 
 #ifdef AP_MPM_WANT_WAIT_OR_TIMEOUT