# HG changeset patch # User Ruslan Ermilov # Date 1361971790 0 # Node ID f7fe817c92a20db67db29d3c18206d2e2fd3687f # Parent a805dc9c85cdba962d3eb7257064a3f6acec139d Correctly handle multiple X-Forwarded-For headers (ticket #106). diff --git a/src/http/modules/ngx_http_geo_module.c b/src/http/modules/ngx_http_geo_module.c --- a/src/http/modules/ngx_http_geo_module.c +++ b/src/http/modules/ngx_http_geo_module.c @@ -314,18 +314,17 @@ static ngx_int_t ngx_http_geo_addr(ngx_http_request_t *r, ngx_http_geo_ctx_t *ctx, ngx_addr_t *addr) { - ngx_table_elt_t *xfwd; + ngx_array_t *xfwd; if (ngx_http_geo_real_addr(r, ctx, addr) != NGX_OK) { return NGX_ERROR; } - xfwd = r->headers_in.x_forwarded_for; + xfwd = &r->headers_in.x_forwarded_for; - if (xfwd != NULL && ctx->proxies != NULL) { - (void) ngx_http_get_forwarded_addr(r, addr, xfwd->value.data, - xfwd->value.len, ctx->proxies, - ctx->proxy_recursive); + if (xfwd->nelts > 0 && ctx->proxies != NULL) { + (void) ngx_http_get_forwarded_addr(r, addr, xfwd, NULL, + ctx->proxies, ctx->proxy_recursive); } return NGX_OK; diff --git a/src/http/modules/ngx_http_geoip_module.c b/src/http/modules/ngx_http_geoip_module.c --- a/src/http/modules/ngx_http_geoip_module.c +++ b/src/http/modules/ngx_http_geoip_module.c @@ -240,19 +240,18 @@ static u_long ngx_http_geoip_addr(ngx_http_request_t *r, ngx_http_geoip_conf_t *gcf) { ngx_addr_t addr; - ngx_table_elt_t *xfwd; + ngx_array_t *xfwd; struct sockaddr_in *sin; addr.sockaddr = r->connection->sockaddr; addr.socklen = r->connection->socklen; /* addr.name = r->connection->addr_text; */ - xfwd = r->headers_in.x_forwarded_for; + xfwd = &r->headers_in.x_forwarded_for; - if (xfwd != NULL && gcf->proxies != NULL) { - (void) ngx_http_get_forwarded_addr(r, &addr, xfwd->value.data, - xfwd->value.len, gcf->proxies, - gcf->proxy_recursive); + if (xfwd->nelts > 0 && gcf->proxies != NULL) { + (void) ngx_http_get_forwarded_addr(r, &addr, xfwd, NULL, + gcf->proxies, gcf->proxy_recursive); } #if (NGX_HAVE_INET6) @@ -293,7 +292,7 @@ static geoipv6_t ngx_http_geoip_addr_v6(ngx_http_request_t *r, ngx_http_geoip_conf_t *gcf) { ngx_addr_t addr; - ngx_table_elt_t *xfwd; + ngx_array_t *xfwd; in_addr_t addr4; struct in6_addr addr6; struct sockaddr_in *sin; @@ -303,12 +302,11 @@ ngx_http_geoip_addr_v6(ngx_http_request_ addr.socklen = r->connection->socklen; /* addr.name = r->connection->addr_text; */ - xfwd = r->headers_in.x_forwarded_for; + xfwd = &r->headers_in.x_forwarded_for; - if (xfwd != NULL && gcf->proxies != NULL) { - (void) ngx_http_get_forwarded_addr(r, &addr, xfwd->value.data, - xfwd->value.len, gcf->proxies, - gcf->proxy_recursive); + if (xfwd->nelts > 0 && gcf->proxies != NULL) { + (void) ngx_http_get_forwarded_addr(r, &addr, xfwd, NULL, + gcf->proxies, gcf->proxy_recursive); } switch (addr.sockaddr->sa_family) { diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -2014,32 +2014,44 @@ static ngx_int_t ngx_http_proxy_add_x_forwarded_for_variable(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data) { - u_char *p; + size_t len; + u_char *p; + ngx_uint_t i, n; + ngx_table_elt_t **h; v->valid = 1; v->no_cacheable = 0; v->not_found = 0; - if (r->headers_in.x_forwarded_for == NULL) { + n = r->headers_in.x_forwarded_for.nelts; + h = r->headers_in.x_forwarded_for.elts; + + len = 0; + + for (i = 0; i < n; i++) { + len += h[i]->value.len + sizeof(", ") - 1; + } + + if (len == 0) { v->len = r->connection->addr_text.len; v->data = r->connection->addr_text.data; return NGX_OK; } - v->len = r->headers_in.x_forwarded_for->value.len - + sizeof(", ") - 1 + r->connection->addr_text.len; - - p = ngx_pnalloc(r->pool, v->len); + len += r->connection->addr_text.len; + + p = ngx_pnalloc(r->pool, len); if (p == NULL) { return NGX_ERROR; } + v->len = len; v->data = p; - p = ngx_copy(p, r->headers_in.x_forwarded_for->value.data, - r->headers_in.x_forwarded_for->value.len); - - *p++ = ','; *p++ = ' '; + for (i = 0; i < n; i++) { + p = ngx_copy(p, h[i]->value.data, h[i]->value.len); + *p++ = ','; *p++ = ' '; + } ngx_memcpy(p, r->connection->addr_text.data, r->connection->addr_text.len); diff --git a/src/http/modules/ngx_http_realip_module.c b/src/http/modules/ngx_http_realip_module.c --- a/src/http/modules/ngx_http_realip_module.c +++ b/src/http/modules/ngx_http_realip_module.c @@ -107,10 +107,12 @@ ngx_module_t ngx_http_realip_module = { static ngx_int_t ngx_http_realip_handler(ngx_http_request_t *r) { - u_char *ip, *p; + u_char *p; size_t len; + ngx_str_t *value; ngx_uint_t i, hash; ngx_addr_t addr; + ngx_array_t *xfwd; ngx_list_part_t *part; ngx_table_elt_t *header; ngx_connection_t *c; @@ -137,19 +139,20 @@ ngx_http_realip_handler(ngx_http_request return NGX_DECLINED; } - len = r->headers_in.x_real_ip->value.len; - ip = r->headers_in.x_real_ip->value.data; + value = &r->headers_in.x_real_ip->value; + xfwd = NULL; break; case NGX_HTTP_REALIP_XFWD: - if (r->headers_in.x_forwarded_for == NULL) { + xfwd = &r->headers_in.x_forwarded_for; + + if (xfwd->elts == NULL) { return NGX_DECLINED; } - len = r->headers_in.x_forwarded_for->value.len; - ip = r->headers_in.x_forwarded_for->value.data; + value = NULL; break; @@ -178,8 +181,8 @@ ngx_http_realip_handler(ngx_http_request && len == header[i].key.len && ngx_strncmp(p, header[i].lowcase_key, len) == 0) { - len = header[i].value.len; - ip = header[i].value.data; + value = &header[i].value; + xfwd = NULL; goto found; } @@ -192,15 +195,13 @@ found: c = r->connection; - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0, "realip: \"%s\"", ip); - addr.sockaddr = c->sockaddr; addr.socklen = c->socklen; /* addr.name = c->addr_text; */ - if (ngx_http_get_forwarded_addr(r, &addr, ip, len, rlcf->from, + if (ngx_http_get_forwarded_addr(r, &addr, xfwd, value, rlcf->from, rlcf->recursive) - == NGX_OK) + != NGX_DECLINED) { return ngx_http_realip_set_addr(r, &addr); } diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -76,6 +76,9 @@ static ngx_uint_t ngx_http_gzip_quantity static char *ngx_http_gzip_disable(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); #endif +static ngx_int_t ngx_http_get_forwarded_addr_internal(ngx_http_request_t *r, + ngx_addr_t *addr, u_char *xff, size_t xfflen, ngx_array_t *proxies, + int recursive); #if (NGX_HAVE_OPENAT) static char *ngx_http_disable_symlinks(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); @@ -2747,10 +2750,58 @@ ngx_http_set_disable_symlinks(ngx_http_r ngx_int_t ngx_http_get_forwarded_addr(ngx_http_request_t *r, ngx_addr_t *addr, + ngx_array_t *headers, ngx_str_t *value, ngx_array_t *proxies, + int recursive) +{ + ngx_int_t rc; + ngx_uint_t i, found; + ngx_table_elt_t **h; + + if (headers == NULL) { + return ngx_http_get_forwarded_addr_internal(r, addr, value->data, + value->len, proxies, + recursive); + } + + i = headers->nelts; + h = headers->elts; + + rc = NGX_DECLINED; + + found = 0; + + while (i-- > 0) { + rc = ngx_http_get_forwarded_addr_internal(r, addr, h[i]->value.data, + h[i]->value.len, proxies, + recursive); + + if (!recursive) { + break; + } + + if (rc == NGX_DECLINED && found) { + rc = NGX_DONE; + break; + } + + if (rc != NGX_OK) { + break; + } + + found = 1; + } + + return rc; +} + + +static ngx_int_t +ngx_http_get_forwarded_addr_internal(ngx_http_request_t *r, ngx_addr_t *addr, u_char *xff, size_t xfflen, ngx_array_t *proxies, int recursive) { u_char *p; in_addr_t inaddr; + ngx_int_t rc; ngx_addr_t paddr; ngx_cidr_t *cidr; ngx_uint_t family, i; @@ -2842,8 +2893,15 @@ ngx_http_get_forwarded_addr(ngx_http_req *addr = paddr; if (recursive && p > xff) { - (void) ngx_http_get_forwarded_addr(r, addr, xff, p - 1 - xff, - proxies, 1); + rc = ngx_http_get_forwarded_addr_internal(r, addr, xff, p - 1 - xff, + proxies, 1); + + if (rc == NGX_DECLINED) { + return NGX_DONE; + } + + /* rc == NGX_OK || rc == NGX_DONE */ + return rc; } return NGX_OK; diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h --- a/src/http/ngx_http_core_module.h +++ b/src/http/ngx_http_core_module.h @@ -516,7 +516,8 @@ ngx_int_t ngx_http_set_disable_symlinks( ngx_http_core_loc_conf_t *clcf, ngx_str_t *path, ngx_open_file_info_t *of); ngx_int_t ngx_http_get_forwarded_addr(ngx_http_request_t *r, ngx_addr_t *addr, - u_char *xff, size_t xfflen, ngx_array_t *proxies, int recursive); + ngx_array_t *headers, ngx_str_t *value, ngx_array_t *proxies, + int recursive); extern ngx_module_t ngx_http_core_module; diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -21,14 +21,14 @@ static ngx_int_t ngx_http_process_header ngx_table_elt_t *h, ngx_uint_t offset); static ngx_int_t ngx_http_process_unique_header_line(ngx_http_request_t *r, ngx_table_elt_t *h, ngx_uint_t offset); +static ngx_int_t ngx_http_process_multi_header_lines(ngx_http_request_t *r, + ngx_table_elt_t *h, ngx_uint_t offset); static ngx_int_t ngx_http_process_host(ngx_http_request_t *r, ngx_table_elt_t *h, ngx_uint_t offset); static ngx_int_t ngx_http_process_connection(ngx_http_request_t *r, ngx_table_elt_t *h, ngx_uint_t offset); static ngx_int_t ngx_http_process_user_agent(ngx_http_request_t *r, ngx_table_elt_t *h, ngx_uint_t offset); -static ngx_int_t ngx_http_process_cookie(ngx_http_request_t *r, - ngx_table_elt_t *h, ngx_uint_t offset); static ngx_int_t ngx_http_process_request_header(ngx_http_request_t *r); static void ngx_http_process_request(ngx_http_request_t *r); @@ -153,7 +153,7 @@ ngx_http_header_t ngx_http_headers_in[] #if (NGX_HTTP_X_FORWARDED_FOR) { ngx_string("X-Forwarded-For"), offsetof(ngx_http_headers_in_t, x_forwarded_for), - ngx_http_process_header_line }, + ngx_http_process_multi_header_lines }, #endif #if (NGX_HTTP_REALIP) @@ -185,7 +185,8 @@ ngx_http_header_t ngx_http_headers_in[] ngx_http_process_header_line }, #endif - { ngx_string("Cookie"), 0, ngx_http_process_cookie }, + { ngx_string("Cookie"), offsetof(ngx_http_headers_in_t, cookies), + ngx_http_process_multi_header_lines }, { ngx_null_string, 0, NULL } }; @@ -930,15 +931,6 @@ ngx_http_process_request_line(ngx_event_ return; } - - if (ngx_array_init(&r->headers_in.cookies, r->pool, 2, - sizeof(ngx_table_elt_t *)) - != NGX_OK) - { - ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); - return; - } - c->log->action = "reading client request headers"; rev->handler = ngx_http_process_request_headers; @@ -1535,20 +1527,31 @@ ngx_http_process_user_agent(ngx_http_req static ngx_int_t -ngx_http_process_cookie(ngx_http_request_t *r, ngx_table_elt_t *h, +ngx_http_process_multi_header_lines(ngx_http_request_t *r, ngx_table_elt_t *h, ngx_uint_t offset) { - ngx_table_elt_t **cookie; - - cookie = ngx_array_push(&r->headers_in.cookies); - if (cookie) { - *cookie = h; - return NGX_OK; + ngx_array_t *headers; + ngx_table_elt_t **ph; + + headers = (ngx_array_t *) ((char *) &r->headers_in + offset); + + if (headers->elts == NULL) { + if (ngx_array_init(headers, r->pool, 1, sizeof(ngx_table_elt_t *)) + != NGX_OK) + { + ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); + return NGX_ERROR; + } } - ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); - - return NGX_ERROR; + ph = ngx_array_push(headers); + if (ph == NULL) { + ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); + return NGX_ERROR; + } + + *ph = h; + return NGX_OK; } diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h --- a/src/http/ngx_http_request.h +++ b/src/http/ngx_http_request.h @@ -200,7 +200,7 @@ typedef struct { ngx_table_elt_t *keep_alive; #if (NGX_HTTP_X_FORWARDED_FOR) - ngx_table_elt_t *x_forwarded_for; + ngx_array_t x_forwarded_for; #endif #if (NGX_HTTP_REALIP) diff --git a/src/http/ngx_http_variables.c b/src/http/ngx_http_variables.c --- a/src/http/ngx_http_variables.c +++ b/src/http/ngx_http_variables.c @@ -138,8 +138,8 @@ static ngx_int_t ngx_http_variable_time_ */ /* - * the $http_host, $http_user_agent, $http_referer, $http_via, - * and $http_x_forwarded_for variables may be handled by generic + * the $http_host, $http_user_agent, $http_referer, and $http_via + * variables may be handled by generic * ngx_http_variable_unknown_header_in(), but for performance reasons * they are handled using dedicated entries */ @@ -161,7 +161,7 @@ static ngx_http_variable_t ngx_http_cor #endif #if (NGX_HTTP_X_FORWARDED_FOR) - { ngx_string("http_x_forwarded_for"), NULL, ngx_http_variable_header, + { ngx_string("http_x_forwarded_for"), NULL, ngx_http_variable_headers, offsetof(ngx_http_request_t, headers_in.x_forwarded_for), 0, 0 }, #endif