Merge pull request #277 from MggMuggins/network-sanity-checks

Network sanity checks

microcloud/cmd/microcloud/main_init.go

@@ -13,6 +13,7 @@ import (
 	"github.com/canonical/lxd/shared"
 	lxdAPI "github.com/canonical/lxd/shared/api"
 	"github.com/canonical/lxd/shared/logger"
+	"github.com/canonical/lxd/shared/validate"
 	cephTypes "github.com/canonical/microceph/microceph/api/types"
 	cephClient "github.com/canonical/microceph/microceph/client"
 	"github.com/canonical/microcluster/client"
@@ -141,6 +142,11 @@ func (c *cmdInit) RunInteractive(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	err = validateSystems(s, systems)
+	if err != nil {
+		return err
+	}
+
 	err = setupCluster(s, systems)
 	if err != nil {
 		return err
@@ -414,6 +420,112 @@ func AddPeers(sh *service.Handler, systems map[string]InitSystem) error {
 	return nil
 }
 
+// validateGatewayNet ensures that the ipv{4,6} gateway in a network's `config`
+// is a valid CIDR address and that any ovn.ranges if present fall within the
+// gateway. `ipPrefix` is one of "ipv4" or "ipv6".
+func validateGatewayNet(config map[string]string, ipPrefix string, cidrValidator func(string) error) (ovnIPRanges []*shared.IPRange, err error) {
+	gateway, hasGateway := config[ipPrefix+".gateway"]
+	ovnRanges, hasOVNRanges := config[ipPrefix+".ovn.ranges"]
+
+	var gatewayIP net.IP
+	var gatewayNet *net.IPNet
+
+	if hasGateway {
+		err = cidrValidator(gateway)
+		if err != nil {
+			return nil, fmt.Errorf("Invalid %s.gateway %q: %w", ipPrefix, gateway, err)
+		}
+
+		gatewayIP, gatewayNet, err = net.ParseCIDR(gateway)
+		if err != nil {
+			return nil, fmt.Errorf("Invalid %s.gateway %q: %w", ipPrefix, gateway, err)
+		}
+	}
+
+	if hasGateway && hasOVNRanges {
+		ovnIPRanges, err = shared.ParseIPRanges(ovnRanges, gatewayNet)
+		if err != nil {
+			return nil, fmt.Errorf("Invalid %s.ovn.ranges %q: %w", ipPrefix, ovnRanges, err)
+		}
+	}
+
+	for _, ipRange := range ovnIPRanges {
+		if ipRange.ContainsIP(gatewayIP) {
+			return nil, fmt.Errorf("UPLINK %s.ovn.ranges must not include gateway address %q", ipPrefix, gatewayNet.IP)
+		}
+	}
+
+	return ovnIPRanges, nil
+}
+
+func validateSystems(s *service.Handler, systems map[string]InitSystem) (err error) {
+	curSystem, bootstrap := systems[s.Name]
+	if !bootstrap {
+		return nil
+	}
+
+	// Assume that the UPLINK network on each system is the same, so grab just
+	// the gateways from the current node's UPLINK to verify against the other
+	// systems' management addrs
+	var ip4OVNRanges, ip6OVNRanges []*shared.IPRange
+
+	for _, network := range curSystem.Networks {
+		if network.Type == "physical" && network.Name == "UPLINK" {
+			ip4OVNRanges, err = validateGatewayNet(network.Config, "ipv4", validate.IsNetworkAddressCIDRV4)
+			if err != nil {
+				return err
+			}
+
+			ip6OVNRanges, err = validateGatewayNet(network.Config, "ipv6", validate.IsNetworkAddressCIDRV6)
+			if err != nil {
+				return err
+			}
+
+			nameservers, hasNameservers := network.Config["dns.nameservers"]
+			if hasNameservers {
+				isIP := func(s string) error {
+					ip := net.ParseIP(s)
+					if ip == nil {
+						return fmt.Errorf("Invalid IP %q", s)
+					} else {
+						return nil
+					}
+				}
+
+				err = validate.IsListOf(isIP)(nameservers)
+				if err != nil {
+					return fmt.Errorf("Invalid dns.nameservers: %w", err)
+				}
+			}
+
+			break
+		}
+	}
+
+	// Ensure that no system's management address falls within the OVN ranges
+	// to prevent OVN from allocating an IP that's already in use.
+	for systemName, system := range systems {
+		systemAddr := net.ParseIP(system.ServerInfo.Address)
+		if systemAddr == nil {
+			return fmt.Errorf("Invalid address %q for system %q", system.ServerInfo.Address, systemName)
+		}
+
+		for _, ipRange := range ip4OVNRanges {
+			if ipRange.ContainsIP(systemAddr) {
+				return fmt.Errorf("UPLINK ipv4.ovn.ranges must not include system address %q", systemAddr)
+			}
+		}
+
+		for _, ipRange := range ip6OVNRanges {
+			if ipRange.ContainsIP(systemAddr) {
+				return fmt.Errorf("UPLINK ipv6.ovn.ranges must not include system address %q", systemAddr)
+			}
+		}
+	}
+
+	return nil
+}
+
 // setupCluster Bootstraps the cluster if necessary, adds all peers to the cluster, and completes any post cluster
 // configuration.
 func setupCluster(s *service.Handler, systems map[string]InitSystem) error {

microcloud/cmd/microcloud/main_init_preseed.go

@@ -159,6 +159,11 @@ func (c *CmdControl) RunPreseed(cmd *cobra.Command, init bool) error {
 		}
 	}
 
+	err = validateSystems(s, systems)
+	if err != nil {
+		return err
+	}
+
 	return setupCluster(s, systems)
 }
 

microcloud/cmd/microcloud/main_init_test.go

@@ -0,0 +1,224 @@
+package main
+
+import (
+	"testing"
+
+	lxdAPI "github.com/canonical/lxd/shared/api"
+
+	"github.com/canonical/microcloud/microcloud/mdns"
+	"github.com/canonical/microcloud/microcloud/service"
+)
+
+func newSystemWithNetworks(address string, networks []lxdAPI.NetworksPost) InitSystem {
+	return InitSystem{
+		ServerInfo: mdns.ServerInfo{
+			Name:    "testSystem",
+			Address: address,
+		},
+		Networks: networks,
+	}
+}
+
+func newSystemWithUplinkNetConfig(address string, config map[string]string) InitSystem {
+	return newSystemWithNetworks(address, []lxdAPI.NetworksPost{{
+		Name: "UPLINK",
+		Type: "physical",
+		NetworkPut: lxdAPI.NetworkPut{
+			Config: config,
+		},
+	}})
+}
+
+func newTestHandler(addr string, t *testing.T) *service.Handler {
+	handler, err := service.NewHandler("testSystem", addr, "/tmp/microcloud_test_hander", true, true)
+	if err != nil {
+		t.Fatalf("Failed to create test service handler: %s", err)
+	}
+
+	return handler
+}
+
+func newTestSystemsMap(systems ...InitSystem) map[string]InitSystem {
+	systemsMap := map[string]InitSystem{}
+
+	for _, system := range systems {
+		systemsMap[system.ServerInfo.Name] = system
+	}
+
+	return systemsMap
+}
+
+func ensureValidateSystemsPasses(handler *service.Handler, testSystems map[string]InitSystem, t *testing.T) {
+	for testName, system := range testSystems {
+		systems := newTestSystemsMap(system)
+
+		err := validateSystems(handler, systems)
+		if err != nil {
+			t.Fatalf("Valid system %q failed validate: %s", testName, err)
+		}
+	}
+}
+
+func ensureValidateSystemsFails(handler *service.Handler, testSystems map[string]InitSystem, t *testing.T) {
+	for testName, system := range testSystems {
+		systems := newTestSystemsMap(system)
+
+		err := validateSystems(handler, systems)
+		if err == nil {
+			t.Fatalf("Invalid system %q passed validation", testName)
+		}
+	}
+}
+
+func TestValidateSystemsIP4(t *testing.T) {
+	address := "192.168.1.27"
+	handler := newTestHandler(address, t)
+
+	// Each entry in these maps is validated individually
+	validSystems := map[string]InitSystem{
+		"plainGateway": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway": "10.234.0.1/16",
+		}),
+		"dns": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway":    "10.234.0.1/16",
+			"dns.nameservers": "1.1.1.1,8.8.8.8",
+		}),
+		"16Net": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway":    "10.42.0.1/16",
+			"ipv4.ovn.ranges": "10.42.1.1-10.42.5.255",
+		}),
+		"24Net": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway":    "190.168.4.1/24",
+			"ipv4.ovn.ranges": "190.168.4.50-190.168.4.60",
+		}),
+	}
+
+	ensureValidateSystemsPasses(handler, validSystems, t)
+
+	invalidSystems := map[string]InitSystem{
+		"invalidNameservers1": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway":    "10.234.0.1/16",
+			"dns.nameservers": "8.8",
+		}),
+		"invalidNameservers2": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway":    "10.234.0.1/16",
+			"dns.nameservers": "8.8.8.128/23",
+		}),
+		"gatewayIsSubnetAddr": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway": "192.168.28.0/24",
+		}),
+		"backwardsRange": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway":    "10.42.0.1/16",
+			"ipv4.ovn.ranges": "10.42.5.255-10.42.1.1",
+		}),
+		"rangesOutsideGateway": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway":    "10.1.1.0/24",
+			"ipv4.ovn.ranges": "10.2.2.50-10.2.2.100",
+		}),
+		"rangesContainGateway": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway":    "192.168.1.1/24",
+			"ipv4.ovn.ranges": "192.168.1.1-192.168.1.20",
+		}),
+		"rangesContainSystem": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv4.gateway":    "192.168.1.1/16",
+			"ipv4.ovn.ranges": "192.168.1.20-192.168.1.30",
+		}),
+	}
+
+	ensureValidateSystemsFails(handler, invalidSystems, t)
+}
+
+func TestValidateSystemsIP6(t *testing.T) {
+	address := "fc00:feed:beef::bed1"
+	handler := newTestHandler(address, t)
+
+	validSystems := map[string]InitSystem{
+		"plainGateway": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway": "fc00:bad:feed::1/64",
+		}),
+		"dns": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway":    "fc00:feed:f00d::1/64",
+			"dns.nameservers": "2001:4860:4860::8888,2001:4860:4860::8844",
+		}),
+		"dnsMulti": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway":    "fc00:feed:f00d::1/64",
+			"dns.nameservers": "2001:4860:4860::8888,1.1.1.1",
+		}),
+		"64Net": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway":    "fc00:bad:feed::1/64",
+			"ipv6.ovn.ranges": "fc00:bad:feed::f-fc00:bad:feed::fffe",
+		}),
+	}
+
+	ensureValidateSystemsPasses(handler, validSystems, t)
+
+	invalidSystems := map[string]InitSystem{
+		"invalidNameservers1": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway":    "fc00:feed:f00d::1/64",
+			"dns.nameservers": "8:8",
+		}),
+		"invalidNameservers2": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway":    "fc00:feed:f00d::1/64",
+			"dns.nameservers": "2001:4860:4860::8888/32",
+		}),
+		"gatewayIsSubnetAddr": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway": "fc00:feed:f00d::0/64",
+		}),
+		"rangesOutsideGateway": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway":    "fc00:feed:f00d::1/64",
+			"ipv6.ovn.ranges": "fc00:feed:beef::f-fc00:feed:beef::fffe",
+		}),
+		"rangesContainGateway": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway":    "fc00:feed:f00d::1/64",
+			"ipv6.ovn.ranges": "fc00:feed:f00d::1-fc00:feed:f00d::ff",
+		}),
+		"rangesContainSystem": newSystemWithUplinkNetConfig(address, map[string]string{
+			"ipv6.gateway":    "fc00:feed:beef::1/64",
+			"ipv6.ovn.ranges": "fc00:feed:beef::bed1-fc00:feed:beef::bedf",
+		}),
+	}
+
+	ensureValidateSystemsFails(handler, invalidSystems, t)
+}
+
+func TestValidateSystemsMultiSystem(t *testing.T) {
+	localAddr := "10.23.1.20"
+	handler := newTestHandler(localAddr, t)
+
+	uplinkConfig := map[string]string{
+		"ipv4.gateway":    "10.23.1.1/16",
+		"ipv4.ovn.ranges": "10.23.1.50-10.23.1.100",
+	}
+
+	sys1 := newSystemWithUplinkNetConfig(localAddr, uplinkConfig)
+
+	sys2 := newSystemWithUplinkNetConfig("10.23.1.72", uplinkConfig)
+	sys2.ServerInfo.Name = "sys2"
+
+	systems := newTestSystemsMap(sys1, sys2)
+
+	err := validateSystems(handler, systems)
+	if err == nil {
+		t.Fatalf("sys2 with conflicting management IP and ipv4.ovn.ranges passed validation")
+	}
+
+	localAddr = "fc00:bad:feed::f00d"
+	handler = newTestHandler(localAddr, t)
+
+	uplinkConfig = map[string]string{
+		"ipv6.gateway":    "fc00:bad:feed::1/64",
+		"ipv6.ovn.ranges": "fc00:bad:feed::1-fc00:bad:feed::ff",
+	}
+
+	sys3 := newSystemWithUplinkNetConfig(localAddr, uplinkConfig)
+
+	sys4 := newSystemWithUplinkNetConfig("fc00:bad:feed::60", uplinkConfig)
+	sys4.ServerInfo.Name = "sys4"
+
+	systems = newTestSystemsMap(sys3, sys4)
+
+	err = validateSystems(handler, systems)
+	if err == nil {
+		t.Fatalf("sys4 with conflicting management IP and ipv6.ovn.ranges passed validation")
+	}
+}