Merge r1610814, r1610686, r1610707 from trunk:

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

Reviewed By:  covener, wrowe, rpluem

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

diff --git a/CHANGES b/CHANGES
index e2b0a17e6aaf73e66288ec6f80d19b9ac2379195..d38b8383677942a8fd9e46593675b28aea57c47e 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -21,6 +21,13 @@ Changes with Apache 2.2.28
      Fix a race condition in scoreboard handling, which could lead to
      a heap buffer overflow.  [Joe Orton, Eric Covener, Jeff Trawick]
 
+  *) 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_proxy: Don't reuse a SSL backend connection whose requested SNI
      differs. PR 55782.  [Yann Ylavic]
  
@@ -630,7 +637,6 @@ Changes with Apache 2.2.16
   *) mod_rewrite: Allow to set environment variables without explicitly
      giving a value. [Rainer Jung]
 
-
 Changes with Apache 2.2.15
 
   *) SECURITY: CVE-2009-3555 (cve.mitre.org)

diff --git a/STATUS b/STATUS
index d5724c8db1e6b3c9fb6c5067e65a03eec63e6554..573b937900f3565ff98f748317ece3733aa54fcb 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -99,22 +99,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) 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. 
-     trunk patch: http://svn.apache.org/r1610814 
-                  http://svn.apache.org/r1610686 (mod_log_config ^XX support) 
-                  http://svn.apache.org/r1610707 (mod_log_cofnig ^XX support)
-     2.2.x patch:  http://people.apache.org/~covener/patches/httpd-2.2.x-trailers-2.diff
-     +1: covener, wrowe, rpluem
-     covener: Since this was not released yet in 2.4.x, maybe it's better to cut 2.2.28 w/o it?
-     mrumph:  Delaying a nonCVE fix would be reasonable to maintain backward compatibility.
-              But for a CVE that has already been made public,
-              wouldn't it make more sense to make the fix available as quickly as possible?
-     
    * mod_deflate: Fix reentrance in output and input filters (buffering of
                   incomplete Zlib header or validation bytes). PR 46146.
      trunk patch: https://svn.apache.org/r1572655

diff --git a/docs/manual/mod/core.xml b/docs/manual/mod/core.xml
index b394ef4bc4e84b1043023bc9289a501c6b1d52ea..8323c9f6e76cb4288e3ed7023da37e8f32b3e37d 100644 (file)
--- a/docs/manual/mod/core.xml
+++ b/docs/manual/mod/core.xml
@@ -3655,4 +3655,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>

diff --git a/docs/manual/mod/mod_log_config.xml b/docs/manual/mod/mod_log_config.xml
index 9596607c7f779413b921858dc42c561ac4fb76d1..6b71a8fe3a577408a0924455866c84e6ded1e322 100644 (file)
--- a/docs/manual/mod/mod_log_config.xml
+++ b/docs/manual/mod/mod_log_config.xml
@@ -225,6 +225,15 @@
     <tr><td><code>%O</code></td>
         <td>Bytes sent, including headers, cannot be zero. 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 d82bbc9a0a1f3dbc265b4d2c5be466964dc949d8..205869674383f9e3bfc9f8e401326db2481fa07b 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -153,6 +153,7 @@
  * 20051115.33 (2.2.24) Add ap_pregsub_ex()
  * 20051115.34 (2.2.28) Add ap_copy_scoreboard_worker()
  * 20051115.35 (2.2.28) Add SSL reusable SNI to mod_proxy.h's proxy_conn_rec
+ * 20051115.36 (2.2.28) Add r->trailers_{in,out}
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503232UL /* "AP22" */
@@ -160,7 +161,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20051115
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 35                    /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 36                    /* 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 7be9d5dcab636f213a5b09f1af85b7fa860a647d..1aa9bd9b5a9e200ed8f1ec7c5de6ef2459874866 100644 (file)
--- a/include/http_core.h
+++ b/include/http_core.h
@@ -613,6 +613,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;
 

diff --git a/include/httpd.h b/include/httpd.h
index f353d9ff7baae17c438cd6d5755fbba3d681a0af..5264184b820813e4e29a2274c35d3ba0a9aaf24b 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -1006,6 +1006,11 @@ struct request_rec {
  * record to improve 64bit alignment the next time we need to break
  * binary compatibility for some other reason.
  */
+
+    /** 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 8b6e371c474bc2dff1c428efe0c320a1c5cd33b7..b7c35c62715dbd36e4482476401cdf058d6d2317 100644 (file)
--- a/modules/http/http_filters.c
+++ b/modules/http/http_filters.c
@@ -206,6 +206,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
@@ -215,6 +258,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;
@@ -222,6 +266,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);
@@ -403,13 +450,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);
             }
         }
     }
@@ -509,13 +551,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;

diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index 9d63aca816bf94f738272d7154c988339ee556ce..aedee167bae09f064173ea794c5a79e0e479ab22 100644 (file)
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -384,8 +384,10 @@ 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);
     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);
 
@@ -495,6 +497,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 a08354039533764146bcd0d4d64531aa0a865ce6..058a306dedabe3b2f67e92a3f4154e8a5f6ac832 100644 (file)
--- a/modules/loggers/mod_log_config.c
+++ b/modules/loggers/mod_log_config.c
@@ -412,6 +412,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)
@@ -495,6 +501,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));
@@ -813,7 +824,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);
@@ -883,7 +894,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];
 
@@ -1389,7 +1409,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)
 {
@@ -1558,6 +1578,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 330f80bd765ce8c5161e3643b456cad49ec5b8be..932b62182b27e5bddb1d138e2e2a4cfc1248ddd4 100644 (file)
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -1229,6 +1229,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;
 
     /*
@@ -1355,6 +1356,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,
@@ -1809,6 +1818,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(rp->trailers_in)) {
+                        apr_table_do(add_trailers, r->trailers_out,
+                                rp->trailers_in, NULL);
+                        apr_table_clear(rp->trailers_in);
+                    }
+
                     apr_brigade_length(bb, 0, &readbytes);
                     backend->worker->s->read += readbytes;
 #if DEBUGGING

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 2a94aa6be91abecffa0c07261f1c3fcb4228d853..454004a5fb4caf76d675878298955d5d07df2e1a 100644 (file)
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -350,8 +350,11 @@ PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)
     rp->status          = HTTP_OK;
 
     rp->headers_in      = apr_table_make(r->pool, 50);
+    rp->trailers_in     = apr_table_make(r->pool, 5);
+
     rp->subprocess_env  = apr_table_make(r->pool, 50);
     rp->headers_out     = apr_table_make(r->pool, 12);
+    rp->trailers_out    = apr_table_make(r->pool, 5);
     rp->err_headers_out = apr_table_make(r->pool, 5);
     rp->notes           = apr_table_make(r->pool, 5);
 

diff --git a/server/core.c b/server/core.c
index ca72713925fdd804ef5172f7a3fd04a91202c5b1..07e099715d5065db296bed01e937c277c6dcfbbc 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -542,6 +542,10 @@ static void *merge_core_server_configs(apr_pool_t *p, void *basev, void *virtv)
                          ? virt->trace_enable
                          : base->trace_enable;
 
+    conf->merge_trailers = (virt->merge_trailers != AP_MERGE_TRAILERS_UNSET)
+                           ? virt->merge_trailers
+                           : base->merge_trailers;
+
     return conf;
 }
 
@@ -3295,6 +3299,16 @@ static const char *set_trace_enable(cmd_parms *cmd, void *dummy,
     return NULL;
 }
 
+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
  */
@@ -3534,6 +3548,8 @@ AP_INIT_TAKE1("TraceEnable", set_trace_enable, NULL, RSRC_CONF,
 AP_INIT_FLAG("Suexec", unixd_set_suexec, NULL, RSRC_CONF,
              "Enable or disable suEXEC support"),
 #endif
+AP_INIT_FLAG("MergeTrailers", set_merge_trailers, NULL, RSRC_CONF,
+              "merge request trailers into request headers or not"),
 { NULL }
 };
 

diff --git a/server/protocol.c b/server/protocol.c
index ac03082d3029ac0494d2022708e547d0d8141e94..579fbcad798ba991c32f1be381080c00ad794c22 100644 (file)
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -708,6 +708,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;
             }
 
@@ -887,9 +889,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);
@@ -1141,7 +1145,8 @@ 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->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
@@ -1153,6 +1158,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;