Merge pull request #1587 from stgraber/bridge-nic-acl

Support for ACLs for bridge NIC device when using nftables driver

Author: stgraber

doc/api-extensions.md

@@ -2685,3 +2685,7 @@ This adds extra fields to the OVN network state struct for the IPv4 and IPv6 add
 ## `qemu_scriptlet_config`
 
 This extends the QEMU scriptlet feature by allowing to modify QEMU configuration before a VM starts, and passing information about the instance to the scriptlet.
+
+## `network_bridge_acl_devices`
+
+This adds support for device ACLs when attached to a bridged network.

doc/howto/network_acls.md

@@ -210,8 +210,9 @@ incus config device set <instance_name> <device_name> security.acls.default.ingr
 When using network ACLs with a bridge network, be aware of the following limitations:
 
 - Unlike OVN ACLs, bridge ACLs are applied only on the boundary between the bridge and the Incus host.
-  This means they can only be used to apply network policies for traffic going to or from external networks.
+  This means they can only be used to apply network policies for traffic going to or from external networks (see exception for `nftables` firewall driver below).
   They cannot be used for to create {spellexception}`intra-bridge` firewalls, thus firewalls that control traffic between instances connected to the same bridge.
+- When using the `nftables` firewall driver you can apply ACLs to the NIC device and control traffic between the instances. In this case the `reject` ACL rules applied to the ingress traffic are converted to `drop` to address `nftables` limitation.
 - {ref}`ACL groups and network selectors <network-acls-selectors>` are not supported.
 - When using the `iptables` firewall driver, you cannot use IP range subjects (for example, `192.0.2.1-192.0.2.10`).
 - Baseline network service rules are added before ACL rules (in their respective INPUT/OUTPUT chains), because we cannot differentiate between INPUT/OUTPUT and FORWARD traffic once we have jumped into the ACL chain.

doc/reference/devices_nic.md

@@ -70,32 +70,37 @@ A `bridged` NIC uses an existing bridge on the host and creates a virtual device
 
 NIC devices of type `bridged` have the following device options:
 
-Key                      | Type    | Default           | Managed | Description
-:--                      | :--     | :--               | :--     | :--
-`boot.priority`          | integer | -                 | no      | Boot priority for VMs (higher value boots first)
-`host_name`              | string  | randomly assigned | no      | The name of the interface inside the host
-`hwaddr`                 | string  | randomly assigned | no      | The MAC address of the new interface
-`ipv4.address`           | string  | -                 | no      | An IPv4 address to assign to the instance through DHCP (can be `none` to restrict all IPv4 traffic when `security.ipv4_filtering` is set)
-`ipv4.routes`            | string  | -                 | no      | Comma-delimited list of IPv4 static routes to add on host to NIC
-`ipv4.routes.external`   | string  | -                 | no      | Comma-delimited list of IPv4 static routes to route to the NIC and publish on uplink network (BGP)
-`ipv6.address`           | string  | -                 | no      | An IPv6 address to assign to the instance through DHCP (can be `none` to restrict all IPv6 traffic when `security.ipv6_filtering` is set)
-`ipv6.routes`            | string  | -                 | no      | Comma-delimited list of IPv6 static routes to add on host to NIC
-`ipv6.routes.external`   | string  | -                 | no      | Comma-delimited list of IPv6 static routes to route to the NIC and publish on uplink network (BGP)
-`limits.egress`          | string  | -                 | no      | I/O limit in bit/s for outgoing traffic (various suffixes supported, see {ref}`instances-limit-units`)
-`limits.ingress`         | string  | -                 | no      | I/O limit in bit/s for incoming traffic (various suffixes supported, see {ref}`instances-limit-units`)
-`limits.max`             | string  | -                 | no      | I/O limit in bit/s for both incoming and outgoing traffic (same as setting both `limits.ingress` and `limits.egress`)
-`limits.priority`        | integer | -                 | no      | The `skb->priority` value (32-bit unsigned integer) for outgoing traffic, to be used by the kernel queuing discipline (qdisc) to prioritize network packets (The effect of this value depends on the particular qdisc implementation, for example, `SKBPRIO` or `QFQ`. Consult the kernel qdisc documentation before setting this value.)
-`mtu`                    | integer | parent MTU        | yes     | The MTU of the new interface
-`name`                   | string  | kernel assigned   | no      | The name of the interface inside the instance
-`network`                | string  | -                 | no      | The managed network to link the device to (instead of specifying the `nictype` directly)
-`parent`                 | string  | -                 | yes     | The name of the host device (required if specifying the `nictype` directly)
-`queue.tx.length`        | integer | -                 | no      | The transmit queue length for the NIC
-`security.ipv4_filtering`| bool    | `false`           | no      | Prevent the instance from spoofing another instance's IPv4 address (enables `security.mac_filtering`)
-`security.ipv6_filtering`| bool    | `false`           | no      | Prevent the instance from spoofing another instance's IPv6 address (enables `security.mac_filtering`)
-`security.mac_filtering` | bool    | `false`           | no      | Prevent the instance from spoofing another instance's MAC address
-`security.port_isolation`| bool    | `false`           | no      | Prevent the NIC from communicating with other NICs in the network that have port isolation enabled
-`vlan`                   | integer | -                 | no      | The VLAN ID to use for non-tagged traffic (can be `none` to remove port from default VLAN)
-`vlan.tagged`            | integer | -                 | no      | Comma-delimited list of VLAN IDs or VLAN ranges to join for tagged traffic
+Key                                   | Type    | Default           | Managed | Description
+:--                                   | :--     | :--               | :--     | :--
+`boot.priority`                       | integer | -                 | no      | Boot priority for VMs (higher value boots first)
+`host_name`                           | string  | randomly assigned | no      | The name of the interface inside the host
+`hwaddr`                              | string  | randomly assigned | no      | The MAC address of the new interface
+`ipv4.address`                        | string  | -                 | no      | An IPv4 address to assign to the instance through DHCP (can be `none` to restrict all IPv4 traffic when `security.ipv4_filtering` is set)
+`ipv4.routes`                         | string  | -                 | no      | Comma-delimited list of IPv4 static routes to add on host to NIC
+`ipv4.routes.external`                | string  | -                 | no      | Comma-delimited list of IPv4 static routes to route to the NIC and publish on uplink network (BGP)
+`ipv6.address`                        | string  | -                 | no      | An IPv6 address to assign to the instance through DHCP (can be `none` to restrict all IPv6 traffic when `security.ipv6_filtering` is set)
+`ipv6.routes`                         | string  | -                 | no      | Comma-delimited list of IPv6 static routes to add on host to NIC
+`ipv6.routes.external`                | string  | -                 | no      | Comma-delimited list of IPv6 static routes to route to the NIC and publish on uplink network (BGP)
+`limits.egress`                       | string  | -                 | no      | I/O limit in bit/s for outgoing traffic (various suffixes supported, see {ref}`instances-limit-units`)
+`limits.ingress`                      | string  | -                 | no      | I/O limit in bit/s for incoming traffic (various suffixes supported, see {ref}`instances-limit-units`)
+`limits.max`                          | string  | -                 | no      | I/O limit in bit/s for both incoming and outgoing traffic (same as setting both `limits.ingress` and `limits.egress`)
+`limits.priority`                     | integer | -                 | no      | The `skb->priority` value (32-bit unsigned integer) for outgoing traffic, to be used by the kernel queuing discipline (qdisc) to prioritize network packets (The effect of this value depends on the particular qdisc implementation, for example, `SKBPRIO` or `QFQ`. Consult the kernel qdisc documentation before setting this value.)
+`mtu`                                 | integer | parent MTU        | yes     | The MTU of the new interface
+`name`                                | string  | kernel assigned   | no      | The name of the interface inside the instance
+`network`                             | string  | -                 | no      | The managed network to link the device to (instead of specifying the `nictype` directly)
+`parent`                              | string  | -                 | yes     | The name of the host device (required if specifying the `nictype` directly)
+`queue.tx.length`                     | integer | -                 | no      | The transmit queue length for the NIC
+`security.acls`                       | string  | -                 | no      | Comma-separated list of network ACLs to apply
+`security.acls.default.egress.action` | string  | `drop`            | no      | Action to use for egress traffic that doesn't match any ACL rule
+`security.acls.default.egress.logged` | bool    | `false`           | no      | Whether to log egress traffic that doesn't match any ACL rule
+`security.acls.default.ingress.action`| string  | `drop`            | no      | Action to use for ingress traffic that doesn't match any ACL rule
+`security.acls.default.ingress.logged`| bool    | `false`           | no      | Whether to log ingress traffic that doesn't match any ACL rule
+`security.ipv4_filtering`             | bool    | `false`           | no      | Prevent the instance from spoofing another instance's IPv4 address (enables `security.mac_filtering`)
+`security.ipv6_filtering`             | bool    | `false`           | no      | Prevent the instance from spoofing another instance's IPv6 address (enables `security.mac_filtering`)
+`security.mac_filtering`              | bool    | `false`           | no      | Prevent the instance from spoofing another instance's MAC address
+`security.port_isolation`             | bool    | `false`           | no      | Prevent the NIC from communicating with other NICs in the network that have port isolation enabled
+`vlan`                                | integer | -                 | no      | The VLAN ID to use for non-tagged traffic (can be `none` to remove port from default VLAN)
+`vlan.tagged`                         | integer | -                 | no      | Comma-delimited list of VLAN IDs or VLAN ranges to join for tagged traffic
 
 (nic-macvlan)=
 ### `nictype`: `macvlan`

internal/server/device/nic_bridged.go

@@ -26,10 +26,13 @@ import (
 	deviceConfig "github.com/lxc/incus/v6/internal/server/device/config"
 	"github.com/lxc/incus/v6/internal/server/dnsmasq"
 	"github.com/lxc/incus/v6/internal/server/dnsmasq/dhcpalloc"
+	firewallDrivers "github.com/lxc/incus/v6/internal/server/firewall/drivers"
 	"github.com/lxc/incus/v6/internal/server/instance"
 	"github.com/lxc/incus/v6/internal/server/instance/instancetype"
 	"github.com/lxc/incus/v6/internal/server/ip"
 	"github.com/lxc/incus/v6/internal/server/network"
+	"github.com/lxc/incus/v6/internal/server/network/acl"
+	"github.com/lxc/incus/v6/internal/server/project"
 	"github.com/lxc/incus/v6/internal/server/resources"
 	localUtil "github.com/lxc/incus/v6/internal/server/util"
 	internalUtil "github.com/lxc/incus/v6/internal/util"
@@ -89,6 +92,11 @@ func (d *nicBridged) validateConfig(instConf instance.ConfigReader) error {
 		"security.ipv4_filtering",
 		"security.ipv6_filtering",
 		"security.port_isolation",
+		"security.acls",
+		"security.acls.default.ingress.action",
+		"security.acls.default.egress.action",
+		"security.acls.default.ingress.logged",
+		"security.acls.default.egress.logged",
 		"boot.priority",
 		"vlan",
 	}
@@ -286,6 +294,24 @@ func (d *nicBridged) validateConfig(instConf instance.ConfigReader) error {
 		}
 	}
 
+	// Check if security ACL(s) are configured.
+	if d.config["security.acls"] != "" {
+		if d.state.Firewall.String() != "nftables" {
+			return fmt.Errorf("Security ACLs are only supported when using nftables firewall")
+		}
+
+		// The NIC's network may be a non-default project, so lookup project and get network's project name.
+		networkProjectName, _, err := project.NetworkProject(d.state.DB.Cluster, instConf.Project().Name)
+		if err != nil {
+			return fmt.Errorf("Failed loading network project name: %w", err)
+		}
+
+		err = acl.Exists(d.state, networkProjectName, util.SplitNTrimSpace(d.config["security.acls"], ",", -1, true)...)
+		if err != nil {
+			return err
+		}
+	}
+
 	rules := nicValidationRules(requiredFields, optionalFields, instConf)
 
 	// Add bridge specific vlan validation.
@@ -445,7 +471,7 @@ func (d *nicBridged) UpdatableFields(oldDevice Type) []string {
 		return []string{}
 	}
 
-	return []string{"limits.ingress", "limits.egress", "limits.max", "limits.priority", "ipv4.routes", "ipv6.routes", "ipv4.routes.external", "ipv6.routes.external", "ipv4.address", "ipv6.address", "security.mac_filtering", "security.ipv4_filtering", "security.ipv6_filtering"}
+	return []string{"limits.ingress", "limits.egress", "limits.max", "limits.priority", "ipv4.routes", "ipv6.routes", "ipv4.routes.external", "ipv6.routes.external", "ipv4.address", "ipv6.address", "security.mac_filtering", "security.ipv4_filtering", "security.ipv6_filtering", "security.acls", "security.acls.default.egress.action", "security.acls.default.egress.logged", "security.acls.default.ingress.action", "security.acls.default.ingress.logged"}
 }
 
 // Add is run when a device is added to a non-snapshot instance whether or not the instance is running.
@@ -851,7 +877,7 @@ func (d *nicBridged) postStop() error {
 	routes = append(routes, util.SplitNTrimSpace(d.config["ipv6.routes.external"], ",", -1, true)...)
 	networkNICRouteDelete(bridgeName, routes...)
 
-	if util.IsTrue(d.config["security.mac_filtering"]) || util.IsTrue(d.config["security.ipv4_filtering"]) || util.IsTrue(d.config["security.ipv6_filtering"]) {
+	if util.IsTrue(d.config["security.mac_filtering"]) || util.IsTrue(d.config["security.ipv4_filtering"]) || util.IsTrue(d.config["security.ipv6_filtering"]) || d.config["security.acls"] != "" {
 		d.removeFilters(d.config)
 	}
 
@@ -964,12 +990,12 @@ func (d *nicBridged) setupHostFilters(oldConfig deviceConfig.Device) (revert.Hoo
 	}
 
 	// Remove any old network filters if non-empty oldConfig supplied as part of update.
-	if oldConfig != nil && (util.IsTrue(oldConfig["security.mac_filtering"]) || util.IsTrue(oldConfig["security.ipv4_filtering"]) || util.IsTrue(oldConfig["security.ipv6_filtering"])) {
+	if oldConfig != nil && (util.IsTrue(oldConfig["security.mac_filtering"]) || util.IsTrue(oldConfig["security.ipv4_filtering"]) || util.IsTrue(oldConfig["security.ipv6_filtering"]) || oldConfig["security.acls"] != "") {
 		d.removeFilters(oldConfig)
 	}
 
 	// Setup network filters.
-	if util.IsTrue(d.config["security.mac_filtering"]) || util.IsTrue(d.config["security.ipv4_filtering"]) || util.IsTrue(d.config["security.ipv6_filtering"]) {
+	if util.IsTrue(d.config["security.mac_filtering"]) || util.IsTrue(d.config["security.ipv4_filtering"]) || util.IsTrue(d.config["security.ipv6_filtering"]) || d.config["security.acls"] != "" {
 		err := d.setFilters()
 		if err != nil {
 			return nil, err
@@ -1059,7 +1085,7 @@ func (d *nicBridged) removeFilters(m deviceConfig.Device) {
 }
 
 // setFilters sets up any network level filters defined for the instance.
-// These are controlled by the security.mac_filtering, security.ipv4_Filtering and security.ipv6_filtering config keys.
+// These are controlled by the security.mac_filtering, security.ipv4_Filtering, security.ipv6_filtering and security.acls config keys.
 func (d *nicBridged) setFilters() (err error) {
 	if d.config["hwaddr"] == "" {
 		return fmt.Errorf("Failed to set network filters: require hwaddr defined")
@@ -1149,7 +1175,16 @@ func (d *nicBridged) setFilters() (err error) {
 		return err
 	}
 
-	err = d.state.Firewall.InstanceSetupBridgeFilter(d.inst.Project().Name, d.inst.Name(), d.name, d.config["parent"], d.config["host_name"], d.config["hwaddr"], IPv4Nets, IPv6Nets, d.network != nil)
+	var aclRules []firewallDrivers.ACLRule
+
+	if config["security.acls"] != "" {
+		aclRules, err = acl.FirewallACLRules(d.state, d.name, d.inst.Project().Name, d.config)
+		if err != nil {
+			return err
+		}
+	}
+
+	err = d.state.Firewall.InstanceSetupBridgeFilter(d.inst.Project().Name, d.inst.Name(), d.name, d.config["parent"], d.config["host_name"], d.config["hwaddr"], IPv4Nets, IPv6Nets, d.network != nil, util.IsTrue(config["security.mac_filtering"]), aclRules)
 	if err != nil {
 		return err
 	}