From 0f7bb9c6af3c5dfb6cb7ea9db2890c39c81b292e Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Fri, 22 Jun 2018 10:04:49 +0000 Subject: [PATCH] Merge r1827362, r1828926, r1828927, r1829557, r1829573, r1829645, r1829657 from trunk: core: ap_getline_core() reads nothing for n == 0. PR62199: add worker parameter ResponseFieldSize to mod_proxy Submitted By: Hank Ibell Committed By: covener add log id for r1828926 core: Add and handle AP_GETLINE_NOSPC_EOL flag in ap_rgetline_core(). This tells the ap_getline() family of functions to consume the end of line when the buffer is exhausted. PR 62198. mod_proxy_http: make use of AP_GETLINE_NOSPC_EOL in ap_proxygetline(). Fixes response header thrown away after the previous one was considered too large and truncated. PR 62196. core: forward flags to recursive/folding call to ap_rgetline_core(). We still need them when folding, other than AP_GETLINE_FOLD itself of course. mod_proxy_http: follow up to r1829573: remain EBCDIC friendly. Keep using ap_rgetline() as before r1829573, since ap_rgetline_core() is EBCDIC agnostic. Submitted by: ylavic, covener, covener, ylavic, ylavic, ylavic, ylavic Reviewed by: ylavic, covener, rpluem git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1834093 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 11 ++++ docs/manual/mod/mod_proxy.xml | 8 +++ include/ap_mmn.h | 6 +- include/http_protocol.h | 4 +- modules/proxy/mod_proxy.c | 8 +++ modules/proxy/mod_proxy.h | 2 + modules/proxy/mod_proxy_http.c | 83 ++++++++++++++++--------- server/protocol.c | 107 +++++++++++++++++++++++++-------- 8 files changed, 173 insertions(+), 56 deletions(-) diff --git a/CHANGES b/CHANGES index 352d44cc50..948bb81ef1 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,17 @@ -*- coding: utf-8 -*- Changes with Apache 2.4.34 + *) mod_proxy_http: Fix response header thrown away after the previous one + was considered too large and truncated. PR 62196. [Yann Ylavic] + + *) core: Add and handle AP_GETLINE_NOSPC_EOL flag for ap_getline() family + of functions to consume the end of line when the buffer is exhausted. + PR 62198. [Yann Ylavic] + + *) mod_proxy_http: Add new worker parameter 'responsefieldsize' to + allow maximum HTTP response header size to be increased past 8192 + bytes. PR62199. [Hank Ibell ] + *) mod_ssl: Extend SSLOCSPEnable with mode 'leaf' that only checks the leaf of a certificate chain. PR62112. [Ricardo Martin Camarero ] diff --git a/docs/manual/mod/mod_proxy.xml b/docs/manual/mod/mod_proxy.xml index 1f0c4e127a..bc3dc5239b 100644 --- a/docs/manual/mod/mod_proxy.xml +++ b/docs/manual/mod/mod_proxy.xml @@ -1111,6 +1111,14 @@ ProxyPass "/mirror/foo/i" "!" to override the ProxyIOBufferSize for a specific worker. This must be at least 512 or set to 0 for the system default of 8192. + responsefieldsize + 8192 + Adjust the size of the proxy response field buffer. The buffer size + should be at least the size of the largest expected header size from + a proxied response. Setting the value to 0 will use the system + default of 8192 bytes.
+ Available in Apache HTTP Server 2.4.34 and later. + keepalive Off

This parameter should be used when you have a firewall between your diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 539fc1e119..839228e29b 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -512,7 +512,9 @@ * ap_regcomp_default_cflag_by_name * 20120211.75 (2.4.30-dev) Add hostname_ex to proxy_worker_shared * 20120211.76 (2.4.30-dev) Add CONN_STATE_NUM to enum conn_state_e - * 20120211.77 (2.4.34-dev) add ap_exists_directive() + * 20120211.77 (2.4.34-dev) Add ap_exists_directive() + * 20120211.78 (2.4.34-dev) Add response_field_size to proxy_worker_shared + * 20120211.79 (2.4.34-dev) Add AP_GETLINE_NOSPC_EOL flag to http_protocol.h */ #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */ @@ -520,7 +522,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20120211 #endif -#define MODULE_MAGIC_NUMBER_MINOR 77 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 79 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/include/http_protocol.h b/include/http_protocol.h index 29d887c61e..11c7b2d187 100644 --- a/include/http_protocol.h +++ b/include/http_protocol.h @@ -606,7 +606,9 @@ AP_DECLARE(apr_status_t) ap_get_basic_auth_components(const request_rec *r, AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, const char *uri); #define AP_GETLINE_FOLD 1 /* Whether to merge continuation lines */ -#define AP_GETLINE_CRLF 2 /*Whether line ends must be in the form CR LF */ +#define AP_GETLINE_CRLF 2 /* Whether line ends must be in the form CR LF */ +#define AP_GETLINE_NOSPC_EOL 4 /* Whether to consume up to and including the + end of line on APR_ENOSPC */ /** * Get the next line of input for the request diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c index bc3410c45a..2336496919 100644 --- a/modules/proxy/mod_proxy.c +++ b/modules/proxy/mod_proxy.c @@ -319,6 +319,14 @@ static const char *set_worker_param(apr_pool_t *p, (int)sizeof(worker->s->upgrade)); } } + else if (!strcasecmp(key, "responsefieldsize")) { + long s = atol(val); + if (s < 0) { + return "ResponseFieldSize must be greater than 0 bytes, or 0 for system default."; + } + worker->s->response_field_size = (s ? s : HUGE_STRING_LEN); + worker->s->response_field_size_set = 1; + } else { if (set_worker_hc_param_f) { return set_worker_hc_param_f(p, s, worker, key, val, NULL); diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 63c2195ffe..f74b7bc541 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -451,6 +451,8 @@ typedef struct { apr_interval_time_t interval; char upgrade[PROXY_WORKER_MAX_SCHEME_SIZE];/* upgrade protocol used by mod_proxy_wstunnel */ char hostname_ex[PROXY_RFC1035_HOSTNAME_SIZE]; /* RFC1035 compliant version of the remote backend address */ + apr_size_t response_field_size; /* Size of proxy response buffer in bytes. */ + unsigned int response_field_size_set:1; } proxy_worker_shared; #define ALIGNED_PROXY_WORKER_SHARED_SIZE (APR_ALIGN_DEFAULT(sizeof(proxy_worker_shared))) diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 7377a118c5..01dc509010 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -28,6 +28,9 @@ static apr_status_t ap_proxy_http_cleanup(const char *scheme, request_rec *r, proxy_conn_rec *backend); +static apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n, + request_rec *r, int flags, int *read); + /* * Canonicalise http-like URLs. * scheme is the scheme for the URL @@ -1028,11 +1031,12 @@ static void ap_proxy_read_headers(request_rec *r, request_rec *rr, { int len; char *value, *end; - char field[MAX_STRING_LEN]; int saw_headers = 0; void *sconf = r->server->module_config; proxy_server_conf *psc; proxy_dir_conf *dconf; + apr_status_t rc; + apr_bucket_brigade *tmp_bb; dconf = ap_get_module_config(r->per_dir_config, &proxy_module); psc = (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module); @@ -1047,8 +1051,29 @@ static void ap_proxy_read_headers(request_rec *r, request_rec *rr, */ ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r, "Headers received from backend:"); - while ((len = ap_getline(buffer, size, rr, 1)) > 0) { - ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r, "%s", buffer); + + tmp_bb = apr_brigade_create(r->pool, c->bucket_alloc); + while (1) { + rc = ap_proxygetline(tmp_bb, buffer, size, rr, + AP_GETLINE_FOLD | AP_GETLINE_NOSPC_EOL, &len); + + if (len <= 0) + break; + + if (APR_STATUS_IS_ENOSPC(rc)) { + /* The header could not fit in the provided buffer, warn. + * XXX: falls through with the truncated header, 5xx instead? + */ + int trunc = (len > 128 ? 128 : len) / 2; + ap_log_rerror(APLOG_MARK, APLOG_WARNING, rc, r, APLOGNO(10124) + "header size is over the limit allowed by " + "ResponseFieldSize (%d bytes). " + "Bad response header: '%.*s[...]%s'", + size, trunc, buffer, buffer + len - trunc); + } + else { + ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r, "%s", buffer); + } if (!(value = strchr(buffer, ':'))) { /* Find the colon separator */ @@ -1116,16 +1141,6 @@ static void ap_proxy_read_headers(request_rec *r, request_rec *rr, */ process_proxy_header(r, dconf, buffer, value); saw_headers = 1; - - /* the header was too long; at the least we should skip extra data */ - if (len >= size - 1) { - while ((len = ap_getline(field, MAX_STRING_LEN, rr, 1)) - >= MAX_STRING_LEN - 1) { - /* soak up the extra data */ - } - if (len == 0) /* time to exit the larger loop as well */ - break; - } } } @@ -1137,23 +1152,19 @@ static int addit_dammit(void *v, const char *key, const char *val) return 1; } -static -apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n, request_rec *r, - int fold, int *writen) +static apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n, + request_rec *r, int flags, int *read) { - char *tmp_s = s; apr_status_t rv; apr_size_t len; - rv = ap_rgetline(&tmp_s, n, &len, r, fold, bb); + rv = ap_rgetline(&s, n, &len, r, flags, bb); apr_brigade_cleanup(bb); - if (rv == APR_SUCCESS) { - *writen = (int) len; - } else if (APR_STATUS_IS_ENOSPC(rv)) { - *writen = n; + if (rv == APR_SUCCESS || APR_STATUS_IS_ENOSPC(rv)) { + *read = (int)len; } else { - *writen = -1; + *read = -1; } return rv; @@ -1184,7 +1195,8 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, proxy_server_conf *conf, char *server_portstr) { conn_rec *c = r->connection; - char buffer[HUGE_STRING_LEN]; + char *buffer; + char fixed_buffer[HUGE_STRING_LEN]; const char *buf; char keepchar; apr_bucket *e; @@ -1193,6 +1205,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, int len, backasswards; int interim_response = 0; /* non-zero whilst interim 1xx responses * are being read. */ + apr_size_t response_field_size = 0; int pread_len = 0; apr_table_t *save_table; int backend_broke = 0; @@ -1220,6 +1233,20 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, bb = apr_brigade_create(p, c->bucket_alloc); pass_bb = apr_brigade_create(p, c->bucket_alloc); + /* Only use dynamically sized buffer if user specifies ResponseFieldSize */ + if(backend->worker->s->response_field_size_set) { + response_field_size = backend->worker->s->response_field_size; + + if (response_field_size != HUGE_STRING_LEN) + buffer = apr_pcalloc(p, response_field_size); + else + buffer = fixed_buffer; + } + else { + response_field_size = HUGE_STRING_LEN; + buffer = fixed_buffer; + } + /* Setup for 100-Continue timeout if appropriate */ if (do_100_continue) { apr_socket_timeout_get(backend->sock, &old_timeout); @@ -1249,11 +1276,11 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, apr_brigade_cleanup(bb); - rc = ap_proxygetline(backend->tmp_bb, buffer, sizeof(buffer), + rc = ap_proxygetline(backend->tmp_bb, buffer, response_field_size, backend->r, 0, &len); if (len == 0) { /* handle one potential stray CRLF */ - rc = ap_proxygetline(backend->tmp_bb, buffer, sizeof(buffer), + rc = ap_proxygetline(backend->tmp_bb, buffer, response_field_size, backend->r, 0, &len); } if (len <= 0) { @@ -1342,7 +1369,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, /* If not an HTTP/1 message or * if the status line was > 8192 bytes */ - if ((major != 1) || (len >= sizeof(buffer)-1)) { + if ((major != 1) || (len >= response_field_size - 1)) { return ap_proxyerror(r, HTTP_BAD_GATEWAY, apr_pstrcat(p, "Corrupt status line returned by remote " "server: ", buffer, NULL)); @@ -1385,7 +1412,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, "Set-Cookie", NULL); /* shove the headers direct into r->headers_out */ - ap_proxy_read_headers(r, backend->r, buffer, sizeof(buffer), origin, + ap_proxy_read_headers(r, backend->r, buffer, response_field_size, origin, &pread_len); if (r->headers_out == NULL) { diff --git a/server/protocol.c b/server/protocol.c index 334757799e..708160f30b 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -224,9 +224,12 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n, int do_alloc = (*s == NULL), saw_eos = 0; int fold = flags & AP_GETLINE_FOLD; int crlf = flags & AP_GETLINE_CRLF; + int nospc_eol = flags & AP_GETLINE_NOSPC_EOL; + int saw_eol = 0, saw_nospc = 0; if (!n) { /* Needs room for NUL byte at least */ + *read = 0; return APR_BADARG; } @@ -238,7 +241,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n, if (last_char) *last_char = '\0'; - for (;;) { + do { apr_brigade_cleanup(bb); rv = ap_get_brigade(r->proto_input_filters, bb, AP_MODE_GETLINE, APR_BLOCK_READ, 0); @@ -282,8 +285,52 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n, /* Would this overrun our buffer? If so, we'll die. */ if (n < bytes_handled + len) { - rv = APR_ENOSPC; - goto cleanup; + /* Before we die, let's fill the buffer up to its limit (i.e. + * fall through with the remaining length, if any), setting + * saw_eol on LF to stop the outer loop appropriately; we may + * come back here once the buffer is filled (no LF seen), and + * either be done at that time or continue to wait for LF here + * if nospc_eol is set. + * + * But there is also a corner cases which we want to address, + * namely if the buffer is overrun by the final LF only (i.e. + * the CR fits in); this is not really an overrun since we'll + * strip the CR finally (and use it for NUL byte), but anyway + * we have to handle the case so that it's not returned to the + * caller as part of the truncated line (it's not!). This is + * easier to consider that LF is out of counting and thus fall + * through with no error (saw_eol is set to 2 so that we later + * ignore LF handling already done here), while folding and + * nospc_eol logics continue to work (or fail) appropriately. + */ + saw_eol = (str[len - 1] == APR_ASCII_LF); + if (/* First time around */ + saw_eol && !saw_nospc + /* Single LF completing the buffered CR, */ + && ((len == 1 && ((*s)[bytes_handled - 1] == APR_ASCII_CR)) + /* or trailing CRLF overuns by LF only */ + || (len > 1 && str[len - 2] == APR_ASCII_CR + && n - bytes_handled + 1 == len))) { + /* In both cases *last_char is (to be) the CR stripped by + * later 'bytes_handled = last_char - *s'. + */ + saw_eol = 2; + } + else { + /* In any other case we'd lose data. */ + rv = APR_ENOSPC; + saw_nospc = 1; + } + len = n - bytes_handled; + if (!len) { + if (saw_eol) { + break; + } + if (nospc_eol) { + continue; + } + goto cleanup; + } } /* Do we have to handle the allocation ourselves? */ @@ -322,18 +369,28 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n, /* If we got a full line of input, stop reading */ if (last_char && (*last_char == APR_ASCII_LF)) { - break; + saw_eol = 1; } + } while (!saw_eol); + + if (rv != APR_SUCCESS) { + /* End of line after APR_ENOSPC above */ + goto cleanup; } /* Now terminate the string at the end of the line; - * if the last-but-one character is a CR, terminate there */ - if (last_char > *s && last_char[-1] == APR_ASCII_CR) { - last_char--; - } - else if (crlf) { - rv = APR_EINVAL; - goto cleanup; + * if the last-but-one character is a CR, terminate there. + * LF is handled above (not accounted) when saw_eol == 2, + * the last char is CR to terminate at still. + */ + if (saw_eol < 2) { + if (last_char > *s && last_char[-1] == APR_ASCII_CR) { + last_char--; + } + else if (crlf) { + rv = APR_EINVAL; + goto cleanup; + } } bytes_handled = last_char - *s; @@ -407,8 +464,8 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n, next_size = n - bytes_handled; - rv = ap_rgetline_core(&tmp, next_size, - &next_len, r, 0, bb); + rv = ap_rgetline_core(&tmp, next_size, &next_len, r, + flags & ~AP_GETLINE_FOLD, bb); if (rv != APR_SUCCESS) { goto cleanup; } @@ -442,22 +499,22 @@ cleanup: if (bytes_handled >= n) { bytes_handled = n - 1; } + + *read = bytes_handled; if (*s) { /* ensure the string is NUL terminated */ - (*s)[bytes_handled] = '\0'; - } - *read = bytes_handled; - - if (rv != APR_SUCCESS) { - return rv; - } + (*s)[*read] = '\0'; - /* PR#43039: We shouldn't accept NULL bytes within the line */ - if (strlen(*s) < bytes_handled) { - return APR_EINVAL; + /* PR#43039: We shouldn't accept NULL bytes within the line */ + bytes_handled = strlen(*s); + if (bytes_handled < *read) { + *read = bytes_handled; + if (rv == APR_SUCCESS) { + rv = APR_EINVAL; + } + } } - - return APR_SUCCESS; + return rv; } #if APR_CHARSET_EBCDIC -- 2.40.0