From b7bb3ea86a494de3c76352a65db510b72c3fc281 Mon Sep 17 00:00:00 2001 From: Wesley Hershberger Date: Wed, 3 Apr 2024 17:44:54 -0500 Subject: [PATCH] microcloud/cmd: Check uplink network config before setting up the cluster Fixes #210 1. Invalid OVN ranges in UPLINK networks (range outside gateway subnet, etc) 2. UPLINK OVN ranges that contain any system's management address; 3. Invalid dns nameserver IPs Signed-off-by: Wesley Hershberger --- microcloud/cmd/microcloud/main_init.go | 112 +++++++++ .../cmd/microcloud/main_init_preseed.go | 5 + microcloud/cmd/microcloud/main_init_test.go | 224 ++++++++++++++++++ 3 files changed, 341 insertions(+) create mode 100644 microcloud/cmd/microcloud/main_init_test.go diff --git a/microcloud/cmd/microcloud/main_init.go b/microcloud/cmd/microcloud/main_init.go index fe1c2339..9b213915 100644 --- a/microcloud/cmd/microcloud/main_init.go +++ b/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 { diff --git a/microcloud/cmd/microcloud/main_init_preseed.go b/microcloud/cmd/microcloud/main_init_preseed.go index 8397f2b6..ab90a40e 100644 --- a/microcloud/cmd/microcloud/main_init_preseed.go +++ b/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) } diff --git a/microcloud/cmd/microcloud/main_init_test.go b/microcloud/cmd/microcloud/main_init_test.go new file mode 100644 index 00000000..d5489279 --- /dev/null +++ b/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") + } +}