Skip to content
This repository has been archived by the owner on Jun 1, 2022. It is now read-only.

API log should reflect forwarded IP addresses #310

Open
simonw opened this issue Apr 15, 2021 · 3 comments · May be fixed by #695
Open

API log should reflect forwarded IP addresses #310

simonw opened this issue Apr 15, 2021 · 3 comments · May be fixed by #695
Labels
api Anything under /api/ bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed research Research needed

Comments

@simonw
Copy link
Collaborator

simonw commented Apr 15, 2021

I just noticed every single one of our API logs is for IP 169.254.8.129 https://vial.calltheshots.us/admin/api/apilog/?remote_ip=169.254.8.129
@simonw simonw added bug Something isn't working api Anything under /api/ labels Apr 15, 2021
@simonw
Copy link
Collaborator Author

simonw commented Apr 15, 2021

Here's why:

vial/vaccinate/api/utils.py

Line 74 in bee2a8b

remote_ip=request.META.get("REMOTE_ADDR") or "",

The challenge here is to securely read the X-Forwarded-For without risk of being confused by clients that deliberately send a fake HTTP header. Different hosting environments have different considerations for this - I'm not sure what the best way to do this on Cloud Run is.

@simonw simonw added good first issue Good for newcomers help wanted Extra attention is needed research Research needed labels Apr 15, 2021
@simonw
Copy link
Collaborator Author

simonw commented Apr 15, 2021

I tweeted asking for recommendations: https://twitter.com/simonw/status/1382531187529240576

@alexmv
Copy link
Member

alexmv commented Apr 15, 2021

https://cloud.google.com/load-balancing/docs/https#x-forwarded-for_header is relevant here. We know that we are only accessible through the loadbalancer, which means that last value in X-Forwarded-For is the only trusted value.

django-xff is a fine solution here, as long as we set the depth to 1.

@MoralCode MoralCode linked a pull request Jun 25, 2021 that will close this issue
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
api Anything under /api/ bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed research Research needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Proxy ip fix MoralCode/vial
2 participants
@simonw @alexmv