Fix closing the back end connection in case of error.

The field "closed" was changed from an int to a bit
field of size one in 2.4.x.
For historical reasons a close instruction was coded
as an increment on the field, which in 2.4.x flips
the field each time. There were mutliple code paths
that would flip it several times for a single error,
so effectively the connection was no longer closed
in these cases.

Especially in the case of an aborted client connection
this lead to a non consumed back end buffer and thus to
response mixup between users.

PR 53727

CVE-2012-3052

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

diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c
index c9f58171f525eef3f82c90ef1cc94065f2d651fd..3736156afcdd1620295ae0d98d312c44d425a7d9 100644 (file)
--- a/modules/proxy/mod_proxy_ajp.c
+++ b/modules/proxy/mod_proxy_ajp.c
@@ -207,7 +207,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
     /* send request headers */
     status = ajp_send_header(conn->sock, r, maxsize, uri);
     if (status != APR_SUCCESS) {
-        conn->close++;
+        conn->close = 1;
         ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00868)
                       "request failed to %pI (%s)",
                       conn->worker->cp->addr,
@@ -234,7 +234,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
     status = ajp_alloc_data_msg(r->pool, &buff, &bufsiz, &msg);
     if (status != APR_SUCCESS) {
         /* We had a failure: Close connection to backend */
-        conn->close++;
+        conn->close = 1;
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00869)
                       "ajp_alloc_data_msg failed");
         return HTTP_INTERNAL_SERVER_ERROR;
@@ -255,7 +255,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
 
         if (status != APR_SUCCESS) {
             /* We had a failure: Close connection to backend */
-            conn->close++;
+            conn->close = 1;
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00871)
                           "ap_get_brigade failed");
             apr_brigade_destroy(input_brigade);
@@ -275,7 +275,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
         status = apr_brigade_flatten(input_brigade, buff, &bufsiz);
         if (status != APR_SUCCESS) {
             /* We had a failure: Close connection to backend */
-            conn->close++;
+            conn->close = 1;
             apr_brigade_destroy(input_brigade);
             ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00874)
                           "apr_brigade_flatten");
@@ -290,7 +290,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
             ajp_msg_log(r, msg, "First ajp_send_data_msg: ajp_ilink_send packet dump");
             if (status != APR_SUCCESS) {
                 /* We had a failure: Close connection to backend */
-                conn->close++;
+                conn->close = 1;
                 apr_brigade_destroy(input_brigade);
                 ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00876)
                               "send failed to %pI (%s)",
@@ -319,7 +319,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
              * for later resusage by the next request again.
              * Close it to clean things up.
              */
-            conn->close++;
+            conn->close = 1;
             return HTTP_BAD_REQUEST;
         }
     }
@@ -330,7 +330,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
                              (ajp_msg_t **)&(conn->data));
     if (status != APR_SUCCESS) {
         /* We had a failure: Close connection to backend */
-        conn->close++;
+        conn->close = 1;
         apr_brigade_destroy(input_brigade);
         ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00878)
                       "read response failed from %pI (%s)",
@@ -559,7 +559,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
          * But: Close this connection to the backend.
          */
         if (r->connection->aborted) {
-            conn->close++;
+            conn->close = 1;
             output_failed = 0;
             result = CMD_AJP13_END_RESPONSE;
             request_ended = 1;
@@ -597,7 +597,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
                       "Processing of request failed backend: %i, "
                       "output: %i", backend_failed, output_failed);
         /* We had a failure: Close connection to backend */
-        conn->close++;
+        conn->close = 1;
         /* Return DONE to avoid error messages being added to the stream */
         if (data_sent) {
             rv = DONE;
@@ -607,7 +607,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00891)
                       "Processing of request didn't terminate cleanly");
         /* We had a failure: Close connection to backend */
-        conn->close++;
+        conn->close = 1;
         backend_failed = 1;
         /* Return DONE to avoid error messages being added to the stream */
         if (data_sent) {
@@ -616,7 +616,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
     }
     else if (!conn_reuse) {
         /* Our backend signalled connection close */
-        conn->close++;
+        conn->close = 1;
     }
     else {
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00892)
@@ -679,7 +679,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
     apr_brigade_destroy(output_brigade);
 
     if (apr_table_get(r->subprocess_env, "proxy-nokeepalive")) {
-        conn->close++;
+        conn->close = 1;
     }
 
     return rv;
@@ -758,7 +758,7 @@ static int proxy_ajp_handler(request_rec *r, proxy_worker *worker,
              * TCP connection gets closed and try it once again.
              */
             if (status != APR_SUCCESS) {
-                backend->close++;
+                backend->close = 1;
                 ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897)
                               "cping/cpong failed to %pI (%s)",
                               worker->cp->addr, worker->s->hostname);

diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index b219854400d9991b3b4702fb92e5c5ba7e381870..243c9a5e7dfc2262e40717f4a8da4af3235b3a07 100644 (file)
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -795,14 +795,14 @@ int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
         }
         buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.0" CRLF, NULL);
         force10 = 1;
-        p_conn->close++;
+        p_conn->close = 1;
     } else {
         buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.1" CRLF, NULL);
         force10 = 0;
     }
     if (apr_table_get(r->subprocess_env, "proxy-nokeepalive")) {
         origin->keepalive = AP_CONN_CLOSE;
-        p_conn->close++;
+        p_conn->close = 1;
     }
     ap_xlate_proto_to_ascii(buf, strlen(buf));
     e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
@@ -1019,7 +1019,7 @@ int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
      */
     if (!r->kept_body && r->main) {
         /* XXX: Why DON'T sub-requests use keepalives? */
-        p_conn->close++;
+        p_conn->close = 1;
         if (old_cl_val) {
             old_cl_val = NULL;
             apr_table_unset(r->headers_in, "Content-Length");
@@ -1056,7 +1056,7 @@ int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
         apr_table_unset(r->headers_in, "Content-Length");
         old_cl_val = NULL;
         origin->keepalive = AP_CONN_CLOSE;
-        p_conn->close++;
+        p_conn->close = 1;
     }
 
     /* Prefetch MAX_MEM_SPOOL bytes
@@ -1705,7 +1705,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
                 ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01106)
                               "bad HTTP/%d.%d header returned by %s (%s)",
                               major, minor, r->uri, r->method);
-                backend->close += 1;
+                backend->close = 1;
                 /*
                  * ap_send_error relies on a headers_out to be present. we
                  * are in a bad position here.. so force everything we send out
@@ -1746,7 +1746,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
                               "server %s:%d returned Transfer-Encoding"
                               " and Content-Length",
                               backend->hostname, backend->port);
-                backend->close += 1;
+                backend->close = 1;
             }
 
             /*
@@ -1755,8 +1755,9 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
              */
             te = apr_table_get(r->headers_out, "Transfer-Encoding");
             /* strip connection listed hop-by-hop headers from response */
-            backend->close += ap_find_token(p,
-                    apr_table_get(r->headers_out, "Connection"), "close");
+            if (ap_find_token(p, apr_table_get(r->headers_out, "Connection"),
+                              "close"))
+                backend->close = 1;
             ap_proxy_clear_connection(p, r->headers_out);
             if ((buf = apr_table_get(r->headers_out, "Content-Type"))) {
                 ap_set_content_type(r, apr_pstrdup(p, buf));
@@ -1801,7 +1802,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
 
             /* cancel keepalive if HTTP/1.0 or less */
             if ((major < 1) || (minor < 1)) {
-                backend->close += 1;
+                backend->close = 1;
                 origin->keepalive = AP_CONN_CLOSE;
             }
         } else {
@@ -1809,7 +1810,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
             backasswards = 1;
             r->status = 200;
             r->status_line = "200 OK";
-            backend->close += 1;
+            backend->close = 1;
         }
 
         if (ap_is_HTTP_INFO(proxy_status)) {