encode / uvicorn

An ASGI web server, for Python. 🦄
https://www.uvicorn.org/
BSD 3-Clause "New" or "Revised" License
8.41k stars 728 forks source link

Add support for Fowarded header (RFC 7239) #2237

Open Kludex opened 8 months ago

Kludex commented 8 months ago

Discussed in https://github.com/encode/uvicorn/discussions/2236

Originally posted by **nhairs** January 28, 2024 We should probably support the `Forwarded` header which was standardised in [RFC 7239](https://datatracker.ietf.org/doc/html/rfc7239). One of the benefits of this header over `X-Forwarded-*` headers is that it supports including the port of the client rather than just the address. See also: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded

[!IMPORTANT]

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.

Fund with Polar

Kludex commented 7 months ago

Reference for myself: https://systemoverlord.com/2020/03/25/security-101-x-forwarded-for-vs-forwarded-vs-proxy.html

Kludex commented 7 months ago

@nhairs Do you have any proposal on how we should implement this?

Can we have the X-Forwarded-* and Forwarded at the same time?

nhairs commented 7 months ago

Can we have the X-Forwarded-* and Forwarded at the same time?

Per the code comments I left in my open PR, I initially thought this was the case. i.e. use the official headers if available otherwise fallback to the x-forwarded headers. But I suspect that such behaviour might introduce vulnerabilities into user's applications. It might be better to take a PEP20 "Explicit is better than implicit" approach which leads me to...

@nhairs Do you have any proposal on how we should implement this?

My gut feeling is that we're better off making users explicitly choose which headers they want to extract info from. Trying to support all of them from the commandline seems like a lot of work though. What about supporting X-Real-IP headers?

Which leads to my suggestion on #2231:

Kludex commented 2 days ago

Makes sense to be explicit on what is supported.

Kludex commented 2 days ago

Maybe we can check how others are doing it? Are we going to pioneer this "choices"?