-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Insecure algorithm for determining remote/scheme/host #2171
Comments
Sorry, I don't follow what do you propose? |
For example, see Gunicorn’s |
Actually, rather than hardcoding “last element”, it might be better to provide, again, an option to choose it — like the |
People will reimplement I like idea of additional opt-in application parameters for configuring corresponding header. |
@asvetlov The parameter could combine headers and indices, like this:
|
I've decided to drop headers lookup for mentioned properties at all but support |
@asvetlov I am a huge fan of the work you do, but this is the second time in the span of a week and half where you have introduced a breaking change in a minor version (first yarl, now aiohttp). The yarl issue affected us while doing a deployment which brought down our production environment. This time, we at noticed the issue in a testing environment (luckily), when as a direct response to the yarl issue, we upgraded to the latest version of aiohttp. We host clusters of aiohttp services behind Amazon ALBs, which redirect all traffic internally via HTTP, not HTTPS. We use the scheme to determine which protocol was used to send URLs back to clients. This is important when running in dev environments vs. production. We do not support HTTP in production, which is where this becomes breaking. Checking the scheme in prod now returns URLs prefixed with HTTP scheme, instead of HTTPS. While it's not a big issue for us to adapt to, breaking changes should not be introduced in minor versions. Therefore we don't go digging through change logs to see what we need to change when performing a minor upgrade. I reiterate, I love the work you do and am super thankful for everything you have provided to the Python community. But please please please be careful with versioning! I hope you understand, |
@thomaspsk thank you for feedback. We have created https://github.com/wikibusiness/aiohttp-remotes for helping users to respect headers like For your case the closest (but not secure) way is adding a middleware:
The library will be moved into aio-libs organization just after writing documentation. Sorry for caused inconvenience. |
P.S. |
Just a slight correction to @asvetlov's example above: from aiohttp-remotes import setup, XForwardedRelaxed
await setup(app, XForwardedRelaxed()) # <- takes app as the first parameter |
use
by the way, all the x-remote not for me cause I'm using google appengine.
below is the raw info i get from request.headers
|
@tyan4g Both |
Long story short
aiohttp.web.BaseRequest
has attributesscheme
,host
and, since 2.3,remote
. Their values are determined by looking at request headers that can be manipulated by a remote user. Under most configurations, the user can set them to any desired values. This is especially bad for theremote
attribute, because the user’s IP address is often relied upon for access controls.(I’m reporting this in the open rather than privately, because
remote
is not yet released, whilescheme
andhost
do not seem like much of a security problem.)Expected behaviour
This involves the headers
Forwarded
andX-Forwarded-For
. The last element (comma-separated) in these headers can be trusted as long as a trusted proxy is configured to append it.The same applies to the headers
X-Forwarded-Host
andX-Forwarded-Proto
, with the exception that they are usually single values (to be replaced by the proxy), not comma-separated lists.Actual behaviour
aiohttp takes the first element from
Forwarded
/X-Forwarded-For
, without knowing which of these headers (if any) are controlled by a trusted proxy.In most deployments where aiohttp sits behind nginx, the
Forwarded
header is not controlled by the proxy. Nobody is aware that it needs to be controlled. It is not mentioned in the example nginx configuration from aiohttp docs. In such deployments, an external user can trivially forcescheme
,host
andremote
to any desired values by sending a header like:In fact, it’s impossible to configure current versions of nginx to correctly append a
Forwarded
header (without writing some C or possibly Lua code).But even if aiohttp was sitting behind a proxy that correctly controlled all of the involved headers, nothing would change, because the proxy would append a comma-separated element to the remote user’s
Forwarded
header, while aiohttp is looking at the first element — which is still controlled by the user.Steps to reproduce
Run this server program:
behind nginx with the following configuration (derived from the example):
and send requests to it with curl:
Your environment
aiohttp Git master, Python 3.5, Linux
The text was updated successfully, but these errors were encountered: