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

Support for ACLs for bridge NIC device when using nftables driver. #1225

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/howto/network_acls.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 are not permitted and when the default action is set to `reject` it is interpreted as `drop`.
- {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.
Expand Down
57 changes: 31 additions & 26 deletions doc/reference/devices_nic.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
3 changes: 3 additions & 0 deletions internal/server/device/config/devices.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ func (list Devices) Update(newlist Devices, updateFields func(Device, Device) []

allChangedKeys := []string{}
for key, d := range addlist {
// Remove the "force_update" key if present.
delete(d, "force_update")

srcOldDevice := rmlist[key]
oldDevice := srcOldDevice.Clone()

Expand Down
47 changes: 41 additions & 6 deletions internal/server/device/nic_bridged.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,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"
Expand Down Expand Up @@ -87,6 +90,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",
}
Expand Down Expand Up @@ -284,6 +292,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.
Expand Down Expand Up @@ -443,7 +469,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.
Expand Down Expand Up @@ -843,7 +869,7 @@ func (d *nicBridged) postStop() error {
routes = append(routes, util.SplitNTrimSpace(d.config["ipv6.routes.external"], ",", -1, true)...)
networkNICRouteDelete(d.config["parent"], 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)
}

Expand Down Expand Up @@ -950,12 +976,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
Expand Down Expand Up @@ -1045,7 +1071,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")
Expand Down Expand Up @@ -1135,7 +1161,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
}
Expand Down
Loading
Loading