From: Jim Jagielski <jim@apache.org>
Date: Thu, 5 Jun 2008 10:01:30 +0000 (+0000)
Subject: Merge r661666 from trunk:
X-Git-Tag: 2.2.9~33
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b74f25bceea9f81abdb1856cb1fbe63734c6b13c;p=thirdparty%2Fapache%2Fhttpd.git

Merge r661666 from trunk:

Prevent CSRF attacks against the balancer-manager (CVE-2007-6420)

* modules/proxy/mod_proxy_balancer.c (balancer_init): New function.
  (balancer_handler): Place a nonce in the form output, and check that
  the submitted form data includes that nonce.
  (ap_proxy_balancer_register_hook): Register the new post_config hook.

Submitted by: jorton
Reviewed by: jim


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

diff --git a/CHANGES b/CHANGES
index a7d5fd49447..604da862d45 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.2.9
 
+  *) SECURITY: CVE-2007-6420 (cve.mitre.org)
+     mod_proxy_balancer: Prevent CSRF attacks against the balancer-manager
+     interface.  [Joe Orton]
+
   *) mod_unique_id: Fix timestamp value in UNIQUE_ID.
      PR 37064 [Kobayashi <kobayashi firstserver.co.jp>]
 
diff --git a/STATUS b/STATUS
index df4c500def4..d9a494e1d06 100644
--- a/STATUS
+++ b/STATUS
@@ -84,13 +84,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
- * proxy_balancer: Prevent CSRF attacks against the balancer-manager
-   (CVE-2007-6420)
-   Trunk version of patch:
-         http://svn.apache.org/viewvc?view=rev&revision=661666
-   Backport version for 2.2.x of patch:
-         Trunk version of patch works (minus CHANGES conflict)
-    +1: jorton, jim, wrowe
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c
index d2ae88bb301..bb07a448c79 100644
--- a/modules/proxy/mod_proxy_balancer.c
+++ b/modules/proxy/mod_proxy_balancer.c
@@ -23,9 +23,12 @@
 #include "ap_mpm.h"
 #include "apr_version.h"
 #include "apr_hooks.h"
+#include "apr_uuid.h"
 
 module AP_MODULE_DECLARE_DATA proxy_balancer_module;
 
+static apr_uuid_t balancer_nonce;
+
 static int proxy_balancer_canon(request_rec *r, char *url)
 {
     char *host, *path, *search;
@@ -589,6 +592,27 @@ static void recalc_factors(proxy_balancer *balancer)
     }
 }
 
+/* post_config hook: */
+static int balancer_init(apr_pool_t *p, apr_pool_t *plog,
+                         apr_pool_t *ptemp, server_rec *s)
+{
+    void *data;
+    const char *userdata_key = "mod_proxy_balancer_init";
+
+    /* balancer_init() will be called twice during startup.  So, only
+     * set up the static data the second time through. */
+    apr_pool_userdata_get(&data, userdata_key, s->process->pool);
+    if (!data) {
+        apr_pool_userdata_set((const void *)1, userdata_key,
+                               apr_pool_cleanup_null, s->process->pool);
+        return OK;
+    }
+
+    apr_uuid_get(&balancer_nonce);
+
+    return OK;
+}
+
 /* Manages the loadfactors and member status
  */
 static int balancer_handler(request_rec *r)
@@ -602,6 +626,9 @@ static int balancer_handler(request_rec *r)
     int access_status;
     int i, n;
     const char *name;
+    char nonce[APR_UUID_FORMATTED_LENGTH + 1];
+
+    apr_uuid_format(nonce, &balancer_nonce);
 
     /* is this for us? */
     if (strcmp(r->handler, "balancer-manager"))
@@ -631,6 +658,14 @@ static int balancer_handler(request_rec *r)
                 return HTTP_BAD_REQUEST;
         }
     }
+    
+    /* Check that the supplied nonce matches this server's nonce;
+     * otherwise ignore all parameters, to prevent a CSRF attack. */
+    if ((name = apr_table_get(params, "nonce")) == NULL 
+        || strcmp(nonce, name) != 0) {
+        apr_table_clear(params);
+    }
+
     if ((name = apr_table_get(params, "b")))
         bsel = ap_proxy_get_balancer(r->pool, conf,
             apr_pstrcat(r->pool, "balancer://", name, NULL));
@@ -762,6 +797,7 @@ static int balancer_handler(request_rec *r)
                 ap_rvputs(r, "<tr>\n<td><a href=\"", r->uri, "?b=",
                           balancer->name + sizeof("balancer://") - 1, "&w=",
                           ap_escape_uri(r->pool, worker->name),
+                          "&nonce=", nonce, 
                           "\">", NULL);
                 ap_rvputs(r, worker->name, "</a></td>", NULL);
                 ap_rvputs(r, "<td>", ap_escape_html(r->pool, worker->s->route),
@@ -825,6 +861,8 @@ static int balancer_handler(request_rec *r)
             ap_rvputs(r, "<input type=hidden name=\"b\" ", NULL);
             ap_rvputs(r, "value=\"", bsel->name + sizeof("balancer://") - 1,
                       "\">\n</form>\n", NULL);
+            ap_rvputs(r, "<input type=hidden name=\"nonce\" value=\"", nonce, "\">\n",
+                      NULL);
             ap_rputs("<hr />\n", r);
         }
         ap_rputs(ap_psignature("",r), r);
@@ -1063,6 +1101,7 @@ static void ap_proxy_balancer_register_hook(apr_pool_t *p)
      */
     static const char *const aszPred[] = { "mpm_winnt.c", "mod_proxy.c", NULL};
      /* manager handler */
+    ap_hook_post_config(balancer_init, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_handler(balancer_handler, NULL, NULL, APR_HOOK_FIRST);
     ap_hook_child_init(child_init, aszPred, NULL, APR_HOOK_MIDDLE);
     proxy_hook_pre_request(proxy_balancer_pre_request, NULL, NULL, APR_HOOK_FIRST);