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

Restrict API key usage by source IP #8233

Closed
PieterL75 opened this issue Jan 5, 2022 · 11 comments
Closed

Restrict API key usage by source IP #8233

PieterL75 opened this issue Jan 5, 2022 · 11 comments
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Milestone

Comments

@PieterL75
Copy link
Contributor

NetBox version

v3.1.4

Feature type

Change to existing functionality

Proposed functionality

Limit the access to the API with a certain API key by source IPs

Use case

We have API keys that belong to 'service accounts'. The source IP of these services consumers are known.
We should be able to limit the usage of these service account API keys to only the known sources.
This cannot be done by firewalls but needs to go to the application layer

If an API key gets compromised, it cannot be used outside of that scope of source IPs

Database changes

API Keys

  • add multivalue 'Source IP'

External dependencies

No response

@PieterL75 PieterL75 added the type: feature Introduction of new functionality to the application label Jan 5, 2022
@DanSheps DanSheps added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Jan 5, 2022
@jeremystretch
Copy link
Member

This should be feasible to implement in our custom TokenAuthentication class. We currently override only the authenticate_credentials() method, which has access only to the token itself, but the authenticate() method receives the entire request. We just need a way to reference the requesting IP address when evaluating the credentials.

We'll also need to do a bit of grunt work around mapping allowed IPs (e.g. networks and/or lists of individual IPs), but I certainly hope we can manage that. 🙂

@hagbarddenstore
Copy link
Contributor

This should be feasible to implement in our custom TokenAuthentication class. We currently override only the authenticate_credentials() method, which has access only to the token itself, but the authenticate() method receives the entire request. We just need a way to reference the requesting IP address when evaluating the credentials.

We'll also need to do a bit of grunt work around mapping allowed IPs (e.g. networks and/or lists of individual IPs), but I certainly hope we can manage that. 🙂

Be aware that Python might not receive the "true" source IP because of being behind a proxy. It would be helpful if the documentation mentioned that where this feature is documented. Perhaps adding external links on how to configure passing remote IP in nginx, apache, etc.

https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2022

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 Mar 7, 2022
@PieterL75
Copy link
Contributor Author

Can we keep this open ? Our compliance team is asking about when this could be implemented

@jeremystretch
Copy link
Member

@PieterL75 would you like to volunteer to own it?

@PieterL75
Copy link
Contributor Author

PieterL75 commented Mar 10, 2022

I lack real coding ethics, but never afraid of a challenge...
I'll give it a try.

Must be in netbox/netbox/api/authentication.py ?

adding a check under TokenPermissions/has_permission() ?

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation pending closure Requires immediate attention to avoid being closed for inactivity labels Mar 10, 2022
@PieterL75
Copy link
Contributor Author

PieterL75 commented Mar 11, 2022

Looks like I have a working solution...
@jeremystretch Can you review this branch in my fork : https://github.com/PieterL75/netbox/tree/master
Or should I create a PR here

@jeremystretch
Copy link
Member

Given the substantial changes being introduced here (namely adding the allowed_ips field to the Token model), I'd like to tag this for v3.2.

@PieterL75 would you mind rebasing your PR on the feature branch? I don't think you'll run into much trouble but please let me know if I can be of assistance.

@DanSheps
Copy link
Member

DanSheps commented Apr 8, 2022

I am assuming we would want to shoot for v3.3 now?

@PieterL75
Copy link
Contributor Author

What about 3.2.1 ?
I'm off for a week, going to do the rebase when I'm back

@jeremystretch
Copy link
Member

It's an API change, so it'll need to go in the next minor release (v3.3).

@DanSheps DanSheps added this to the v3.3 milestone Apr 8, 2022
jeremystretch added a commit that referenced this issue Jun 22, 2022
Closes #8233: Restrict API key access by source IP
@jeremystretch jeremystretch self-assigned this Jun 22, 2022
jeremystretch added a commit that referenced this issue Jun 23, 2022
Closes #8233: Restrict API tokens by source IP
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants