[lxc-devel] [lxd/master] Network: OVN NIC external route overlap validation
tomponline on Github
lxc-bot at linuxcontainers.org
Wed Oct 28 17:44:40 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 703 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201028/fff0790d/attachment.bin>
-------------- next part --------------
From 4c1e883f9dbb88e9e4850ae9c98e5a23839d3f74 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 28 Oct 2020 14:10:00 +0000
Subject: [PATCH 1/6] lxd/project/project: Updates
StorageVolumeProjectFromRecord to not return error (as never populated)
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/project/project.go | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lxd/project/project.go b/lxd/project/project.go
index d7d40479fa..f1dbf556b5 100644
--- a/lxd/project/project.go
+++ b/lxd/project/project.go
@@ -79,26 +79,26 @@ func StorageVolumeProject(c *db.Cluster, projectName string, volumeType int) (st
return "", errors.Wrapf(err, "Failed to load project %q", projectName)
}
- return StorageVolumeProjectFromRecord(project, volumeType)
+ return StorageVolumeProjectFromRecord(project, volumeType), nil
}
-// StorageVolumeProjectFromRecord returns the project name to use to for the volume based on the requested project.
-// For custom volume type, if the project specified has the "features.storage.volumes" flag enabled then the
+// StorageVolumeProjectFromRecord returns the project name to use to for the volume based on the supplied project.
+// For custom volume type, if the project supplied has the "features.storage.volumes" flag enabled then the
// project name is returned, otherwise the default project name is returned. For all other volume types the
// supplied project's name is returned.
-func StorageVolumeProjectFromRecord(p *api.Project, volumeType int) (string, error) {
+func StorageVolumeProjectFromRecord(p *api.Project, volumeType int) string {
// Non-custom volumes always use the project specified.
if volumeType != db.StoragePoolVolumeTypeCustom {
- return p.Name, nil
+ return p.Name
}
// Custom volumes only use the project specified if the project has the features.storage.volumes feature
// enabled, otherwise the legacy behaviour of using the default project for custom volumes is used.
if shared.IsTrue(p.Config["features.storage.volumes"]) {
- return p.Name, nil
+ return p.Name
}
- return Default, nil
+ return Default
}
// NetworkProject returns the project name to use for the network based on the requested project.
From 94a12db197d93e724177c6e883aa2c8c4d77d66b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 28 Oct 2020 14:10:51 +0000
Subject: [PATCH 2/6] lxd/project/project: Adds NetworkProjectFromRecord
function
And updates NetworkProject to use it.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/project/project.go | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/lxd/project/project.go b/lxd/project/project.go
index f1dbf556b5..202f059650 100644
--- a/lxd/project/project.go
+++ b/lxd/project/project.go
@@ -106,16 +106,29 @@ func StorageVolumeProjectFromRecord(p *api.Project, volumeType int) string {
// otherwise the default project name is returned. The second return value is the project's config if non-default
// project is being returned, nil if not.
func NetworkProject(c *db.Cluster, projectName string) (string, map[string]string, error) {
- project, err := c.GetProject(projectName)
+ p, err := c.GetProject(projectName)
if err != nil {
return "", nil, errors.Wrapf(err, "Failed to load project %q", projectName)
}
+ projectName = NetworkProjectFromRecord(p)
+
+ if projectName != Default {
+ return projectName, p.Config, nil
+ }
+
+ return Default, nil, nil
+}
+
+// NetworkProjectFromRecord returns the project name to use for the network based on the supplied project.
+// If the project supplied has the "features.networks" flag enabled then the project name is returned,
+// otherwise the default project name is returned.
+func NetworkProjectFromRecord(p *api.Project) string {
// Networks only use the project specified if the project has the features.networks feature enabled,
// otherwise the legacy behaviour of using the default project for networks is used.
- if shared.IsTrue(project.Config["features.networks"]) {
- return projectName, project.Config, nil
+ if shared.IsTrue(p.Config["features.networks"]) {
+ return p.Name
}
- return Default, nil, nil
+ return Default
}
From b6c2647d92e2278444bd4201afc5eeb79bdf575a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 28 Oct 2020 17:38:16 +0000
Subject: [PATCH 3/6] Applying: lxd/storage/utils:
project.StorageVolumeProjectFromRecord usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/utils.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index d4f40b0dc4..e98ff029bf 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -701,7 +701,7 @@ func VolumeUsedByInstances(s *state.State, poolName string, projectName string,
volumeNameWithType := fmt.Sprintf("%s/%s", volumeTypeName, volumeName)
return s.Cluster.InstanceList(func(inst db.Instance, p api.Project, profiles []api.Profile) error {
- instStorageProject, err := project.StorageVolumeProjectFromRecord(&p, volumeType)
+ instStorageProject := project.StorageVolumeProjectFromRecord(&p, volumeType)
if err != nil {
return err
}
From 8c3aae2040633ca921f1d362554ce8bc9913a962 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 28 Oct 2020 17:38:43 +0000
Subject: [PATCH 4/6] lxd/network/driver/ovn: Adds NIC external route overlap
validation of other OVN external network subnets and OVN NIC external routes
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/driver_ovn.go | 193 ++++++++++++++++++++++++++++++++++++--
1 file changed, 185 insertions(+), 8 deletions(-)
diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index bfda8d7ba7..81f3ebf7a7 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -17,6 +17,8 @@ import (
"github.com/lxc/lxd/lxd/cluster"
"github.com/lxc/lxd/lxd/db"
+ deviceConfig "github.com/lxc/lxd/lxd/device/config"
+ "github.com/lxc/lxd/lxd/instance"
"github.com/lxc/lxd/lxd/locking"
"github.com/lxc/lxd/lxd/network/openvswitch"
"github.com/lxc/lxd/lxd/project"
@@ -1869,12 +1871,10 @@ func (n *ovn) getInstanceDevicePortName(instanceID int, deviceName string) openv
}
// InstanceDevicePortValidateExternalRoutes validates the external routes for an OVN instance port.
-func (n *ovn) InstanceDevicePortValidateExternalRoutes(externalRoutes []*net.IPNet) error {
- // Load the project to get uplink network restrictions.
- p, err := n.state.Cluster.GetProject(n.project)
- if err != nil {
- return errors.Wrapf(err, "Failed to load network restrictions from project %q", n.project)
- }
+func (n *ovn) InstanceDevicePortValidateExternalRoutes(deviceInstance instance.Instance, deviceName string, portExternalRoutes []*net.IPNet) error {
+ var err error
+ var p *api.Project
+ var projectNetworks map[string]map[int64]api.Network
// Get uplink routes.
uplinkRoutes, err := n.uplinkRoutes(n.config["network"])
@@ -1882,19 +1882,91 @@ func (n *ovn) InstanceDevicePortValidateExternalRoutes(externalRoutes []*net.IPN
return err
}
+ err = n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
+ // Load the project to get uplink network restrictions.
+ p, err = tx.GetProject(n.project)
+ if err != nil {
+ return errors.Wrapf(err, "Failed to load network restrictions from project %q", n.project)
+ }
+
+ // Get all managed networks across all projects.
+ projectNetworks, err = tx.GetNonPendingNetworks()
+ if err != nil {
+ return errors.Wrapf(err, "Failed to load all networks")
+ }
+
+ return nil
+ })
+ if err != nil {
+ return err
+ }
+
+ // Work out which OVN networks (in all projects and including our own) use the same uplink as us.
+ ovnProjectNetworksWithOurUplink := make(map[string][]*api.Network)
+ for netProject, networks := range projectNetworks {
+ for _, network := range networks {
+ netInfo := network // Local var for adding pointer to ovnProjectNetworksWithOurUplink.
+ // Skip non-OVN networks or those networks that don't use the same uplink as us.
+ if netInfo.Type != "ovn" || netInfo.Config["network"] != n.config["network"] {
+ continue
+ }
+
+ if ovnProjectNetworksWithOurUplink[netProject] == nil {
+ ovnProjectNetworksWithOurUplink[netProject] = []*api.Network{&netInfo}
+ } else {
+ ovnProjectNetworksWithOurUplink[netProject] = append(ovnProjectNetworksWithOurUplink[netProject], &netInfo)
+ }
+ }
+ }
+
+ // Get external subnets used by other OVN networks using our uplink.
+ ovnNetworkExternalSubnets, err := n.ovnNetworkExternalSubnets(ovnProjectNetworksWithOurUplink, uplinkRoutes)
+ if err != nil {
+ return err
+ }
+
// Get project restricted routes.
projectRestrictedSubnets, err := n.projectRestrictedSubnets(p, n.config["network"])
if err != nil {
return err
}
- for _, externalRoute := range externalRoutes {
- err = n.validateExternalSubnet(uplinkRoutes, projectRestrictedSubnets, externalRoute)
+ // If validating with an instance, get external routes configured on OVN NICs (excluding ours) using
+ // networks that use our uplink.
+ var ovnNICExternalRoutes []*net.IPNet
+ if deviceInstance != nil {
+ ovnNICExternalRoutes, err = n.ovnNICExternalRoutes(deviceInstance, deviceName, ovnProjectNetworksWithOurUplink)
if err != nil {
return err
}
}
+ for _, portExternalRoute := range portExternalRoutes {
+ // Check the external port route is allowed within both the uplink's external routes and any
+ // project restricted subnets.
+ err = n.validateExternalSubnet(uplinkRoutes, projectRestrictedSubnets, portExternalRoute)
+ if err != nil {
+ return err
+ }
+
+ // Check the external port route doesn't fall within any existing OVN network external subnets.
+ for _, ovnNetworkExternalSubnet := range ovnNetworkExternalSubnets {
+ if SubnetContains(ovnNetworkExternalSubnet, portExternalRoute) || SubnetContains(portExternalRoute, ovnNetworkExternalSubnet) {
+ // This error is purposefully vague so that it doesn't reveal any names of
+ // resources potentially outside of the NIC's project.
+ return fmt.Errorf("External route %q overlaps with another OVN network's external subnet", portExternalRoute.String())
+ }
+ }
+
+ for _, ovnNICExternalRoute := range ovnNICExternalRoutes {
+ if SubnetContains(ovnNICExternalRoute, portExternalRoute) || SubnetContains(portExternalRoute, ovnNICExternalRoute) {
+ // This error is purposefully vague so that it doesn't reveal any names of
+ // resources potentially outside of the NIC's project.
+ return fmt.Errorf("External route %q overlaps with another OVN NIC's external route", portExternalRoute.String())
+ }
+ }
+ }
+
return nil
}
@@ -2187,3 +2259,108 @@ func (n *ovn) DHCPv6Subnet() *net.IPNet {
return subnet
}
+
+// ovnNetworkExternalSubnets returns a list of external subnets used by OVN networks (including our own) using the
+// same uplink as this OVN network. OVN networks are considered to be using external subnets if their ipv4.address
+// and/or ipv6.address are in the uplink's external routes and the associated NAT is disabled for the IP family.
+func (n *ovn) ovnNetworkExternalSubnets(tx *db.ClusterTx, uplinkRoutes []*net.IPNet) ([]*net.IPNet, error) {
+ // Get all managed networks across all projects.
+ projectNetworks, err := tx.GetNonPendingNetworks()
+ if err != nil {
+ return nil, errors.Wrapf(err, "Failed to load all networks")
+ }
+
+ externalSubnets := make([]*net.IPNet, 0)
+ for _, networks := range projectNetworks {
+ for _, netInfo := range networks {
+ // Skip non-OVN networks or those networks that don't use the same uplink as us.
+ if netInfo.Type != "ovn" || netInfo.Config["network"] != n.config["network"] {
+ continue
+ }
+
+ for _, keyPrefix := range []string{"ipv4", "ipv6"} {
+ if !shared.IsTrue(netInfo.Config[fmt.Sprintf("%s.nat", keyPrefix)]) {
+ _, ipNet, _ := net.ParseCIDR(netInfo.Config[fmt.Sprintf("%s.address", keyPrefix)])
+ if ipNet == nil {
+ // If the network doesn't have a valid IP, skip it.
+ continue
+ }
+
+ // Check the network's subnet is a valid external route on uplink.
+ err = n.validateExternalSubnet(uplinkRoutes, nil, ipNet)
+ if err != nil {
+ return nil, errors.Wrapf(err, "Failed checking if OVN network external subnet %q is valid external route on uplink %q", ipNet.String(), n.config["network"])
+ }
+
+ externalSubnets = append(externalSubnets, ipNet)
+ }
+ }
+ }
+ }
+
+ return externalSubnets, nil
+}
+
+// ovnNICExternalRoutes returns a list of external routes currently used by OVN NICs (excluding our own) that are
+// connected to OVN networks that share the same uplink as this network uses.
+func (n *ovn) ovnNICExternalRoutes(ourDeviceInstance instance.Instance, ourDeviceName string, ovnProjectNetworksWithOurUplink map[string][]*api.Network) ([]*net.IPNet, error) {
+ externalRoutes := make([]*net.IPNet, 0)
+
+ // nicUsesNetwork returns true if the nicDev's "network" property matches one of the projectNetworks names
+ // and the instNetworkProject matches the projectNetworks's project. As we only use network name and
+ // project to match we rely on projectNetworks only including OVN networks that use our uplink.
+ nicUsesNetwork := func(instNetworkProject string, nicDev map[string]string, projectNetworks map[string][]*api.Network) bool {
+ for netProject, networks := range projectNetworks {
+ for _, network := range networks {
+ if netProject == instNetworkProject && network.Name == nicDev["network"] {
+ return true
+ }
+ }
+ }
+
+ return false
+ }
+
+ err := n.state.Cluster.InstanceList(func(inst db.Instance, p api.Project, profiles []api.Profile) error {
+ // Get the instance's effective network project name.
+ instNetworkProject := project.NetworkProjectFromRecord(&p)
+ devices := db.ExpandInstanceDevices(deviceConfig.NewDevices(inst.Devices), profiles).CloneNative()
+
+ // Iterate through each of the instance's devices, looking for OVN NICs that are linked to networks
+ // that use our uplink.
+ for devName, devConfig := range devices {
+ if devConfig["type"] != "nic" {
+ continue
+ }
+
+ // Skip our own device.
+ if ourDeviceInstance != nil && inst.Name == ourDeviceInstance.Name() && inst.Project == ourDeviceInstance.Project() && ourDeviceName == devName {
+ continue
+ }
+
+ // Check whether the NIC device references one of the OVN networks supplied.
+ if !nicUsesNetwork(instNetworkProject, devConfig, ovnProjectNetworksWithOurUplink) {
+ continue
+ }
+
+ // For OVN NICs that are connected to networks that use the same uplink as we do, check
+ // if they have any external routes configured, and if so add them to the list to return.
+ for _, keyPrefix := range []string{"ipv4", "ipv6"} {
+ _, ipNet, _ := net.ParseCIDR(devConfig[fmt.Sprintf("%s.routes.external", keyPrefix)])
+ if ipNet == nil {
+ // If the NIC device doesn't have a valid external route setting, skip.
+ continue
+ }
+
+ externalRoutes = append(externalRoutes, ipNet)
+ }
+ }
+
+ return nil
+ })
+ if err != nil {
+ return nil, err
+ }
+
+ return externalRoutes, nil
+}
From e9e411a061695d23245eeed25742b3d8ba1e9e21 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 28 Oct 2020 17:39:26 +0000
Subject: [PATCH 5/6] lxd/device/nic/ovn: Updates ovnNet interface's
InstanceDevicePortValidateExternalRoutes to add instance argument
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/nic_ovn.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index f743083165..5e86e662ba 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -28,7 +28,7 @@ import (
type ovnNet interface {
network.Network
- InstanceDevicePortValidateExternalRoutes(externalRoutes []*net.IPNet) error
+ InstanceDevicePortValidateExternalRoutes(deviceInstance instance.Instance, deviceName string, externalRoutes []*net.IPNet) error
InstanceDevicePortAdd(instanceID int, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error)
InstanceDevicePortDelete(instanceID int, deviceName string, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error
InstanceDevicePortDynamicIPs(instanceID int, deviceName string) ([]net.IP, error)
From c47c3ecc793876764e21e7531c2c5ff4b31ce99a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 28 Oct 2020 17:40:07 +0000
Subject: [PATCH 6/6] lxd/device/nic/ovn:
d.network.InstanceDevicePortValidateExternalRoutes usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/nic_ovn.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index 5e86e662ba..5eb94e5125 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -179,7 +179,7 @@ func (d *nicOVN) validateConfig(instConf instance.ConfigReader) error {
}
}
- err = d.network.InstanceDevicePortValidateExternalRoutes(externalRoutes)
+ err = d.network.InstanceDevicePortValidateExternalRoutes(d.inst, d.name, externalRoutes)
if err != nil {
return err
}
More information about the lxc-devel
mailing list