*) 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.

Submitted By: Edward Lu, Yann Ylavic, Joe Orton, Eric Covener
Committed By: covener

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1610814 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index d1b782f47ccef5caf7711386b53e5ceaab202eaf..4b414c0b5bf42cd8eef3753b7ab324c6ea733267 100644 (file)
--- 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 411af8188774a47fdd5f1e3fdf7a2c36607feec2..8f637e2cf8671d75734b7375cba57a8085650fbf 100644 (file)
--- 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.</p>
 </usage>
 </directivesynopsis>
 
+<directivesynopsis>
+<name>MergeTrailers</name>
+<description>Determins whether trailers are merged into headers</description>
+<syntax>MergeTrailers [on|off]</syntax>
+<default>MergeTrailers off</default>
+<contextlist><context>server config</context><context>virtual host</context></contextlist>
+<compatibility>2.4.10 and later</compatibility>
+
+<usage>
+    <p>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.</p>
+    <p>This option is provided for compatibility with releases prior to 2.4.10,
+    where trailers were always merged.</p>
+</usage>
+</directivesynopsis>
+
+
 </modulesynopsis>

diff --git a/docs/manual/mod/mod_log_config.xml b/docs/manual/mod/mod_log_config.xml
index bbd7f642a4b35e627e3f3996a866ccbfff3f3e64..df07a75b1e3b6c2ce7d3e2e74cf3d86bac9938b6 100644 (file)
--- 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 <module>mod_logio</module> to use this.</td></tr>
 
+    <tr><td><code>%{<var>VARNAME</var>}^ti</code></td>
+        <td>The contents of <code><var>VARNAME</var>:</code> trailer line(s)
+        in the request sent to the server.  </td></tr>
+
+    <tr><td><code>%{<var>VARNAME</var>}^to</code></td>
+        <td>The contents of <code><var>VARNAME</var>:</code> trailer line(s)
+        in the response sent from the server.  </td></tr>
+
     </table>
 
     <section id="modifiers"><title>Modifiers</title>

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index ee60c646f1ddf2319a1ed4ab333a421bf01f411f..ea496962c1627077c1fc7a605f00663f1dd2bcf8 100644 (file)
--- 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 337859ba0550d4798e8bdc47e718ab401e6fbace..3ed37939c4929974a82f02d22fd343ee2599d61d 100644 (file)
--- 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 77345b2bfc364bff74fb0a2df203a56a07bc9d20..ba0bc990b9e7f73abdbdeeed5481eaa9b736cbd6 100644 (file)
--- 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 d2c618dd62129775741689d9007c014554192523..a9ef0374109722bef2a292f33cd75efcd40935b3 100644 (file)
--- 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 796d506e8418ad94fa0ea930411e6c0e5a288a4f..cdfec8b5689a8b5b536c7de7d36a4e1f8549624c 100644 (file)
--- 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 569a7484051fadc44112ab21473640a7ccad2555..75e131eeba9b5b0c4149e4857c27a39a643ef5ed 100644 (file)
--- 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 5f696691ae2102ed64a75e7ab943c9a64656d723..91d14f5a98a8a27b72cbcf3eafba4c05824ee276 100644 (file)
--- 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 5032bc796bf684402ce80a18442dbce4d7547d34..6a62c03e0094acb10b8c9515c261e376f6a2b2dc 100644 (file)
--- 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 6c8948cf9b542889f33b8dadf8523d0dca776fbd..d3b9de8a0a577346151e04ce0742fcdf74d82088 100644 (file)
--- 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;