[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