Ability to get remote IP from a request

[Kentzo]

Sometimes it is desired to know an IP, e.g. for google re-captcha.

[rutsky]

Remote IP can be obtained from request transport, see this part of documentation:

peername = request.transport.get_extra_info('peername')
if peername is not None:
    host, port = peername

[Kentzo]

Awesome, thanks!

[kxepal]

Worth to note (just for case) that if you have proxy in front, you'd better check headers first for forwarded peer IP.

[Kentzo]

Maybe it would make sense to have a method that would check headers fallback to peer IP?

[kxepal]

It depends on your proxy configuration. You can have standard X-Forwarded-For or Forwarded (per RFC 7239) or else you found more suitable. Mostly, if you have proxy, you're aware about and shouldn't forget to check these at the first place.

[Kentzo]

@kxepal Indeed, when it's your code. But imagine amount of unnecessary copy-parsing because of that?

Unless aiohttp targets to be low-level HTTP library, I would expect it to implement standard and RFCed methods.

[rutsky]

Forwarding headers should be used only if all connections to the aiohttp server made through trusted forwarding proxy, otherwise users will be able to impersonate someone else IP by just setting X-Forwarded-For header.

aiohttp doesn't know anything about forward proxy configuration and I don't think automatic fallback can be easily handled (e.g. for non-trivial cases, like when all connections from specific IP (of SSL forward proxy) are forwarded connections, and all other connections are direct connections from end users).

[Kentzo]

@rutsky But shouldn't it be easy to implement verification? E.g.:

class Request:
    def get_client_ip(trusted_proxies=('127.0.0.1')):
        def get_peername_host():
            peername = request.transport.get_extra_info('peername')

            if peername is not None:
                host, port = peername
                return host
            else:
                return None    

        client_ip = get_peername_host()

        if trusted_proxies:
            if not client_ip in trusted_proxies:
                raise …

        # Check Commong HTTP Headers

        return client_ip

[popravich]

Client IP lookup really depends on your service implementation/configuration.
aiohttp app can be easily served through unix socket so implementing all kind of checks
to find something looking like a real client IP (and not a localhost or proxy ip) is big overhead.

[asvetlov]

aiohttp already has complex logic for determining http scheme: https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/web_reqrep.py#L131-L144
A PR with similar approach for client ip would make sense.
I don't want to code it (at least not right now) but I'm open for reviewing and merging.

[Kentzo]

@popravich Could you further explain? I believe trusted_proxies could be further expanded to understand unix paths.

[popravich]

I'm just saying that if you're putting information about proxies into your application (read trusted_proxies parameter) than you know how your application is deployed -- say "behind nginx proxy running on port" and you can easily "make a convention" -- "Nginx must set "X-Forwarded-For" header with remote client ip address". And you'll need to do in your app is to read that header no peername lookups.

[popravich]

Regarding get_client_ip implementation I believe that in 99.9% of time your application will fallback to headers check because I'm, again, 99.9% sure you'll be running your app behind forwarding proxy)

[asvetlov]

Yes, @popravich is right.

[asvetlov]

Volunteers are welcome.

[Kentzo]

@asvetlov I'm going to work on it.

What should be the right behavior for this function in case when it cannot figure out client IP? Returning None or raising an exception?

[asvetlov]

What should be the right behavior for this function in case when it cannot figure out client IP? Returning None or raising an exception?

Let's take a look on use cases: in what cases server cannot figure out client IP at all?

I can imagine only single situation: aiohttp is behind a reverse proxy, connected by UNIX socket and no X-Forwarded-For-like header sent.

I have no strong preference but inclining to raising an error: it is very rare case and users most likely will never expect it in their code. Returning None will provide cryptic errors like NoneType has no attribute 'split' several lines below actual error source. If you agree with me please push the previous sentence as comment in function's implementation.

[Kentzo]

Another issue would be an incorrect value of a header. In other hand, it's probably job of the proxy to take care of those.

[kxepal]

@Kentzo There could be no proxy to blame. Assume you have aiohttp served directly to the world and some client sent X-Forwarded-For header (just for lulz or with some intention) with a weird value.

[asvetlov]

Superseded by #1134

[lock]

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.