]> granicus.if.org Git - apache/commitdiff
SECURITY: CVE-2013-5704 (cve.mitre.org)
authorWilliam A. Rowe Jr <wrowe@apache.org>
Fri, 22 Aug 2014 18:18:08 +0000 (18:18 +0000)
committerWilliam A. Rowe Jr <wrowe@apache.org>
Fri, 22 Aug 2014 18:18:08 +0000 (18:18 +0000)
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
Backports: r1610814
Reviewed by: covener, wrowe, ylavic

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

12 files changed:
CHANGES
docs/manual/mod/core.xml
docs/manual/mod/mod_log_config.xml
include/ap_mmn.h
include/http_core.h
include/httpd.h
modules/http/http_filters.c
modules/http/http_request.c
modules/loggers/mod_log_config.c
modules/proxy/mod_proxy_http.c
server/core.c
server/protocol.c

diff --git a/CHANGES b/CHANGES
index 23b28820c918e7b7f259ea39cc7c14adacb571aa..9bab34fceb74af053e3666e1bd52180556e010e9 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,13 @@
 
 Changes with Apache 2.4.11
 
+  *) 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]
+
   *) mod_lua: Don't quote Expires and Path values. PR 56734.
      [Keith Mashinter, <kmashint yahoo com>]
 
@@ -249,7 +256,6 @@ Changes with Apache 2.4.10
   *) mod_lua: More verbose error logging when a handler function cannot be
      found. [Daniel Gruno]
 
-
 Changes with Apache 2.4.9
 
   *) mod_ssl: Work around a bug in some older versions of OpenSSL that
index e2f89f2cfb3689301a109a3bf37cad6ef9b7c38b..682688589d204fa075709861a04a831d5e45ae89 100644 (file)
@@ -4431,4 +4431,23 @@ hostname or IP address</description>
     different sections are combined when a request is received</seealso>
 </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>
index bbd7f642a4b35e627e3f3996a866ccbfff3f3e64..df07a75b1e3b6c2ce7d3e2e74cf3d86bac9938b6 100644 (file)
         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>
index a36a9997c390105b7637c06b8fae98c5daefd96d..83d979467baf097e18e4b060d143ab9103a775a5 100644 (file)
  * 20120211.35 (2.4.10-dev) Add "r", "must_rebind", and last_backend_conn  
                             to util_ldap_connection_t
  * 20120211.36 (2.4.10-dev) Add ap_copy_scoreboard_worker()
+ * 20120211.37 (2.4.11-dev) Add r->trailers_{in,out}
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20120211
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 36                   /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 37                   /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
index 8730d1fde94269d39cb502b9d502afc22f833fb5..5cef6224fc6fa530b728621a5c83c5c2db3c2183 100644 (file)
@@ -667,6 +667,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;
 
 } core_server_config;
 
index e1510be2d3a88e2484af773ae5f0385c4b0de7bd..c6cd8279800fa9c26a7ecfe550087813aa49445d 100644 (file)
@@ -1035,6 +1035,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;
 };
 
 /**
index 2a0a979d5fa755aa9439c10fbf46e91584730fa1..0b8600962f4ed4bf699d05931dfabdda458d4024 100644 (file)
@@ -231,6 +231,49 @@ static apr_status_t get_chunk_line(http_ctx_t *ctx, apr_bucket_brigade *b,
 }
 
 
+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);
+    ctx->state = BODY_NONE;
+    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
@@ -240,6 +283,7 @@ 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;
@@ -247,6 +291,9 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
     int http_error = HTTP_REQUEST_ENTITY_TOO_LARGE;
     apr_bucket_brigade *bb;
 
+    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);
@@ -425,13 +472,8 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
             }
 
             if (!ctx->remaining) {
-                /* Handle trailers by calling ap_get_mime_headers again! */
-                ctx->state = BODY_NONE;
-                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;
+                return read_chunked_trailers(ctx, f, b,
+                        conf->merge_trailers == AP_MERGE_TRAILERS_ENABLE);
             }
         }
     }
@@ -534,13 +576,8 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
                 }
 
                 if (!ctx->remaining) {
-                    /* Handle trailers by calling ap_get_mime_headers again! */
-                    ctx->state = BODY_NONE;
-                    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;
+                    return read_chunked_trailers(ctx, f, b,
+                            conf->merge_trailers == AP_MERGE_TRAILERS_ENABLE);
                 }
             }
             break;
index 796d506e8418ad94fa0ea930411e6c0e5a288a4f..cdfec8b5689a8b5b536c7de7d36a4e1f8549624c 100644 (file)
@@ -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);
 
index 792756db07b3e5d280eb9d1105493ae4f0d4d4d7..c1b0e1ba672637663435801ff35163791b12abcd 100644 (file)
@@ -431,6 +431,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)
@@ -514,6 +520,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));
@@ -916,7 +927,7 @@ static char *parse_log_misc_string(apr_pool_t *p, log_format_item *it,
 static char *parse_log_item(apr_pool_t *p, log_format_item *it, const char **sa)
 {
     const char *s = *sa;
-    ap_log_handler *handler;
+    ap_log_handler *handler = NULL;
 
     if (*s != '%') {
         return parse_log_misc_string(p, it, sa);
@@ -986,7 +997,16 @@ static char *parse_log_item(apr_pool_t *p, log_format_item *it, const char **sa)
             break;
 
         default:
-            handler = (ap_log_handler *)apr_hash_get(log_hash, s++, 1);
+            /* check for '^' + two character format first */
+            if (*s == '^' && *(s+1) && *(s+2)) { 
+                handler = (ap_log_handler *)apr_hash_get(log_hash, s, 3); 
+                if (handler) { 
+                   s += 3;
+                }
+            }
+            if (!handler) {  
+                handler = (ap_log_handler *)apr_hash_get(log_hash, s++, 1);  
+            }
             if (!handler) {
                 char dummy[2];
 
@@ -1516,7 +1536,7 @@ static void ap_register_log_handler(apr_pool_t *p, char *tag,
     log_struct->func = handler;
     log_struct->want_orig_default = def;
 
-    apr_hash_set(log_hash, tag, 1, (const void *)log_struct);
+    apr_hash_set(log_hash, tag, strlen(tag), (const void *)log_struct);
 }
 static ap_log_writer_init *ap_log_set_writer_init(ap_log_writer_init *handle)
 {
@@ -1694,6 +1714,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 */
index 141452bf21c09a758a9e75a4afe407da052b09ad..1a4d59372feb7e5042b51bc4cebf67383f65fc30 100644 (file)
@@ -1011,8 +1011,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);
 
@@ -1093,6 +1096,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;
 
     /*
@@ -1223,6 +1227,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
 apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
                                             proxy_conn_rec **backend_ptr,
@@ -1735,6 +1747,12 @@ apr_status_t 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
index dd1a375d8958d129d1731f8ea1178f50c0fa7da5..613ffa40447c4f75f81cf631734c08ad208ae5cd 100644 (file)
@@ -520,6 +520,10 @@ static void *merge_core_server_configs(apr_pool_t *p, void *basev, void *virtv)
     if (virt->error_log_req)
         conf->error_log_req = virt->error_log_req;
 
+    conf->merge_trailers = (virt->merge_trailers != AP_MERGE_TRAILERS_UNSET)
+                           ? virt->merge_trailers
+                           : base->merge_trailers;
+
     return conf;
 }
 
@@ -3882,6 +3886,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
  */
@@ -4129,6 +4143,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"),
 { NULL }
 };
 
@@ -4211,7 +4227,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 =
index bf915a0ab9d5ae93d2280cc41f31c4eb05d5ccff..960117dca9d1324ba40889cde9b35c5cb547e6cc 100644 (file)
@@ -718,6 +718,8 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb
                 r->status = HTTP_REQUEST_TIME_OUT;
             }
             else {
+                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, 
+                              "Failed to read request header line %s", field);
                 r->status = HTTP_BAD_REQUEST;
             }
 
@@ -917,9 +919,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);
@@ -1185,6 +1189,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
@@ -1196,6 +1201,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;