uvicorn icon indicating copy to clipboard operation
uvicorn copied to clipboard

Published 20 hours ago • verified publisher icon encode

Improve ProxyHeadersMiddleware

Open pypae opened this issue 3 years ago • 13 comments

Summary This PR addresses multiple issues mentioned in https://github.com/encode/uvicorn/issues/1068 to improve the ProxyHeadersMiddleware.

  • :bug: Fix the host for requests from clients running on the proxy server itself. (The main issue)
  • :bug: Fallback to host that was already set for empty x-forwarded-for headers. (Mentioned by @b0g3r)
  • ✨ Also allow to specify IP ranges in CIDR notation as trusted hosts. (Mentioned by @jalaziz) This greatly simplifies deployments on docker swarm / kubernetes, where the reverse proxy might have a dynamic IP.

pypae avatar Aug 22 '22 15:08 pypae

@b0g3r I don't fully grasp your third issue:

  • It doesn't check request_addr like it is one of the proxies. It means that hacker who has direct network access to the server can impersonate IPs (example)

Isn't this covered by the following line? https://github.com/encode/uvicorn/blob/6947d19ff39286dc1d126eb5299f1c58c321dad3/uvicorn/middleware/proxy_headers.py#L81

pypae avatar Aug 22 '22 15:08 pypae

Is this a feature that you would like to see merged? Can we do anything on our side?

For the time being, we're using this middleware outside of uvicorn. For anyone coming across the same issue: Just copy the code, manually add it as a middleware and disable proxy headers in uvicorn. (For example by setting the environment variable UVICORN_PROXY_HEADERS=False)

pypae avatar Sep 08 '22 08:09 pypae

As a side note -- would it be reasonable to add support for X-Forwarded-Host and X-Forwarded-Port, like Werkzeug does? (There's also X-Forwarded-Prefix, but I'm not familiar enough with Quart to know if that would be the right place to handle it)

UPD: Actually, that would be #965. Also, docs mention X-Forwarded-Port but it doesn't seem to be handled anywhere in the code, as far as I can see.

purplesyringa avatar Oct 25 '22 12:10 purplesyringa

I think we may need to add more strategic comments on this PR, since it's not trivial to follow. :sweat_smile:

Ref.: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For

Kludex avatar Mar 10 '23 19:03 Kludex

@pypae Are you still interested in this PR?

Kludex avatar Mar 20 '23 13:03 Kludex

Are you still interested in this PR?

Yes, I still think this feature could be of use for many users. I'll have another look at it later this week and add some strategic comments.

pypae avatar Mar 20 '23 13:03 pypae

:wave: :runner: :dash:

Kludex avatar Apr 15 '23 14:04 Kludex

@pypae Are you still interested in this PR?

Kludex avatar Apr 22 '23 13:04 Kludex

Hello @Kludex @pypae I hope you're doing well. I'm also interested in this functionality. Currently in our code we have to do this manually, and it would be very nice to have the out-of-the-box mechanism in uvicorn.

@pypae are you planning to further work on this functionality?

If not, are you @Kludex planning to take over and finish this pull request? Or are you going to close it without merging?

amadeuszhercog-ocado avatar May 15 '23 08:05 amadeuszhercog-ocado

I'm waiting for the author here. If someone wants to take over, feel free to do it. I'm not closing this PR anyway.

I need my review comments to be replied.

Kludex avatar May 17 '23 03:05 Kludex

It will be nice to have it. :)

gonchik avatar Dec 14 '23 16:12 gonchik

Nothing to add. Go ahead. 🙏

Kludex avatar Jan 24 '24 08:01 Kludex

For those subscribed - I've started a new PR #2231

nhairs avatar Jan 25 '24 06:01 nhairs

  • Let's continue on https://github.com/encode/uvicorn/pull/2231.

Kludex avatar Feb 29 '24 21:02 Kludex

Thanks @Kludex

gonchik avatar Mar 01 '24 13:03 gonchik