search

netbox-community / netbox-acls

A NetBox plugin for Access Lists based off of the NetBox Plugin Demo
https://pypi.org/project/netbox-acls/
Apache License 2.0
90 stars 27 forks source link

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

Open fansari opened 1 year ago

fansari commented 1 year ago

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

github-actions[bot] commented 1 year ago

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

abhi1693 commented 1 year ago

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 commented 1 year ago

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

etfeet commented 6 months ago

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 - https://github.com/netbox-community/netbox/issues/10968 https://github.com/netbox-community/netbox/issues/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 commented 3 months ago

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 commented 3 months ago

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

killermoehre commented 3 months ago

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

rvveber commented 3 months ago

@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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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

Eldiabolo21 commented 1 month ago

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

Thanks for your work!

betadrome commented 1 month ago

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 commented 2 weeks ago

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 commented 1 week ago

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 commented 6 days ago

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