From: William A. Rowe Jr Date: Tue, 20 Sep 2005 18:14:29 +0000 (+0000) Subject: Backport TraceEnable option, correcting RFC violation by mod_proxy as this X-Git-Tag: 2.0.55~39 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=073e44a68b7021fcf03972a9e9756620f551e3d7;p=thirdparty%2Fapache%2Fhttpd.git Backport TraceEnable option, correcting RFC violation by mod_proxy as this now drops any proxied TRACE request which tries to pass a body, unless the user explicitly forces 'TraceEnable extended'. Per colm; removed \n's from error_notes, docs coming next. Reviewed by: jimj, colm git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.0.x@290502 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index c35de13ec94..dc3d2617574 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,12 @@ -*- coding: utf-8 -*- Changes with Apache 2.0.55 + *) Added TraceEnable [on|off|extended] per-server directive to alter + the behavior of the TRACE method. This addresses a flaw in proxy + conformance to RFC 2616 - previously the proxy server would accept + a TRACE request body although the RFC prohibited it. The default + remains 'TraceEnable on'. [William Rowe] + *) Add ap_log_cerror() for logging messages associated with particular client connections. [Jeff Trawick] diff --git a/STATUS b/STATUS index 9ea9aa75f68..243263b88f4 100644 --- a/STATUS +++ b/STATUS @@ -211,18 +211,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: +1: jorton, wrowe wrowe cautions to backport to 2.2.x branch as well. - *) Correct RFC 2616 non-compliance by refusing to proxy a request body - in a TRACE request, unless TraceEnable extended is configured. - Introduces TraceEnable [on|off|extended] to give the administrator - full control of TRACE request handling. RFC 2616 does NOT require - TRACE (although to disable remains silly). Current patch at; - http://people.apache.org/~wrowe/httpd-2.0-trace.patch - +1 wrowe, jimjag, colm - colm notes: There are some \n's in apr_table_setn calls that are - not consistent with other calls to apr_table_setn. - There is no documentation for TraceEnable in trunk to - backport, shouldn't release while still undocumented. - *) mod_headers: Support {...}s tag for SSL variable lookup. http://www.apache.org/~jorton/mod_headers-2.0-ssl.diff +1: jorton, trawick diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 3163d31db36..f6da660774f 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -84,6 +84,7 @@ * 20020903.9 (2.0.51-dev) create pcommands and initialize arrays before * calling ap_setup_prelinked_modules * 20020903.10 (2.0.55-dev) add ap_log_cerror() + * 20020903.11 (2.0.55-dev) added trace_enable to core_server_config */ #define MODULE_MAGIC_COOKIE 0x41503230UL /* "AP20" */ @@ -91,7 +92,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20020903 #endif -#define MODULE_MAGIC_NUMBER_MINOR 10 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 11 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/include/http_core.h b/include/http_core.h index 9028ff012bb..dfcb0fc2569 100644 --- a/include/http_core.h +++ b/include/http_core.h @@ -549,6 +549,14 @@ typedef struct { /* recursion backstopper */ int redirect_limit; /* maximum number of internal redirects */ int subreq_limit; /* maximum nesting level of subrequests */ + + /* TRACE control */ +#define AP_TRACE_UNSET -1 +#define AP_TRACE_DISABLE 0 +#define AP_TRACE_ENABLE 1 +#define AP_TRACE_EXTENDED 2 + int trace_enable; + } core_server_config; /* for AddOutputFiltersByType in core.c */ diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c index 8b584532559..0ff727c6021 100644 --- a/modules/http/http_protocol.c +++ b/modules/http/http_protocol.c @@ -1321,6 +1321,9 @@ static char *make_allow(request_rec *r) apr_int64_t mask; apr_array_header_t *allow = apr_array_make(r->pool, 10, sizeof(char *)); apr_hash_index_t *hi = apr_hash_first(r->pool, methods_registry); + /* For TRACE below */ + core_server_config *conf = + ap_get_module_config(r->server->module_config, &core_module); mask = r->allowed_methods->method_mask; @@ -1338,8 +1341,9 @@ static char *make_allow(request_rec *r) } } - /* TRACE is always allowed */ - *(const char **)apr_array_push(allow) = "TRACE"; + /* TRACE is tested on a per-server basis */ + if (conf->trace_enable != AP_TRACE_DISABLE) + *(const char **)apr_array_push(allow) = "TRACE"; list = apr_array_pstrcat(r->pool, allow, ','); @@ -1364,9 +1368,16 @@ static char *make_allow(request_rec *r) AP_DECLARE_NONSTD(int) ap_send_http_trace(request_rec *r) { + core_server_config *conf; int rv; - apr_bucket_brigade *b; + apr_bucket_brigade *bb; header_struct h; + apr_bucket *b; + int body; + char *bodyread, *bodyoff; + apr_size_t bodylen = 0; + apr_size_t bodybuf; + long res; if (r->method_number != M_TRACE) { return DECLINED; @@ -1376,23 +1387,87 @@ AP_DECLARE_NONSTD(int) ap_send_http_trace(request_rec *r) while (r->prev) { r = r->prev; } + conf = (core_server_config *)ap_get_module_config(r->server->module_config, + &core_module); + + if (conf->trace_enable == AP_TRACE_DISABLE) { + apr_table_setn(r->notes, "error-notes", + "TRACE denied by server configuration"); + return HTTP_FORBIDDEN; + } - if ((rv = ap_setup_client_block(r, REQUEST_NO_BODY))) { + if (conf->trace_enable == AP_TRACE_EXTENDED) + /* XX should be = REQUEST_CHUNKED_PASS */ + body = REQUEST_CHUNKED_DECHUNK; + else + body = REQUEST_NO_BODY; + + if ((rv = ap_setup_client_block(r, body))) { + if (rv == HTTP_REQUEST_ENTITY_TOO_LARGE) + apr_table_setn(r->notes, "error-notes", + "TRACE with a request body is not allowed"); return rv; } + if (ap_should_client_block(r)) { + + if (r->remaining > 0) { + if (r->remaining > 65536) { + apr_table_setn(r->notes, "error-notes", + "Extended TRACE request bodies cannot exceed 64k"); + return HTTP_REQUEST_ENTITY_TOO_LARGE; + } + /* always 32 extra bytes to catch chunk header exceptions */ + bodybuf = (apr_size_t)r->remaining + 32; + } + else { + /* Add an extra 8192 for chunk headers */ + bodybuf = 73730; + } + + bodyoff = bodyread = apr_palloc(r->pool, bodybuf); + + /* only while we have enough for a chunked header */ + while ((!bodylen || bodybuf >= 32) && + (res = ap_get_client_block(r, bodyoff, bodybuf)) > 0) { + bodylen += res; + bodybuf -= res; + bodyoff += res; + } + if (res > 0 && bodybuf < 32) { + /* discard_rest_of_request_body into our buffer */ + while (ap_get_client_block(r, bodyread, bodylen) > 0) + ; + apr_table_setn(r->notes, "error-notes", + "Extended TRACE request bodies cannot exceed 64k"); + return HTTP_REQUEST_ENTITY_TOO_LARGE; + } + + if (res < 0) { + return HTTP_BAD_REQUEST; + } + } + ap_set_content_type(r, "message/http"); /* Now we recreate the request, and echo it back */ - b = apr_brigade_create(r->pool, r->connection->bucket_alloc); - apr_brigade_putstrs(b, NULL, NULL, r->the_request, CRLF, NULL); + bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); + apr_brigade_putstrs(bb, NULL, NULL, r->the_request, CRLF, NULL); h.pool = r->pool; - h.bb = b; + h.bb = bb; apr_table_do((int (*) (void *, const char *, const char *)) form_header_field, (void *) &h, r->headers_in, NULL); - apr_brigade_puts(b, NULL, NULL, CRLF); - ap_pass_brigade(r->output_filters, b); + apr_brigade_puts(bb, NULL, NULL, CRLF); + + /* If configured to accept a body, echo the body */ + if (bodylen) { + b = apr_bucket_pool_create(bodyread, bodylen, + r->pool, bb->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(bb, b); + } + + ap_pass_brigade(r->output_filters, bb); return DONE; } diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c index e9961d6cb19..e22b4b7ac21 100644 --- a/modules/proxy/mod_proxy.c +++ b/modules/proxy/mod_proxy.c @@ -350,6 +350,42 @@ static int proxy_handler(request_rec *r) apr_table_set(r->headers_in, "Max-Forwards", apr_psprintf(r->pool, "%ld", (maxfwd > 0) ? maxfwd : 0)); + if (r->method_number == M_TRACE) { + core_server_config *coreconf = (core_server_config *) + ap_get_module_config(sconf, &core_module); + + if (coreconf->trace_enable == AP_TRACE_DISABLE) + { + /* Allow "error-notes" string to be printed by ap_send_error_response() + * Note; this goes nowhere, canned error response need an overhaul. + */ + apr_table_setn(r->notes, "error-notes", + "TRACE forbidden by server configuration"); + apr_table_setn(r->notes, "verbose-error-to", "*"); + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "proxy: TRACE forbidden by server configuration"); + return HTTP_FORBIDDEN; + } + + /* Can't test ap_should_client_block, we aren't ready to send + * the client a 100 Continue response till the connection has + * been established + */ + if (coreconf->trace_enable != AP_TRACE_EXTENDED + && (r->read_length || r->read_chunked || r->remaining)) + { + /* Allow "error-notes" string to be printed by ap_send_error_response() + * Note; this goes nowhere, canned error response need an overhaul. + */ + apr_table_setn(r->notes, "error-notes", + "TRACE with request body is not allowed"); + apr_table_setn(r->notes, "verbose-error-to", "*"); + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "proxy: TRACE with request body is not allowed"); + return HTTP_REQUEST_ENTITY_TOO_LARGE; + } + } + url = r->filename + 6; p = strchr(url, ':'); if (p == NULL) diff --git a/server/core.c b/server/core.c index ee331ca26e3..70f74c58237 100644 --- a/server/core.c +++ b/server/core.c @@ -458,6 +458,8 @@ static void *create_core_server_config(apr_pool_t *a, server_rec *s) conf->redirect_limit = 0; /* 0 == unset */ conf->subreq_limit = 0; + conf->trace_enable = AP_TRACE_UNSET; + return (void *)conf; } @@ -489,6 +491,10 @@ static void *merge_core_server_configs(apr_pool_t *p, void *basev, void *virtv) ? virt->subreq_limit : base->subreq_limit; + conf->trace_enable = (virt->trace_enable != AP_TRACE_UNSET) + ? virt->trace_enable + : base->trace_enable; + return conf; } @@ -1587,7 +1593,7 @@ AP_CORE_DECLARE_NONSTD(const char *) ap_limit_section(cmd_parms *cmd, methnum = ap_method_number_of(method); if (methnum == M_TRACE && !tog) { - return "TRACE cannot be controlled by "; + return "TRACE cannot be controlled by , see TraceEnable"; } else if (methnum == M_INVALID) { /* method has not been registered yet, but resorce restriction @@ -3113,6 +3119,28 @@ static apr_status_t emulate_sendfile(core_net_rec *c, apr_file_t *fd, return rv; } +static const char *set_trace_enable(cmd_parms *cmd, void *dummy, + const char *arg1) +{ + core_server_config *conf = ap_get_module_config(cmd->server->module_config, + &core_module); + + if (strcasecmp(arg1, "on") == 0) { + conf->trace_enable = AP_TRACE_ENABLE; + } + else if (strcasecmp(arg1, "off") == 0) { + conf->trace_enable = AP_TRACE_DISABLE; + } + else if (strcasecmp(arg1, "extended") == 0) { + conf->trace_enable = AP_TRACE_EXTENDED; + } + else { + return "TraceEnable must be one of 'on', 'off', or 'extended'"; + } + + return NULL; +} + /* Note --- ErrorDocument will now work from .htaccess files. * The AllowOverride of Fileinfo allows webmasters to turn it off */ @@ -3338,6 +3366,8 @@ AP_INIT_TAKE1("MaxMemFree", ap_mpm_set_max_mem_free, NULL, RSRC_CONF, AP_INIT_TAKE1("EnableExceptionHook", ap_mpm_set_exception_hook, NULL, RSRC_CONF, "Controls whether exception hook may be called after a crash"), #endif +AP_INIT_TAKE1("TraceEnable", set_trace_enable, NULL, RSRC_CONF, + "'on' (default), 'off' or 'extended' to trace request body content"), { NULL } };