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

Add CONFIG parameter that stores the REAL_IP of the client #8867

Closed
PieterL75 opened this issue Mar 14, 2022 · 7 comments
Closed

Add CONFIG parameter that stores the REAL_IP of the client #8867

PieterL75 opened this issue Mar 14, 2022 · 7 comments
Labels
pending closure Requires immediate attention to avoid being closed for inactivity status: under review Further discussion is needed to determine this issue's scope and/or implementation type: feature Introduction of new functionality to the application

Comments

@PieterL75
Copy link
Contributor

PieterL75 commented Mar 14, 2022

NetBox version

v3.1.9

Feature type

New functionality

Proposed functionality

The real IP of a client is currently unknown to netbox. All traffic goes through a reverse proxy (nginx), hiding the original client IP.
We have to create a configurable paramters that identified the HTTP Header that stores the original IP. This is required because not all of the reverse proxies use the same HTTP Header (X-Forwarded-For, X-Real-IP, ...)

This FR will add a new configuration parameter that customizes the HTTP header that stores the real IP of the client.

Proposed Configuration variable name : PROXY_HEADER_REALIP


PROXY_HEADER_REALIP

Default: HTTP_X_REAL_IP

This parameters sets the HTTP Header that contains the REAL IP of a client that connects through a PROXY. The Real IP is required to validate an API token's Allowed IPRanges.
Other common values are HTTP_X_FORWARDED_FOR, HTTP_X_CLIENT_IP

Use case

The FR '#8233 Restrict API key usage by source IP' requires to see the real IP address.
For now a static HTTP Header of 'HTTP_X_REAL_IP is used in that FR, but this needs to be customizable

Database changes

No response

External dependencies

No response

@PieterL75 PieterL75 added the type: feature Introduction of new functionality to the application label Mar 14, 2022
@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Mar 18, 2022
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label May 18, 2022
@PieterL75
Copy link
Contributor Author

I'm good to take this and implement, if approved of course

@jeremystretch
Copy link
Member

I don't think this is needed; see what I ended up doing with the get_client_ip() utility function for #8233. There are only three headers (AFAIK) that we need to consider:

  • HTTP_X_REAL_IP
  • HTTP_X_FORWARDED_FOR
  • REMOTE_ADDR

At least one of these should always be defined by the HTTP frontend, so I think this is a reasonable approach.

It's also worth pointing out that this challenge (reliably determining the client IP address) is a very generic "Django problem" rather than something specific to NetBox. I'm happy to defer to whatever is considered best practice by the Django community.

@PieterL75
Copy link
Contributor Author

@jeremystretch I think you should leave the option for the admin to choose what http header stores the real ip
Those 3 are indeed the most commen used, but sometimes, you are behind more than one proxy, and a custom headers is set to the first one, so that the backen applications always get the real source ip

@jeremystretch
Copy link
Member

That can easily be handled within the HTTP frontend configuration.

@jeremystretch
Copy link
Member

I'm going to close this out as it doesn't appear to be needed. Happy to revisit this if we receive actionable feedback for #8233 in the v3.3 release.

@jeremystretch jeremystretch closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2022
@PieterL75
Copy link
Contributor Author

I was actually thinking of this one today...
The best use case, is that admins want to set their own http header to prevent spoofed client ips, when commonly known headers are used.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending closure Requires immediate attention to avoid being closed for inactivity status: under review Further discussion is needed to determine this issue's scope and/or implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

2 participants