X-Forwarded-For Ignored if non 10.x or 127.0.0.1 source

[truekyleo]

Hello,

Unless your source address is a 10.0.0.0/8 or 127.0.0.1 the x-forwarded-for header is ignored by mochiweb. If you have a source with either a non-rfc1918 address or the other non-10.x rfc1918 address then mochiweb ignores the header.

src/mochiweb_request.erl:

get(peer, {?MODULE, [Socket, _Opts, _Method, _RawPath, _Version, _Headers]}=THIS) ->
    case mochiweb_socket:peername(Socket) of
        {ok, {Addr={10, _, _, _}, _Port}} ->
            case get_header_value("x-forwarded-for", THIS) of
                undefined ->
                    inet_parse:ntoa(Addr);
                Hosts ->
                    string:strip(lists:last(string:tokens(Hosts, ",")))
            end;
        {ok, {{127, 0, 0, 1}, _Port}} ->
            case get_header_value("x-forwarded-for", THIS) of
                undefined ->
                    "127.0.0.1";
                Hosts ->
                    string:strip(lists:last(string:tokens(Hosts, ",")))
            end;
        {ok, {Addr, _Port}} ->
            inet_parse:ntoa(Addr);
        {error, enotconn} ->
            exit(normal)
    end;

Shouldn't we always want to honour the x-forwarded-for header no matter what? Would this be better

Let me preface this with... I'm not an erlang dev or any kind of dev so forgive me if the suggested syntax is less than ideal.

get(peer, {?MODULE, [Socket, _Opts, _Method, _RawPath, _Version, _Headers]}=THIS) ->
    case mochiweb_socket:peername(Socket) of
        {ok, {Addr={_, _, _, _}, _Port}} ->
            case get_header_value("x-forwarded-for", THIS) of
                undefined ->
                    inet_parse:ntoa(Addr);
                Hosts ->
                    string:strip(lists:last(string:tokens(Hosts, ",")))
            end;
        {error, enotconn} ->
            exit(normal)
    end;

[etrepum]

The purpose of this function was to get the remote address of the peer, for consumer analytics and other purposes. In cases where the actual peer was loopback or 10/8 then we knew it was a load balancer of some kind and would skip it and instead look for the header. If the peer was not from a LAN or loopback address we did not trust its headers. If anything, adding the other reserved addresses to this would make sense (we didn't need it), but blindly trusting X-Forwarded-For is not a great idea.

[truekyleo]

My load balancer uses a non-internet route'able external IP address and and because of this Mochiweb is ignoring X-Forwarded-For and capturing the IP of the load balancer. We would have the same issue if our load balancer was using 192.168/16 or 172.16/12.

I made the suggested change on my side but it didn't seem to have the desired result. I can understand if you don't think this is an issue, but would you be able to recommend a change that would resolve my problem?

Thank you, Kyle

[etrepum]

You shouldn't need to make any changes to mochiweb itself, this function isn't used internally, so if you need different behavior you can just use a more direct route to get the data you want.

If you want to add support for more RFC 1918 addresses to this (e.g. including other other IPv4 reserved addresses or perhaps also IPv6), then a PR would certainly be accepted. Otherwise it would be too large of a semantic behavior change to accept for this function, but a second function could be added with that behavior.

[truekyleo]

just submitted a PR for the ipv4 fix.

a nice fix would be to have a config parameter to to allow x-forwarded-for from certain networks, but like I said, I'm no developer :)

[mworrell]

In Zotonic we are in the process of adding a proxy_whitelist configuration. You can check the work-in-progress version of the code here:

https://github.com/zotonic/z_stdlib/blob/master/src/z_ip_address.erl

There is some hard-coded matching to check if an ip address is local, ie. a non routable LAN-like address.