From aa7bc4e37168e4cdb4469ba8b728d7f5157e46b5 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 26 Aug 2021 12:10:08 +0200 Subject: network create: validate the input subnet Check that the given subnet does not conflict with existing ones (other configs or host interfaces). Signed-off-by: Paul Holzinger --- libpod/network/cni/config.go | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) (limited to 'libpod/network/cni/config.go') diff --git a/libpod/network/cni/config.go b/libpod/network/cni/config.go index ee203f80d..44842c664 100644 --- a/libpod/network/cni/config.go +++ b/libpod/network/cni/config.go @@ -73,13 +73,18 @@ func (n *cniNetwork) networkCreate(net types.Network, writeToDisk bool) (*networ net.Name = name } + usedNetworks, err := n.getUsedSubnets() + if err != nil { + return nil, err + } + switch net.Driver { case types.BridgeNetworkDriver: // if the name was created with getFreeDeviceName set the interface to it as well if name != "" && net.NetworkInterface == "" { net.NetworkInterface = name } - err = n.createBridge(&net) + err = n.createBridge(&net, usedNetworks) if err != nil { return nil, err } @@ -93,7 +98,7 @@ func (n *cniNetwork) networkCreate(net types.Network, writeToDisk bool) (*networ } for i := range net.Subnets { - err := validateSubnet(&net.Subnets[i], !net.Internal) + err := validateSubnet(&net.Subnets[i], !net.Internal, usedNetworks) if err != nil { return nil, err } @@ -218,7 +223,7 @@ func createMacVLAN(network *types.Network) error { return nil } -func (n *cniNetwork) createBridge(network *types.Network) error { +func (n *cniNetwork) createBridge(network *types.Network, usedNetworks []*net.IPNet) error { if network.NetworkInterface != "" { bridges := n.getBridgeInterfaceNames() if pkgutil.StringInSlice(network.NetworkInterface, bridges) { @@ -236,7 +241,7 @@ func (n *cniNetwork) createBridge(network *types.Network) error { } if len(network.Subnets) == 0 { - freeSubnet, err := n.getFreeIPv4NetworkSubnet() + freeSubnet, err := n.getFreeIPv4NetworkSubnet(usedNetworks) if err != nil { return err } @@ -256,14 +261,14 @@ func (n *cniNetwork) createBridge(network *types.Network) error { } } if !ipv4 { - freeSubnet, err := n.getFreeIPv4NetworkSubnet() + freeSubnet, err := n.getFreeIPv4NetworkSubnet(usedNetworks) if err != nil { return err } network.Subnets = append(network.Subnets, *freeSubnet) } if !ipv6 { - freeSubnet, err := n.getFreeIPv6NetworkSubnet() + freeSubnet, err := n.getFreeIPv6NetworkSubnet(usedNetworks) if err != nil { return err } @@ -278,10 +283,14 @@ func (n *cniNetwork) createBridge(network *types.Network) error { // given gateway and lease range are part of this subnet. If the // gateway is empty and addGateway is true it will get the first // available ip in the subnet assigned. -func validateSubnet(s *types.Subnet, addGateway bool) error { +func validateSubnet(s *types.Subnet, addGateway bool, usedNetworks []*net.IPNet) error { if s == nil { return errors.New("subnet is nil") } + if s.Subnet.IP == nil { + return errors.New("subnet ip is nil") + } + // Reparse to ensure subnet is valid. // Do not use types.ParseCIDR() because we want the ip to be // the network address and not a random ip in the subnet. @@ -289,6 +298,12 @@ func validateSubnet(s *types.Subnet, addGateway bool) error { if err != nil { return errors.Wrap(err, "subnet invalid") } + + // check that the new subnet does not conflict with existing ones + if util.NetworkIntersectsWithNetworks(net, usedNetworks) { + return errors.Errorf("subnet %s is already used on the host or by another config", net.String()) + } + s.Subnet = types.IPNet{IPNet: *net} if s.Gateway != nil { if !s.Subnet.Contains(s.Gateway) { -- cgit v1.2.3-54-g00ecf From 24bec9a76bcc2d8e93670047ed0ee8c16d9d7620 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 26 Aug 2021 15:06:38 +0200 Subject: default network: do not validate the used subnets The default network should not be validated against used subnets, we have to ensure that this network can always be created even when a subnet is already used on the host. This could happen if you run a container on this net, then the cni interface will be created on the host and "block" this subnet from being used again. Therefore the next podman command tries to create the default net again and it would fail because it thinks the network is used on the host. Signed-off-by: Paul Holzinger --- libpod/network/cni/config.go | 84 ++++++++++++++++++++++++------------------ libpod/network/cni/network.go | 2 +- libpod/network/cni/run_test.go | 4 ++ 3 files changed, 53 insertions(+), 37 deletions(-) (limited to 'libpod/network/cni/config.go') diff --git a/libpod/network/cni/config.go b/libpod/network/cni/config.go index 44842c664..d31cd3002 100644 --- a/libpod/network/cni/config.go +++ b/libpod/network/cni/config.go @@ -24,7 +24,7 @@ func (n *cniNetwork) NetworkCreate(net types.Network) (types.Network, error) { if err != nil { return types.Network{}, err } - network, err := n.networkCreate(net, true) + network, err := n.networkCreate(net, false) if err != nil { return types.Network{}, err } @@ -33,94 +33,106 @@ func (n *cniNetwork) NetworkCreate(net types.Network) (types.Network, error) { return *network.libpodNet, nil } -func (n *cniNetwork) networkCreate(net types.Network, writeToDisk bool) (*network, error) { +// networkCreate will fill out the given network struct and return the new network entry. +// If defaultNet is true it will not validate against used subnets and it will not write the cni config to disk. +func (n *cniNetwork) networkCreate(newNetwork types.Network, defaultNet bool) (*network, error) { // if no driver is set use the default one - if net.Driver == "" { - net.Driver = types.DefaultNetworkDriver + if newNetwork.Driver == "" { + newNetwork.Driver = types.DefaultNetworkDriver } // FIXME: Should we use a different type for network create without the ID field? // the caller is not allowed to set a specific ID - if net.ID != "" { + if newNetwork.ID != "" { return nil, errors.Wrap(define.ErrInvalidArg, "ID can not be set for network create") } - if net.Labels == nil { - net.Labels = map[string]string{} + if newNetwork.Labels == nil { + newNetwork.Labels = map[string]string{} } - if net.Options == nil { - net.Options = map[string]string{} + if newNetwork.Options == nil { + newNetwork.Options = map[string]string{} } - if net.IPAMOptions == nil { - net.IPAMOptions = map[string]string{} + if newNetwork.IPAMOptions == nil { + newNetwork.IPAMOptions = map[string]string{} } var name string var err error // validate the name when given - if net.Name != "" { - if !define.NameRegex.MatchString(net.Name) { - return nil, errors.Wrapf(define.RegexError, "network name %s invalid", net.Name) + if newNetwork.Name != "" { + if !define.NameRegex.MatchString(newNetwork.Name) { + return nil, errors.Wrapf(define.RegexError, "network name %s invalid", newNetwork.Name) } - if _, ok := n.networks[net.Name]; ok { - return nil, errors.Wrapf(define.ErrNetworkExists, "network name %s already used", net.Name) + if _, ok := n.networks[newNetwork.Name]; ok { + return nil, errors.Wrapf(define.ErrNetworkExists, "network name %s already used", newNetwork.Name) } } else { name, err = n.getFreeDeviceName() if err != nil { return nil, err } - net.Name = name + newNetwork.Name = name } - usedNetworks, err := n.getUsedSubnets() - if err != nil { - return nil, err + // Only get the used networks for validation if we do not create the default network. + // The default network should not be validated against used subnets, we have to ensure + // that this network can always be created even when a subnet is already used on the host. + // This could happen if you run a container on this net, then the cni interface will be + // created on the host and "block" this subnet from being used again. + // Therefore the next podman command tries to create the default net again and it would + // fail because it thinks the network is used on the host. + var usedNetworks []*net.IPNet + if !defaultNet { + usedNetworks, err = n.getUsedSubnets() + if err != nil { + return nil, err + } } - switch net.Driver { + switch newNetwork.Driver { case types.BridgeNetworkDriver: // if the name was created with getFreeDeviceName set the interface to it as well - if name != "" && net.NetworkInterface == "" { - net.NetworkInterface = name + if name != "" && newNetwork.NetworkInterface == "" { + newNetwork.NetworkInterface = name } - err = n.createBridge(&net, usedNetworks) + err = n.createBridge(&newNetwork, usedNetworks) if err != nil { return nil, err } case types.MacVLANNetworkDriver: - err = createMacVLAN(&net) + err = createMacVLAN(&newNetwork) if err != nil { return nil, err } default: - return nil, errors.Wrapf(define.ErrInvalidArg, "unsupported driver %s", net.Driver) + return nil, errors.Wrapf(define.ErrInvalidArg, "unsupported driver %s", newNetwork.Driver) } - for i := range net.Subnets { - err := validateSubnet(&net.Subnets[i], !net.Internal, usedNetworks) + for i := range newNetwork.Subnets { + err := validateSubnet(&newNetwork.Subnets[i], !newNetwork.Internal, usedNetworks) if err != nil { return nil, err } - if util.IsIPv6(net.Subnets[i].Subnet.IP) { - net.IPv6Enabled = true + if util.IsIPv6(newNetwork.Subnets[i].Subnet.IP) { + newNetwork.IPv6Enabled = true } } // generate the network ID - net.ID = getNetworkIDFromName(net.Name) + newNetwork.ID = getNetworkIDFromName(newNetwork.Name) // FIXME: Should this be a hard error? - if net.DNSEnabled && net.Internal && hasDNSNamePlugin(n.cniPluginDirs) { - logrus.Warnf("dnsname and internal networks are incompatible. dnsname plugin not configured for network %s", net.Name) - net.DNSEnabled = false + if newNetwork.DNSEnabled && newNetwork.Internal && hasDNSNamePlugin(n.cniPluginDirs) { + logrus.Warnf("dnsname and internal networks are incompatible. dnsname plugin not configured for network %s", newNetwork.Name) + newNetwork.DNSEnabled = false } - cniConf, path, err := n.createCNIConfigListFromNetwork(&net, writeToDisk) + cniConf, path, err := n.createCNIConfigListFromNetwork(&newNetwork, !defaultNet) if err != nil { return nil, err } - return &network{cniNet: cniConf, libpodNet: &net, filename: path}, nil + return &network{cniNet: cniConf, libpodNet: &newNetwork, filename: path}, nil } // NetworkRemove will remove the Network with the given name or ID. diff --git a/libpod/network/cni/network.go b/libpod/network/cni/network.go index 479f9e0eb..4e4ea7ee5 100644 --- a/libpod/network/cni/network.go +++ b/libpod/network/cni/network.go @@ -179,7 +179,7 @@ func (n *cniNetwork) createDefaultNetwork() (*network, error) { {Subnet: n.defaultSubnet}, }, } - return n.networkCreate(net, false) + return n.networkCreate(net, true) } // getNetwork will lookup a network by name or ID. It returns an diff --git a/libpod/network/cni/run_test.go b/libpod/network/cni/run_test.go index 32e88ca61..f6da22a76 100644 --- a/libpod/network/cni/run_test.go +++ b/libpod/network/cni/run_test.go @@ -140,6 +140,10 @@ var _ = Describe("run CNI", func() { Expect(res[defNet].DNSServerIPs).To(BeEmpty()) Expect(res[defNet].DNSSearchDomains).To(BeEmpty()) + // reload the interface so the networks are reload from disk + libpodNet, err := getNetworkInterface(cniConfDir, false) + Expect(err).To(BeNil()) + err = libpodNet.Teardown(netNSContainer.Path(), types.TeardownOptions(setupOpts)) Expect(err).To(BeNil()) }) -- cgit v1.2.3-54-g00ecf