You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ic...@apache.org on 2019/08/09 11:59:15 UTC
svn commit: r1864787 - in /httpd/httpd/branches/2.4.x: CHANGES STATUS
modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Author: icing
Date: Fri Aug 9 11:59:15 2019
New Revision: 1864787
URL: http://svn.apache.org/viewvc?rev=1864787&view=rev
Log:
Merge of r1864693,1864695,1864703 from trunk;
*) mod_proxy: Improve XSRF/XSS protection. [Joe Orton]
Modified:
httpd/httpd/branches/2.4.x/CHANGES
httpd/httpd/branches/2.4.x/STATUS
httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_balancer.c
httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1864787&r1=1864786&r2=1864787&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Fri Aug 9 11:59:15 2019
@@ -1,6 +1,8 @@
-*- coding: utf-8 -*-
Changes with Apache 2.4.41
+ *) mod_proxy: Improve XSRF/XSS protection. [Joe Orton]
+
*) mod_session: Introduce SessionExpiryUpdateInterval which allows to
configure the session/cookie expiry's update interval. PR 57300.
[Paul Spangler <paul.spangler ni.com>]
Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1864787&r1=1864786&r2=1864787&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Fri Aug 9 11:59:15 2019
@@ -150,13 +150,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
reworked, etc.. I would like to avoid a patch that is clearly wrong for some reviewer sitting here for
months/years without any action item :)
- *) mod_proxy: Improve XSRF/XSS protection.
- trunk patch: http://svn.apache.org/r1864693
- http://svn.apache.org/r1864695
- http://svn.apache.org/r1864703
- 2.4.x patch: http://people.apache.org/~jorton/ap_pb_xsrf.patch
- +1: jorton, covener, icing
-
PATCHES/ISSUES THAT ARE BEING WORKED
[ New entries should be added at the START of the list ]
Modified: httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_balancer.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_balancer.c?rev=1864787&r1=1864786&r2=1864787&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_balancer.c (original)
+++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_balancer.c Fri Aug 9 11:59:15 2019
@@ -1095,6 +1095,18 @@ static void push2table(const char *input
}
}
+/* Returns non-zero if the Referer: header value passed matches the
+ * host of the request. */
+static int safe_referer(request_rec *r, const char *ref)
+{
+ apr_uri_t uri;
+
+ if (apr_uri_parse(r->pool, ref, &uri) || !uri.hostname)
+ return 0;
+
+ return strcmp(uri.hostname, ap_get_server_name(r)) == 0;
+}
+
/* Manages the loadfactors and member status
* The balancer, worker and nonce are obtained from
* the request args (?b=...&w=...&nonce=....).
@@ -1113,7 +1125,7 @@ static int balancer_handler(request_rec
apr_table_t *params;
int i, n;
int ok2change = 1;
- const char *name;
+ const char *name, *ref;
const char *action;
apr_status_t rv;
@@ -1169,6 +1181,16 @@ static int balancer_handler(request_rec
buf[len] = '\0';
push2table(buf, params, NULL, r->pool);
}
+
+ /* Ignore parameters if this looks like XSRF */
+ ref = apr_table_get(r->headers_in, "Referer");
+ if (apr_table_elts(params)
+ && (!ref || !safe_referer(r, ref))) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10187)
+ "ignoring params in balancer-manager cross-site access");
+ apr_table_clear(params);
+ }
+
if ((name = apr_table_get(params, "b")))
bsel = ap_proxy_get_balancer(r->pool, conf,
apr_pstrcat(r->pool, BALANCER_PREFIX, name, NULL), 0);
@@ -1440,7 +1462,7 @@ static int balancer_handler(request_rec
/* Start proxy_balancer */
ap_rvputs(r, " <httpd:name>", balancer->s->name, "</httpd:name>\n", NULL);
if (*balancer->s->sticky) {
- ap_rvputs(r, " <httpd:stickysession>", balancer->s->sticky,
+ ap_rvputs(r, " <httpd:stickysession>", ap_escape_html(r->pool, balancer->s->sticky),
"</httpd:stickysession>\n", NULL);
ap_rprintf(r,
" <httpd:nofailover>%s</httpd:nofailover>\n",
@@ -1650,10 +1672,10 @@ static int balancer_handler(request_rec
for (i = 0; i < conf->balancers->nelts; i++) {
ap_rputs("<hr />\n<h3>LoadBalancer Status for ", r);
- ap_rvputs(r, "<a href='", ap_escape_uri(r->pool, r->uri), "?b=",
+ ap_rvputs(r, "<a href=\"", ap_escape_uri(r->pool, r->uri), "?b=",
balancer->s->name + sizeof(BALANCER_PREFIX) - 1,
"&nonce=", balancer->s->nonce,
- "'>", NULL);
+ "\">", NULL);
ap_rvputs(r, balancer->s->name, "</a> [",balancer->s->sname, "]</h3>\n", NULL);
ap_rputs("\n\n<table><tr>"
"<th>MaxMembers</th><th>StickySession</th><th>DisableFailover</th><th>Timeout</th><th>FailoverAttempts</th><th>Method</th>"
@@ -1664,11 +1686,11 @@ static int balancer_handler(request_rec
balancer->max_workers - (int)storage->num_free_slots(balancer->wslot));
if (*balancer->s->sticky) {
if (strcmp(balancer->s->sticky, balancer->s->sticky_path)) {
- ap_rvputs(r, "<td>", balancer->s->sticky, " | ",
- balancer->s->sticky_path, NULL);
+ ap_rvputs(r, "<td>", ap_escape_html(r->pool, balancer->s->sticky), " | ",
+ ap_escape_html(r->pool, balancer->s->sticky_path), NULL);
}
else {
- ap_rvputs(r, "<td>", balancer->s->sticky, NULL);
+ ap_rvputs(r, "<td>", ap_escape_html(r->pool, balancer->s->sticky), NULL);
}
}
else {
@@ -1703,12 +1725,12 @@ static int balancer_handler(request_rec
for (n = 0; n < balancer->workers->nelts; n++) {
char fbuf[50];
worker = *workers;
- ap_rvputs(r, "<tr>\n<td><a href='",
+ ap_rvputs(r, "<tr>\n<td><a href=\"",
ap_escape_uri(r->pool, r->uri), "?b=",
balancer->s->name + sizeof(BALANCER_PREFIX) - 1, "&w=",
ap_escape_uri(r->pool, worker->s->name),
"&nonce=", balancer->s->nonce,
- "'>", NULL);
+ "\">", NULL);
ap_rvputs(r, (*worker->s->uds_path ? "<i>" : ""), ap_proxy_worker_name(r->pool, worker),
(*worker->s->uds_path ? "</i>" : ""), "</a></td>", NULL);
ap_rvputs(r, "<td>", ap_escape_html(r->pool, worker->s->route),
@@ -1730,7 +1752,7 @@ static int balancer_handler(request_rec
ap_rprintf(r, "<td>%" APR_TIME_T_FMT "ms</td>", apr_time_as_msec(worker->s->interval));
ap_rprintf(r, "<td>%d (%d)</td>", worker->s->passes,worker->s->pcount);
ap_rprintf(r, "<td>%d (%d)</td>", worker->s->fails, worker->s->fcount);
- ap_rprintf(r, "<td>%s</td>", worker->s->hcuri);
+ ap_rprintf(r, "<td>%s</td>", ap_escape_html(r->pool, worker->s->hcuri));
ap_rprintf(r, "<td>%s", worker->s->hcexpr);
}
ap_rputs("</td></tr>\n", r);
@@ -1747,20 +1769,20 @@ static int balancer_handler(request_rec
if (wsel && bsel) {
ap_rputs("<h3>Edit worker settings for ", r);
ap_rvputs(r, (*wsel->s->uds_path?"<i>":""), ap_proxy_worker_name(r->pool, wsel), (*wsel->s->uds_path?"</i>":""), "</h3>\n", NULL);
- ap_rputs("<form method='POST' enctype='application/x-www-form-urlencoded' action='", r);
- ap_rvputs(r, ap_escape_uri(r->pool, action), "'>\n", NULL);
+ ap_rputs("<form method='POST' enctype='application/x-www-form-urlencoded' action=\"", r);
+ ap_rvputs(r, ap_escape_uri(r->pool, action), "\">\n", NULL);
ap_rputs("<table><tr><td>Load factor:</td><td><input name='w_lf' id='w_lf' type=text ", r);
ap_rprintf(r, "value='%.2f'></td></tr>\n", (float)(wsel->s->lbfactor)/100.0);
ap_rputs("<tr><td>LB Set:</td><td><input name='w_ls' id='w_ls' type=text ", r);
ap_rprintf(r, "value='%d'></td></tr>\n", wsel->s->lbset);
ap_rputs("<tr><td>Route:</td><td><input name='w_wr' id='w_wr' type=text ", r);
- ap_rvputs(r, "value='", ap_escape_html(r->pool, wsel->s->route),
+ ap_rvputs(r, "value=\"", ap_escape_html(r->pool, wsel->s->route),
NULL);
- ap_rputs("'></td></tr>\n", r);
+ ap_rputs("\"></td></tr>\n", r);
ap_rputs("<tr><td>Route Redirect:</td><td><input name='w_rr' id='w_rr' type=text ", r);
- ap_rvputs(r, "value='", ap_escape_html(r->pool, wsel->s->redirect),
+ ap_rvputs(r, "value=\"", ap_escape_html(r->pool, wsel->s->redirect),
NULL);
- ap_rputs("'></td></tr>\n", r);
+ ap_rputs("\"></td></tr>\n", r);
ap_rputs("<tr><td>Status:</td>", r);
ap_rputs("<td><table><tr>"
"<th>Ignore Errors</th>"
@@ -1805,15 +1827,15 @@ static int balancer_handler(request_rec
ap_rprintf(r, "<tr><td>Fails trigger)</td><td><input name='w_hf' id='w_hf' type='text'"
"value='%d'></td></tr>\n", wsel->s->fails);
ap_rprintf(r, "<tr><td>HC uri</td><td><input name='w_hu' id='w_hu' type='text'"
- "value='%s'></td></tr>\n", ap_escape_html(r->pool, wsel->s->hcuri));
+ "value=\"%s\"></td></tr>\n", ap_escape_html(r->pool, wsel->s->hcuri));
ap_rputs("</table>\n</td></tr>\n", r);
}
ap_rputs("<tr><td colspan='2'><input type=submit value='Submit'></td></tr>\n", r);
ap_rvputs(r, "</table>\n<input type=hidden name='w' id='w' ", NULL);
- ap_rvputs(r, "value='", ap_escape_uri(r->pool, wsel->s->name), "'>\n", NULL);
+ ap_rvputs(r, "value=\"", ap_escape_uri(r->pool, wsel->s->name), "\">\n", NULL);
ap_rvputs(r, "<input type=hidden name='b' id='b' ", NULL);
- ap_rvputs(r, "value='", bsel->s->name + sizeof(BALANCER_PREFIX) - 1,
- "'>\n", NULL);
+ ap_rvputs(r, "value=\"", ap_escape_html(r->pool, bsel->s->name + sizeof(BALANCER_PREFIX) - 1),
+ "\">\n", NULL);
ap_rvputs(r, "<input type=hidden name='nonce' id='nonce' value='",
bsel->s->nonce, "'>\n", NULL);
ap_rputs("</form>\n", r);
@@ -1823,9 +1845,9 @@ static int balancer_handler(request_rec
const ap_list_provider_names_t *pname;
int i;
ap_rputs("<h3>Edit balancer settings for ", r);
- ap_rvputs(r, bsel->s->name, "</h3>\n", NULL);
- ap_rputs("<form method='POST' enctype='application/x-www-form-urlencoded' action='", r);
- ap_rvputs(r, ap_escape_uri(r->pool, action), "'>\n", NULL);
+ ap_rvputs(r, ap_escape_html(r->pool, bsel->s->name), "</h3>\n", NULL);
+ ap_rputs("<form method='POST' enctype='application/x-www-form-urlencoded' action=\"", r);
+ ap_rvputs(r, ap_escape_uri(r->pool, action), "\">\n", NULL);
ap_rputs("<table>\n", r);
provs = ap_list_provider_names(r->pool, PROXY_LBMETHOD, "0");
if (provs) {
@@ -1849,13 +1871,13 @@ static int balancer_handler(request_rec
ap_rputs("</tr>\n", r);
ap_rputs("<tr><td>Sticky Session:</td><td><input name='b_ss' id='b_ss' size=64 type=text ", r);
if (strcmp(bsel->s->sticky, bsel->s->sticky_path)) {
- ap_rvputs(r, "value ='", bsel->s->sticky, " | ",
- bsel->s->sticky_path, NULL);
+ ap_rvputs(r, "value =\"", ap_escape_html(r->pool, bsel->s->sticky), " | ",
+ ap_escape_html(r->pool, bsel->s->sticky_path), NULL);
}
else {
- ap_rvputs(r, "value ='", bsel->s->sticky, NULL);
+ ap_rvputs(r, "value =\"", ap_escape_html(r->pool, bsel->s->sticky), NULL);
}
- ap_rputs("'> (Use '-' to delete)</td></tr>\n", r);
+ ap_rputs("\"> (Use '-' to delete)</td></tr>\n", r);
if (storage->num_free_slots(bsel->wslot) != 0) {
ap_rputs("<tr><td>Add New Worker:</td><td><input name='b_nwrkr' id='b_nwrkr' size=32 type=text>"
" Are you sure? <input name='b_wyes' id='b_wyes' type=checkbox value='1'>"
@@ -1863,8 +1885,8 @@ static int balancer_handler(request_rec
}
ap_rputs("<tr><td colspan=2><input type=submit value='Submit'></td></tr>\n", r);
ap_rvputs(r, "</table>\n<input type=hidden name='b' id='b' ", NULL);
- ap_rvputs(r, "value='", bsel->s->name + sizeof(BALANCER_PREFIX) - 1,
- "'>\n", NULL);
+ ap_rvputs(r, "value=\"", ap_escape_html(r->pool, bsel->s->name + sizeof(BALANCER_PREFIX) - 1),
+ "\">\n", NULL);
ap_rvputs(r, "<input type=hidden name='nonce' id='nonce' value='",
bsel->s->nonce, "'>\n", NULL);
ap_rputs("</form>\n", r);
Modified: httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c?rev=1864787&r1=1864786&r2=1864787&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c Fri Aug 9 11:59:15 2019
@@ -1252,10 +1252,11 @@ PROXY_DECLARE(apr_status_t) ap_proxy_sha
if (*balancer->s->nonce == PROXY_UNSET_NONCE) {
char nonce[APR_UUID_FORMATTED_LENGTH + 1];
apr_uuid_t uuid;
- /* Retrieve a UUID and store the nonce for the lifetime of
- * the process.
- */
- apr_uuid_get(&uuid);
+
+ /* Generate a pseudo-UUID from the PRNG to use as a nonce for
+ * the lifetime of the process. uuid.data is a char array so
+ * this is an adequate substitute for apr_uuid_get(). */
+ ap_random_insecure_bytes(uuid.data, sizeof uuid.data);
apr_uuid_format(nonce, &uuid);
rv = PROXY_STRNCPY(balancer->s->nonce, nonce);
}