diff --git a/CHANGES b/CHANGES index d1b782f47c..4b414c0b5b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,13 @@ -*- coding: utf-8 -*- Changes with Apache 2.5.0 + *) SECURITY: CVE-2013-5704 (cve.mitre.org) + core: HTTP trailers could be used to replace HTTP headers + late during request processing, potentially undoing or + otherwise confusing modules that examined or modified + request headers earlier. Adds "MergeTrailers" directive to restore + legacy behavior. [Edward Lu, Yann Ylavic, Joe Orton, Eric Covener] + *) http_protocol: fix logic in ap_method_list_(add|remove) in order: - to correctly reset bits - not to modify the 'method_mask' bitfield unnecessarily diff --git a/docs/manual/mod/core.xml b/docs/manual/mod/core.xml index 411af81887..8f637e2cf8 100644 --- a/docs/manual/mod/core.xml +++ b/docs/manual/mod/core.xml @@ -4535,4 +4535,23 @@ for external processing, e.g. to a CGI script.

+ +MergeTrailers +Determins whether trailers are merged into headers +MergeTrailers [on|off] +MergeTrailers off +server configvirtual host +2.4.10 and later + + +

This directive controls whether HTTP trailers are copied into the + internal representation of HTTP headers. This mergeing occurs when the + request body has been completely consumed, long after most header + processing would have a chance to examine or modify request headers.

+

This option is provided for compatibility with releases prior to 2.4.10, + where trailers were always merged.

+
+
+ + diff --git a/docs/manual/mod/mod_log_config.xml b/docs/manual/mod/mod_log_config.xml index bbd7f642a4..df07a75b1e 100644 --- a/docs/manual/mod/mod_log_config.xml +++ b/docs/manual/mod/mod_log_config.xml @@ -261,6 +261,14 @@ cannot be zero. This is the combination of %I and %O. You need to enable mod_logio to use this. + %{VARNAME}^ti + The contents of VARNAME: trailer line(s) + in the request sent to the server. + + %{VARNAME}^to + The contents of VARNAME: trailer line(s) + in the response sent from the server. +
Modifiers diff --git a/include/ap_mmn.h b/include/ap_mmn.h index ee60c646f1..ea496962c1 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -468,6 +468,7 @@ Added ap_proxy_define_match_worker(). * 20140627.3 (2.5.0-dev) Add ap_copy_scoreboard_worker() * 20140627.4 (2.5.0-dev) Added ap_parse_token_list_strict() to httpd.h. + * 20140627.5 (2.5.0-dev) Add r->trailers_{in,out} */ #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */ @@ -475,7 +476,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20140627 #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/http_core.h b/include/http_core.h index 337859ba05..3ed37939c4 100644 --- a/include/http_core.h +++ b/include/http_core.h @@ -669,6 +669,10 @@ typedef struct { #define AP_TRACE_ENABLE 1 #define AP_TRACE_EXTENDED 2 int trace_enable; +#define AP_MERGE_TRAILERS_UNSET 0 +#define AP_MERGE_TRAILERS_ENABLE 1 +#define AP_MERGE_TRAILERS_DISABLE 2 + int merge_trailers; apr_array_header_t *conn_log_level; diff --git a/include/httpd.h b/include/httpd.h index 77345b2bfc..ba0bc990b9 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -1039,6 +1039,11 @@ struct request_rec { */ apr_sockaddr_t *useragent_addr; char *useragent_ip; + + /** MIME trailer environment from the request */ + apr_table_t *trailers_in; + /** MIME trailer environment from the response */ + apr_table_t *trailers_out; }; /** diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c index d2c618dd62..a9ef037410 100644 --- a/modules/http/http_filters.c +++ b/modules/http/http_filters.c @@ -184,6 +184,48 @@ static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer, return APR_SUCCESS; } +static apr_status_t read_chunked_trailers(http_ctx_t *ctx, ap_filter_t *f, + apr_bucket_brigade *b, int merge) +{ + int rv; + apr_bucket *e; + request_rec *r = f->r; + apr_table_t *saved_headers_in = r->headers_in; + int saved_status = r->status; + + r->status = HTTP_OK; + r->headers_in = r->trailers_in; + apr_table_clear(r->headers_in); + ap_get_mime_headers(r); + + if(r->status == HTTP_OK) { + r->status = saved_status; + e = apr_bucket_eos_create(f->c->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(b, e); + ctx->eos_sent = 1; + rv = APR_SUCCESS; + } + else { + const char *error_notes = apr_table_get(r->notes, + "error-notes"); + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "Error while reading HTTP trailer: %i%s%s", + r->status, error_notes ? ": " : "", + error_notes ? error_notes : ""); + rv = APR_EINVAL; + } + + if(!merge) { + r->headers_in = saved_headers_in; + } + else { + r->headers_in = apr_table_overlay(r->pool, saved_headers_in, + r->trailers_in); + } + + return rv; +} + /* This is the HTTP_INPUT filter for HTTP requests and responses from * proxied servers (mod_proxy). It handles chunked and content-length * bodies. This can only be inserted/used after the headers @@ -192,12 +234,16 @@ static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer, apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes) { + core_server_config *conf; apr_bucket *e; http_ctx_t *ctx = f->ctx; apr_status_t rv; apr_off_t totalread; int again; + conf = (core_server_config *) + ap_get_module_config(f->r->server->module_config, &core_module); + /* just get out of the way of things we don't want. */ if (mode != AP_MODE_READBYTES && mode != AP_MODE_GETLINE) { return ap_get_brigade(f->next, b, mode, block, readbytes); @@ -396,11 +442,10 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, again = 1; /* come around again */ if (ctx->state == BODY_CHUNK_TRAILER) { - ap_get_mime_headers(f->r); - e = apr_bucket_eos_create(f->c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(b, e); - ctx->eos_sent = 1; - return APR_SUCCESS; + /* Treat UNSET as DISABLE - trailers aren't merged by default */ + int merge_trailers = + conf->merge_trailers == AP_MERGE_TRAILERS_ENABLE; + return read_chunked_trailers(ctx, f, b, merge_trailers); } break; diff --git a/modules/http/http_request.c b/modules/http/http_request.c index 796d506e84..cdfec8b568 100644 --- a/modules/http/http_request.c +++ b/modules/http/http_request.c @@ -463,6 +463,7 @@ static request_rec *internal_internal_redirect(const char *new_uri, new->main = r->main; new->headers_in = r->headers_in; + new->trailers_in = r->trailers_in; new->headers_out = apr_table_make(r->pool, 12); if (ap_is_HTTP_REDIRECT(new->status)) { const char *location = apr_table_get(r->headers_out, "Location"); @@ -470,6 +471,7 @@ static request_rec *internal_internal_redirect(const char *new_uri, apr_table_setn(new->headers_out, "Location", location); } new->err_headers_out = r->err_headers_out; + new->trailers_out = apr_table_make(r->pool, 5); new->subprocess_env = rename_original_env(r->pool, r->subprocess_env); new->notes = apr_table_make(r->pool, 5); @@ -583,6 +585,8 @@ AP_DECLARE(void) ap_internal_fast_redirect(request_rec *rr, request_rec *r) r->headers_out); r->err_headers_out = apr_table_overlay(r->pool, rr->err_headers_out, r->err_headers_out); + r->trailers_out = apr_table_overlay(r->pool, rr->trailers_out, + r->trailers_out); r->subprocess_env = apr_table_overlay(r->pool, rr->subprocess_env, r->subprocess_env); diff --git a/modules/loggers/mod_log_config.c b/modules/loggers/mod_log_config.c index 569a748405..75e131eeba 100644 --- a/modules/loggers/mod_log_config.c +++ b/modules/loggers/mod_log_config.c @@ -432,6 +432,12 @@ static const char *log_header_in(request_rec *r, char *a) return ap_escape_logitem(r->pool, apr_table_get(r->headers_in, a)); } +static const char *log_trailer_in(request_rec *r, char *a) +{ + return ap_escape_logitem(r->pool, apr_table_get(r->trailers_in, a)); +} + + static APR_INLINE char *find_multiple_headers(apr_pool_t *pool, const apr_table_t *table, const char *key) @@ -515,6 +521,11 @@ static const char *log_header_out(request_rec *r, char *a) return ap_escape_logitem(r->pool, cp); } +static const char *log_trailer_out(request_rec *r, char *a) +{ + return ap_escape_logitem(r->pool, apr_table_get(r->trailers_out, a)); +} + static const char *log_note(request_rec *r, char *a) { return ap_escape_logitem(r->pool, apr_table_get(r->notes, a)); @@ -1737,6 +1748,9 @@ static int log_pre_config(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp) log_pfn_register(p, "U", log_request_uri, 1); log_pfn_register(p, "s", log_status, 1); log_pfn_register(p, "R", log_handler, 1); + + log_pfn_register(p, "^ti", log_trailer_in, 0); + log_pfn_register(p, "^to", log_trailer_out, 0); } /* reset to default conditions */ diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 5f696691ae..91d14f5a98 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -1003,8 +1003,11 @@ static request_rec *make_fake_req(conn_rec *c, request_rec *r) rp->status = HTTP_OK; rp->headers_in = apr_table_make(pool, 50); + rp->trailers_in = apr_table_make(pool, 5); + rp->subprocess_env = apr_table_make(pool, 50); rp->headers_out = apr_table_make(pool, 12); + rp->trailers_out = apr_table_make(pool, 5); rp->err_headers_out = apr_table_make(pool, 5); rp->notes = apr_table_make(pool, 5); @@ -1085,6 +1088,7 @@ static void ap_proxy_read_headers(request_rec *r, request_rec *rr, psc = (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module); r->headers_out = apr_table_make(r->pool, 20); + r->trailers_out = apr_table_make(r->pool, 5); *pread_len = 0; /* @@ -1215,6 +1219,14 @@ apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n, request_rec #define AP_MAX_INTERIM_RESPONSES 10 #endif +static int add_trailers(void *data, const char *key, const char *val) +{ + if (val) { + apr_table_add((apr_table_t*)data, key, val); + } + return 1; +} + static int ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, proxy_conn_rec **backend_ptr, proxy_worker *worker, @@ -1738,6 +1750,12 @@ int ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, /* next time try a non-blocking read */ mode = APR_NONBLOCK_READ; + if (!apr_is_empty_table(backend->r->trailers_in)) { + apr_table_do(add_trailers, r->trailers_out, + backend->r->trailers_in, NULL); + apr_table_clear(backend->r->trailers_in); + } + apr_brigade_length(bb, 0, &readbytes); backend->worker->s->read += readbytes; #if DEBUGGING diff --git a/server/core.c b/server/core.c index 5032bc796b..6a62c03e00 100644 --- a/server/core.c +++ b/server/core.c @@ -553,6 +553,10 @@ static void *merge_core_server_configs(apr_pool_t *p, void *basev, void *virtv) } } + conf->merge_trailers = (virt->merge_trailers != AP_MERGE_TRAILERS_UNSET) + ? virt->merge_trailers + : base->merge_trailers; + return conf; } @@ -4107,6 +4111,16 @@ AP_DECLARE(void) ap_register_errorlog_handler(apr_pool_t *p, char *tag, } +static const char *set_merge_trailers(cmd_parms *cmd, void *dummy, int arg) +{ + core_server_config *conf = ap_get_module_config(cmd->server->module_config, + &core_module); + conf->merge_trailers = (arg ? AP_MERGE_TRAILERS_ENABLE : + AP_MERGE_TRAILERS_DISABLE); + + return NULL; +} + /* Note --- ErrorDocument will now work from .htaccess files. * The AllowOverride of Fileinfo allows webmasters to turn it off */ @@ -4358,6 +4372,8 @@ AP_INIT_TAKE1("EnableExceptionHook", ap_mpm_set_exception_hook, NULL, RSRC_CONF, #endif AP_INIT_TAKE1("TraceEnable", set_trace_enable, NULL, RSRC_CONF, "'on' (default), 'off' or 'extended' to trace request body content"), +AP_INIT_FLAG("MergeTrailers", set_merge_trailers, NULL, RSRC_CONF, + "merge request trailers into request headers or not"), AP_INIT_ITERATE("HttpProtocol", set_http_protocol, NULL, RSRC_CONF, "'min=0.9' (default) or 'min=1.0' to allow/deny HTTP/0.9; " "'liberal', 'strict', 'strict,log-only'"), @@ -4449,7 +4465,6 @@ static int core_map_to_storage(request_rec *r) static int do_nothing(request_rec *r) { return OK; } - static int core_override_type(request_rec *r) { core_dir_config *conf = diff --git a/server/protocol.c b/server/protocol.c index 6c8948cf9b..d3b9de8a0a 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -1010,9 +1010,11 @@ request_rec *ap_read_request(conn_rec *conn) r->allowed_methods = ap_make_method_list(p, 2); r->headers_in = apr_table_make(r->pool, 25); + r->trailers_in = apr_table_make(r->pool, 5); r->subprocess_env = apr_table_make(r->pool, 25); r->headers_out = apr_table_make(r->pool, 12); r->err_headers_out = apr_table_make(r->pool, 5); + r->trailers_out = apr_table_make(r->pool, 5); r->notes = apr_table_make(r->pool, 5); r->request_config = ap_create_request_config(r->pool); @@ -1289,6 +1291,7 @@ AP_DECLARE(void) ap_set_sub_req_protocol(request_rec *rnew, rnew->status = HTTP_OK; rnew->headers_in = apr_table_copy(rnew->pool, r->headers_in); + rnew->trailers_in = apr_table_copy(rnew->pool, r->trailers_in); /* did the original request have a body? (e.g. POST w/SSI tags) * if so, make sure the subrequest doesn't inherit body headers @@ -1300,6 +1303,7 @@ AP_DECLARE(void) ap_set_sub_req_protocol(request_rec *rnew, rnew->subprocess_env = apr_table_copy(rnew->pool, r->subprocess_env); rnew->headers_out = apr_table_make(rnew->pool, 5); rnew->err_headers_out = apr_table_make(rnew->pool, 5); + rnew->trailers_out = apr_table_make(rnew->pool, 5); rnew->notes = apr_table_make(rnew->pool, 5); rnew->expecting_100 = r->expecting_100;