<div dir="ltr">hi,<br><br>we are currently using nginx v0.5.35 in our production environment.<br>fast, lean and stable. thanks!<br><br>and, we have run into one problem recently.<br><br>i've devised what i believe is a fix. the attached a patch file shows<br>
what i've done. my hope is that it could be helpful to share this<br>patch, and that i might get some feedback, such as: "yes, your fix<br>seems workable", or "you've overlooked this whole other situation...".<br>
<br>i developed this patch against latest stable release, v0.6.32 and have<br>been running it on a staging machine. hoping to get it proofed enough<br>to move into production soon. it was designed to just minimally<br>attend to the specific problem i am having. i realize it's not a<br>
general-coverage type of solution (more on this below).<br><br>our web server has recently had an appliance known as "netscaler" put<br>in front of it. an overview:<br><br> <a href="http://www.infoworld.com/article/07/11/12/46TC-citrix-netscaler_1.html">http://www.infoworld.com/article/07/11/12/46TC-citrix-netscaler_1.html</a><br>
<br>as a result, we now need to count on nginx's realip module to derive<br>the remote_addr value based on the X-Forwarded-For headers that come<br>through. we want a remote_addr value that reflects the client's ip<br>
number (or their most outward proxy) as much as possible to make<br>logging meaningful, and so that we can derive location info through<br>the geo module.<br><br>some incoming HTTP headers have a single X-Forwarded-For line, with a<br>
single value:<br><br> X-Forwarded-For: <a href="http://66.249.70.42">66.249.70.42</a><br><br>or a single line with a set of comma separated values:<br><br> X-Forwarded-For: <a href="http://66.249.70.40">66.249.70.40</a>, <a href="http://66.249.70.41">66.249.70.41</a><br>
<br>both of these cases are handled ok. <br><br>when multiple values exist the realip module always takes the<br>rightmost ip number. this seems to work ok based on the emperical<br>evidence i see from a tcpdump.<br><br>
the problem comes in when a header contains multiple X-Forwarded-For<br>lines:<br><br> GET /stylesheets/search.css?1210024315 HTTP/1.0<br> Accept: */*<br> Referer: <a href="http://www.somewhere.com/com/usa/en/searches">http://www.somewhere.com/com/usa/en/searches</a><br>
Accept-Language: en-us<br> UA-CPU: x86<br> If-Modified-Since: Mon, 05 May 2008 21:51:55 GMT; length=4732<br> User-Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 1.1.4322; .NET CLR 2.0.50727; InfoPath.1; .NET CLR 3.0.04506.30; .NET CLR 3.0.04506.648)<br>
Host: <a href="http://www.somewhere.com">www.somewhere.com</a><br> X-Forwarded-For: <a href="http://10.1.1.69">10.1.1.69</a><br> Cache-Control: max-age=259200<br> Connection: keep-alive<br> X-Forwarded-For: <a href="http://64.126.102.126">64.126.102.126</a> <br>
<br>the behavior i see in both v0.5.35 and v0.6.32 is that the value from<br>the first-encountered X-Forwarded-For line is used and any remaining<br>ones seem to be ignored. so, for the above case, our remote_addr ends<br>
up being the non-public value of <a href="http://10.1.1.69">10.1.1.69</a>.<br><br>after some searching about how multiple lines of the same type are<br>supposed to be handled, i found this:<br><br> <a href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2">http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2</a><br>
<br>which contains:<br><br> Multiple message-header fields with the same field-name MAY be<br> present in a message if and only if the entire field-value for that<br> header field is defined as a comma-separated list [i.e.,<br>
#(values)]. It MUST be possible to combine the multiple header<br> fields into one "field-name: field-value" pair, without changing the<br> semantics of the message, by appending each subsequent field-value<br>
to the first, each separated by a comma. The order in which header<br> fields with the same field-name are received is therefore<br> significant to the interpretation of the combined field value, and<br> thus a proxy MUST NOT change the order of these field values when a<br>
message is forwarded.<br><br>so, for that header above, if i could have the values from the<br>multiple lines coalesced, it would be as if i received a single line<br>like this:<br><br> X-Forwarded-For: <a href="http://10.1.1.69">10.1.1.69</a>, <a href="http://64.126.102.126">64.126.102.126</a> <br>
<br>i have developed a coding change that makes nginx accumulate the<br>values for any and all X-Forwarded-For lines within an incoming<br>header. i found that this is almost enough let the realip module<br>operate properly.<br>
<br>once this coding change was put in place, i could proceed with<br>testing. i then discovered a bug within the realip module that makes<br>it fail to replace the ip address in some cases. a string was not<br>being properly zero-terminated so, just depending on what came after<br>
it in memory, the replacement would work sometimes, but not others. i<br>made another coding change to correct this.<br><br>the attached patch file modifies two source files:<br><br> ./nginx-0.6.32/src/http/ngx_http_request.c<br>
./nginx-0.6.32/src/http/modules/ngx_http_realip_module.c<br><br>the changes to ngx_http_request.c have to do with accumulating<br>the values from any and all X-Forwarded-For lines.<br><br>the changes to ngx_http_realip_module.c have to do with making sure<br>
the ip replacement decision is made with a zero-terminated string.<br><br>when i wrote the value-accumualation code within ngx_http_request.c, i<br>ended up needing a local buffer. i'm allocating that buffer's memory<br>
from the request pool. my belief is that i don't need to explicitly<br>free this memory since the entire request-based pool will be dropped<br>once the request is fully processed. i do not see any free calls<br>being made for any of the other allocations made against this<br>
request-based pool. is my presumption correct?<br><br>from my reading of that rfc2616 text, i believe that nginx needs to<br>apply this type of value coalescing to any and all cases of multiple<br>header lines of the same type. my modification only applies this<br>
treatment for the X-Forwarded-For case. i did this to reduce the<br>amount of testing required.<br><br>i hope this info is helpful and that, if i'm overlooking something,<br>someone might raise a warning to me.<br><br>
thanks in advance...<br><br>martin stitt<br><br><br></div>