[lxc-devel] [lxd/master] Network: Adds external subnet overlap validation for OVN networks

tomponline on Github lxc-bot at linuxcontainers.org
Thu Oct 29 11:07:34 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201029/d52adf78/attachment.bin>
-------------- next part --------------
From f899806b77a562de10764e8f52afb0ac1d6d52fd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 29 Oct 2020 11:03:01 +0000
Subject: [PATCH 1/5] lxd/network/driver/ovn: Adds ovnProjectNetworksWithUplink
 function

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 9ce25782e7..0f141d73e2 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -2353,3 +2353,27 @@ func (n *ovn) ovnNICExternalRoutes(ourDeviceInstance instance.Instance, ourDevic
 
 	return externalRoutes, nil
 }
+
+// ovnProjectNetworksWithUplink accepts a map of all networks in all projects and returns a filtered map of OVN
+// networks that use the uplink specified.
+func (n *ovn) ovnProjectNetworksWithUplink(uplink string, projectNetworks map[string]map[int64]api.Network) map[string][]*api.Network {
+	ovnProjectNetworksWithOurUplink := make(map[string][]*api.Network)
+	for netProject, networks := range projectNetworks {
+		for _, ni := range networks {
+			network := ni // Local var creating pointer to rather than iterator.
+
+			// Skip non-OVN networks or those networks that don't use the uplink specified.
+			if network.Type != "ovn" || network.Config["network"] != uplink {
+				continue
+			}
+
+			if ovnProjectNetworksWithOurUplink[netProject] == nil {
+				ovnProjectNetworksWithOurUplink[netProject] = []*api.Network{&network}
+			} else {
+				ovnProjectNetworksWithOurUplink[netProject] = append(ovnProjectNetworksWithOurUplink[netProject], &network)
+			}
+		}
+	}
+
+	return ovnProjectNetworksWithOurUplink
+}

From c6d6045b8f43cf749666eefe99e6c4fa0d36d2e9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 29 Oct 2020 11:04:42 +0000
Subject: [PATCH 2/5] lxd/network/driver/ovn: Updates ovnNetworkExternalSubnets
 to allow optional filtering of our own network's subnets

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 0f141d73e2..cbfaa1f080 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -2260,13 +2260,18 @@ 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(ovnProjectNetworksWithOurUplink map[string][]*api.Network, uplinkRoutes []*net.IPNet) ([]*net.IPNet, error) {
+// ovnNetworkExternalSubnets returns a list of external subnets used by OVN networks (optionally exluding our own
+// if both ourProject and ourNetwork are non-empty) 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(ourProject string, ourNetwork string, ovnProjectNetworksWithOurUplink map[string][]*api.Network, uplinkRoutes []*net.IPNet) ([]*net.IPNet, error) {
 	externalSubnets := make([]*net.IPNet, 0)
-	for _, networks := range ovnProjectNetworksWithOurUplink {
+	for netProject, networks := range ovnProjectNetworksWithOurUplink {
 		for _, netInfo := range networks {
+			if netProject == ourProject && netInfo.Name == ourNetwork {
+				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)])

From 7872cdf100bfb56814b20357c8d869da194d6db3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 29 Oct 2020 11:05:15 +0000
Subject: [PATCH 3/5] lxd/network/driver/ovn: Updates ovnNICExternalRoutes to
 optionally filter our own NIC's external routes

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index cbfaa1f080..c44d32547a 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -2295,8 +2295,9 @@ func (n *ovn) ovnNetworkExternalSubnets(ourProject string, ourNetwork string, ov
 	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.
+// ovnNICExternalRoutes returns a list of external routes currently used by OVN NICs (optionally excluding our
+// own if both ourDeviceInstance and ourDeviceName are non-empty) 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)
 
@@ -2327,8 +2328,8 @@ func (n *ovn) ovnNICExternalRoutes(ourDeviceInstance instance.Instance, ourDevic
 				continue
 			}
 
-			// Skip our own device.
-			if inst.Name == ourDeviceInstance.Name() && inst.Project == ourDeviceInstance.Project() && ourDeviceName == devName {
+			// Skip our own device (if instance and device name were supplied).
+			if ourDeviceInstance != nil && ourDeviceName != "" && inst.Name == ourDeviceInstance.Name() && inst.Project == ourDeviceInstance.Project() && ourDeviceName == devName {
 				continue
 			}
 

From f1f9e0a46909c4c88c278d0e76fee077d3eb4df2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 29 Oct 2020 11:05:57 +0000
Subject: [PATCH 4/5] lxd/network/driver/ovn: Updates
 InstanceDevicePortValidateExternalRoutes to use new functions and signatures

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 33 +++++++--------------------------
 1 file changed, 7 insertions(+), 26 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index c44d32547a..59a885595e 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -1901,26 +1901,11 @@ func (n *ovn) InstanceDevicePortValidateExternalRoutes(deviceInstance instance.I
 		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 OVN networks that use the same uplink as us.
+	ovnProjectNetworksWithOurUplink := n.ovnProjectNetworksWithUplink(n.config["network"], projectNetworks)
 
 	// Get external subnets used by other OVN networks using our uplink.
-	ovnNetworkExternalSubnets, err := n.ovnNetworkExternalSubnets(ovnProjectNetworksWithOurUplink, uplinkRoutes)
+	ovnNetworkExternalSubnets, err := n.ovnNetworkExternalSubnets("", "", ovnProjectNetworksWithOurUplink, uplinkRoutes)
 	if err != nil {
 		return err
 	}
@@ -1931,14 +1916,10 @@ func (n *ovn) InstanceDevicePortValidateExternalRoutes(deviceInstance instance.I
 		return err
 	}
 
-	// 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
-		}
+	// Get external routes configured on OVN NICs (excluding ours) using networks that use our uplink.
+	ovnNICExternalRoutes, err := n.ovnNICExternalRoutes(deviceInstance, deviceName, ovnProjectNetworksWithOurUplink)
+	if err != nil {
+		return err
 	}
 
 	for _, portExternalRoute := range portExternalRoutes {

From 5a53d16c5db5ef98c3fe13682ca50ee0c9caf810 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 29 Oct 2020 11:03:34 +0000
Subject: [PATCH 5/5] lxd/network/driver/ovn: Updates Validate to check
 external subnets dont overlap with other OVN networks or NICs sharing our
 uplink

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 58 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 59a885595e..de2f32024f 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -230,7 +230,8 @@ func (n *ovn) Validate(config map[string]string) error {
 		return err
 	}
 
-	// If NAT disabled, check subnets are within the uplink network's routes and project's subnet restrictions.
+	// If NAT disabled, parse the external subnets that are being requested.
+	var externalSubnets []*net.IPNet
 	for _, keyPrefix := range []string{"ipv4", "ipv6"} {
 		if !shared.IsTrue(config[fmt.Sprintf("%s.nat", keyPrefix)]) && validate.IsOneOf(config[fmt.Sprintf("%s.address", keyPrefix)], []string{"", "none", "auto"}) != nil {
 			_, ipNet, err := net.ParseCIDR(config[fmt.Sprintf("%s.address", keyPrefix)])
@@ -238,10 +239,63 @@ func (n *ovn) Validate(config map[string]string) error {
 				return err
 			}
 
-			err = n.validateExternalSubnet(uplinkRoutes, projectRestrictedSubnets, ipNet)
+			externalSubnets = append(externalSubnets, ipNet)
+		}
+	}
+
+	if len(externalSubnets) > 0 {
+		var projectNetworks map[string]map[int64]api.Network
+		err = n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
+			// 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
+		}
+
+		// Get OVN networks that use the same uplink as us.
+		ovnProjectNetworksWithOurUplink := n.ovnProjectNetworksWithUplink(config["network"], projectNetworks)
+
+		// Get external subnets used by other OVN networks using our uplink.
+		ovnNetworkExternalSubnets, err := n.ovnNetworkExternalSubnets(n.project, n.name, ovnProjectNetworksWithOurUplink, uplinkRoutes)
+		if err != nil {
+			return err
+		}
+
+		// Get external routes configured on OVN NICs using networks that use our uplink.
+		ovnNICExternalRoutes, err := n.ovnNICExternalRoutes(nil, "", ovnProjectNetworksWithOurUplink)
+		if err != nil {
+			return err
+		}
+
+		// Check external subnets are within the uplink network's routes and project's subnet restrictions.
+		for _, externalSubnet := range externalSubnets {
+			err = n.validateExternalSubnet(uplinkRoutes, projectRestrictedSubnets, externalSubnet)
 			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, externalSubnet) || SubnetContains(externalSubnet, ovnNetworkExternalSubnet) {
+					// This error is purposefully vague so that it doesn't reveal any names of
+					// resources potentially outside of the network's project.
+					return fmt.Errorf("External subnet %q overlaps with another OVN network's external subnet", externalSubnet.String())
+				}
+			}
+
+			for _, ovnNICExternalRoute := range ovnNICExternalRoutes {
+				if SubnetContains(ovnNICExternalRoute, externalSubnet) || SubnetContains(externalSubnet, ovnNICExternalRoute) {
+					// This error is purposefully vague so that it doesn't reveal any names of
+					// resources potentially outside of the networks's project.
+					return fmt.Errorf("External subnet %q overlaps with another OVN NIC's external route", externalSubnet.String())
+				}
+			}
 		}
 	}
 


More information about the lxc-devel mailing list