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

[Feature]: source and destination range for acl have to be a prefix in Netbox? #129

Open
fansari opened this issue Feb 2, 2023 · 17 comments · May be fixed by #207
Open

[Feature]: source and destination range for acl have to be a prefix in Netbox? #129

fansari opened this issue Feb 2, 2023 · 17 comments · May be fixed by #207
Labels
enhancement New feature or request

Comments

@fansari
Copy link

fansari commented Feb 2, 2023

NetBox version

v3.4.3

Feature type

New Model to plugin

Proposed functionality

I had a look on this plugin for our use case and one thing I noticed (except that as already mentioned ACLs are bound to devices) is that if you want to use a source or destination range it has to be part of "prefixes". I did not find a way to use an aggregate or a host or any IP range not defined in Netbox.

The only way I found to forbid e.g. a bogus IP range like 192.0.2.0/24 was to add a prefix for this in Netbox.

Also with hosts: if I want to create a rule for example to allow access to a single host it is not possible except I create an additional prefix for this.

Is there a special reason why every source or destination range has to be an exsting prefix?

Use case

You could setup source and destinations with any IP range regardless whether they exist in Netbox or not.

External dependencies

don't know about dependencies for this

@fansari fansari added the enhancement New feature or request label Feb 2, 2023
@github-actions
Copy link

github-actions bot commented Feb 2, 2023

Thanks for opening this Issue! We really appreciate the feedback & testing from users like you!

@abhi1693
Copy link
Member

abhi1693 commented Feb 2, 2023

I let Ryan comment on the questions asked in this. But IMO, even if ip addresses are to be used, we should use them via the ipam > ipaddress model as a foreign key. I'd discourage the use of char fields for adding any arbitrary ip addresses.

@cyberndj
Copy link

+1 for adding IPs as a src or dst item. We have alot of ACLs that have host IPs listed in entries.

@etfeet
Copy link

etfeet commented May 14, 2024

also of note, as a result of the source/destination object being required to be a netbox "prefix" object you can't represent a default any prefix object. Netbox does not allow you to create a prefix to represent any that is 0.0.0.0/0 prefix/netmask combination. There have been several requests asking for that functionality that were rejected. so that is unlikely to change.

adding support for /0 netmask was discussed/rejected here -
netbox-community/netbox#10968
netbox-community/netbox#6825

as a result you can't configure the following policy/rule with netbox-acl:

source: any / 0.0.0.0/0
destination: 192.168.1.0/24
destination port: 22
protocol: tcp
action: deny

@betadrome
Copy link

also of note, as a result of the source/destination object being required to be a netbox "prefix" object you can't represent a default any prefix object. Netbox does not allow you to create a prefix to represent any that is 0.0.0.0/0 prefix/netmask combination. There have been several requests asking for that functionality that were rejected. so that is unlikely to change.

adding support for /0 netmask was discussed/rejected here - netbox-community/netbox#10968 netbox-community/netbox#6825

as a result you can't configure the following policy/rule with netbox-acl:

source: any / 0.0.0.0/0
destination: 192.168.1.0/24
destination port: 22
protocol: tcp
action: deny

Hey, you can leave the Source/Destination-Prefix/Port field blank and then you have your "Any" rule.

@rvveber
Copy link

rvveber commented Aug 7, 2024

I'd like to work on this feature (ideally implement it for v1.5.0 and newer), can it be approved and assigned to me?
@ryanmerolle
@abhi1693

rvveber added a commit to rvveber/netbox-acls that referenced this issue Aug 7, 2024
@killermoehre
Copy link

Being able to select a Service as target instead of an IP address and port would be awesome as well.

@rvveber
Copy link

rvveber commented Aug 28, 2024

@ryanmerolle
@abhi1693
@cruse1977

Right now i'm simply adding source_<model> and destination_<model> fields, that each link to their respective model.
And i'm setting form- and model constraints, so that only one source/destination can be set.
However, in the Interface Assignment, i discovered how you implemented dynamic association through assigned_object.
I could replicate this behavior for the source and destination, but it would likely get unnecessarily complicated, and the changes to the API would not be backwards compatible.

I'm proceeding with my approach for now, but let me know, if the assigned_object approach is preferred!

@rvveber rvveber linked a pull request Sep 2, 2024 that will close this issue
2 tasks
@rvveber
Copy link

rvveber commented Sep 3, 2024

@ryanmerolle
@abhi1693
@cruse1977

I have completed the work for this feature.
Please have a look at the merge request above :)

(I was not able to backport it to 1.5.0. I think we would need version specific release branches for that)

@cruse1977
Copy link
Member

hi @rvveber will look to review this next week - thanks for your submission, please note for future commits please don't bump the version number, more than likely this will go into a 1.7.0 release (for NetBox 4.1)

@rvveber
Copy link

rvveber commented Sep 10, 2024

Great! Thanks!
(I have removed the version bump for your convenience)

@Eldiabolo21
Copy link

Hi people, is there a blocker to this PR? I would love to have this merged.

Thanks for your work!

@betadrome
Copy link

Hi people, is there a blocker to this PR? I would love to have this merged.

Thanks for your work!

+1. We also need this Feature very urgently!

@rvveber
Copy link

rvveber commented Nov 12, 2024

hi @rvveber will look to review this next week - thanks for your submission, please note for future commits please don't bump the version number, more than likely this will go into a 1.7.0 release (for NetBox 4.1)

I really don't want to be rude, but i put a lot of effort into the solution 2 months ago, and it's kind of important that it will be available before the end of this year.
@cruse1977 Could you please consider merging and releasing?

@betadrome
Copy link

Hey @jeremystretch, please can you take a look on this? We need this function as soon as possible, but are unfortunately dependent on the merge request. Many thanks in advance!

@granidier99
Copy link

Do we have update on this? I urgently need this feature

@rvveber
Copy link

rvveber commented Dec 6, 2024

@ryanmerolle may i ask to help you maintain this project?
Looks like the maintainers are too busy and contributions have come to a halt.
Maybe because the development environment is in a non functional state
and maybe because the big refactor you planned was a little too big.
I could provide a nix based development environment,
removing the need for docker, vscode plugins and other dependencies entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants