Skip to content
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

Need to check real IP address for proxy deployments or document behaviour #47

Open
vmsh0 opened this issue Jun 8, 2018 · 0 comments
Open
Labels
3.x Issue or PR for stable 3.x version docs Something is missing in docs enhancement Make it better! security
Projects
Milestone

Comments

@vmsh0
Copy link

vmsh0 commented Jun 8, 2018

Checking the X-Forwarded-For header field for IP address filtering here might not be enough for some deployments.

E.g.

  • The bot is bind on 0.0.0.0 and an appropriate firewall rule is not used
  • The bot is bind on a public Internet IP address to which a different server forwards Telegram packets and an appropriate firewall rule is not used

The exemplified use cases are (hopefully) unusual but still very possible, especially in testing environments. In each of these cases, one could easily impersonate Telegram by sending HTTP requests to the server's public IP address and forging the X-Forwarded-For header field.
Although this does seem more like a IT administration issue than an aiogram issue, I think this should at least be documented, since a developer (especially one that's less-then-savvy with computer networks) could legitimately presume that the IP address filtering functionality would provide complete security against such attacks. I could also imagine larger deployments in which the above presumptions could be true for performance reasons.

If you are also considering a hard fix, here are some ideas:

  • Add default logic to presume that non-HTTPS deployments should only be local (and from a local IP address), only allow a different behavior if explicitly enabled (secure by default, no need to break code compatibility)
  • Change the IP address filtering option to force the developer to explicitly state what should be in the X-Forwarded-For HTTP header field instead of determining it through the current logic (secure by updating bot code, probably breaks code compatibility)
@JrooTJunior JrooTJunior added enhancement Make it better! security labels Jun 9, 2018
@evgfilim1 evgfilim1 added the stale "Dead" or inactive issue label Nov 19, 2020
@evgfilim1 evgfilim1 removed the stale "Dead" or inactive issue label Mar 2, 2021
@JrooTJunior JrooTJunior added docs Something is missing in docs 3.x Issue or PR for stable 3.x version labels Aug 4, 2023
@JrooTJunior JrooTJunior added this to the 3.0 milestone Aug 4, 2023
@JrooTJunior JrooTJunior added this to To do in 3.0 via automation Aug 4, 2023
@JrooTJunior JrooTJunior modified the milestones: 3.0, 3.x future Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Issue or PR for stable 3.x version docs Something is missing in docs enhancement Make it better! security
Projects
3.0
  
To do
Development

No branches or pull requests

3 participants