Implement network address sets

[stgraber]

One current missing piece of our networking puzzle is support for address sets.
That is, a named set of address and/or subnets which can be used as source/destination selector within ACLs.

There is a native OVN object for this feature, so it would make sense to initially implement this for OVN networking.

Address sets should be scoped the same way as ACLs as they'll be very closely linked. This means address sets should be part of a project when features.networks=true

The expected changes should roughly be:

• Allocate API extension
• New database table (schema change)
• New database functions (using the generator)
• Add REST API structs for network address sets
• Add OVN NB bindings for managing address sets
• Add REST API to manipulate address sets
• Add integration with OVN ACLs
• Implement CLI command
• Refresh translations
• Add tests

[irhndt]

Hello, I’m giving this a try, but I’m new to both Go and Incus development. I’m approaching this as an enthusiast and treating it as a learning exercise so you may want someone that is already proficient. I already have a few questions.

In internal/server/network/ovn/ovn_nb_actions.go, I noticed some bindings for address sets. Am I correct in assuming that the ones we need to implement are in driver_ovn.go?

Additionally, I’m currently exploring where to implement the REST API and believe I’ve identified the relevant structures for API definitions. To summarize, I’m focusing on implementing:

OVN NB bindings for managing address sets
A REST API to manipulate address sets

Any guidance would be appreciated.

[stgraber]

Oh, I completely forgot that we were already making use of address sets internally for the @internal and @external selectors in our ACLs, so that indeed means that the work in ovn_nb_actions.go shouldn't be necessary as those functions already exist.

[stgraber]

@irhndt I assigned this issue to you so we don't end up with someone else working on this in parallel. Let me know if you have any questions as you start digging into this one!

[irhndt]

@stgraber Thank you for assigning me this, I'll do my best.

I understand a bit better the code structure but still it's a huge and complex project <3.
I came with the following roadmap to help me focus and organize. However I still have some doubts on the right way to implement things and mostly where. I already worked on a few other files not mentioned in the roadmap such as internal/version/api.go or internal/lifecycle/network_address_set.go, mainly by grepping acl to find related files and assessing if I need a similar pattern for address sets.

I hope to begin tackle the main logic in the next few days, so before I take the big dive I would like to know if there is some precision for the phase 3 of my roadmap I should know before starting. Also if you spot some obvious mistakes or missing parts from previous phases let me know.

1. Database Schema:

   • Defined address_sets table in the fresh schema.
   • Added updateFromV75 migration function.
   • Updated the updates map with version 76.
   • Incremented SchemaVersion to 76.
   • Ran make update-schema to regenerate schema.go.

2. Database Layer:

   • Created the AddressSet struct.
   • Implemented CRUD operations in address_sets.go.

3. API and Logic Layer:

   • Defined API structures in shared/api/network_address_set.go.
   • Implemented handlers in internal/server/network/address_sets/.
   • Implement driver integration in internal/server/network/driver_*
   • Implement firewall integration either by a smart acl integration or direclty in internal/server/firewall looking forward to understand how ACL are used outside of OVN, also looking to understand how firewall backend integrates with OVN own filtering.

4. OVN Integration:

   • Ensured OVN NBCTL supports required operations.

5. CLI Integration:

   • Implemented CLI commands in cmd/incus/address-set.go.
   • Provided help texts and usage examples.

6. Translations:

   • Updated translation files with address set-related messages.
   • Ensured translations are available in all supported languages.

7. Testing:

   • Wrote unit tests for database operations.
   • Wrote tests for API interactions.
   • Wrote tests for CLI commands.
   • Verified OVN integration through testing.

8. Documentation:
   Think about doc/network-addr-sets.md

   • Updated API documentation in doc/reference and doc/manpages.
   • Created user guides in doc/howto.

9. Review and Validation:

   • Conducted code reviews for all new additions.
   • Performed security audits on the new feature.

[stgraber]

In 2), note that we also generate as much of the database access functions as possible.
This isn't implemented for every object so depending on what you used as an example you may have missed it.

network_integrations.go would be a good place to look for another network related feature which does have code generated database functions.

[irhndt]

Just a heads up, I got back on the development of the feature lately. For now I am missing:

• Enhanced testing for nftables (basics works)
• Testing for ovn
• Documentation

There should be some errors, but the feature is comming.

For the notice I made the choice to be able to use IP elements such as address and ranges in conjunction of address set in source / destination of ACLs. I will see if I can do the same for OVN while I test it.

[irhndt]

I am having trouble with blocking icmpv6 with address sets in netfilter and eventually even with direct ip6 address reference in ACL. Is there any limitation I might now be aware of ? Or simply I messed too much with nftables driver firewall code ? I'll move on to different test cases as pretty much everythin I have tested works so far but I am hanging on this peculiar case?

To add context, the cod correctly generates the following nft ruleset:

...
set testAS_ipv6 {
		type ipv6_addr
		flags interval
		elements = { fd42:743b:5370:a2b3:216:3eff:fe77:cf72 }
	}
...
chain acl.incusbr0 {
...
oifname "incusbr0" ip6 daddr @testAS_ipv6 ip6 nexthdr ipv6-icmp drop
..
}

However the icmpv6 packet is not blocked.