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 <pholzing@redhat.com>

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) {

libpod/network/cni/config_test.go

@@ -508,6 +508,9 @@ var _ = Describe("Config", func() {
 			Expect(network1.Subnets[0].Gateway.String()).To(Equal("10.0.0.1"))
 			Expect(network1.Subnets[0].LeaseRange.StartIP.String()).To(Equal(startIP))
 
+			err = libpodNet.NetworkRemove(network1.Name)
+			Expect(err).To(BeNil())
+
 			endIP := "10.0.0.10"
 			network = types.Network{
 				Driver: "bridge",
@@ -529,6 +532,9 @@ var _ = Describe("Config", func() {
 			Expect(network1.Subnets[0].Gateway.String()).To(Equal("10.0.0.1"))
 			Expect(network1.Subnets[0].LeaseRange.EndIP.String()).To(Equal(endIP))
 
+			err = libpodNet.NetworkRemove(network1.Name)
+			Expect(err).To(BeNil())
+
 			network = types.Network{
 				Driver: "bridge",
 				Subnets: []types.Subnet{
@@ -590,7 +596,7 @@ var _ = Describe("Config", func() {
 			}
 			_, err := libpodNet.NetworkCreate(network)
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("subnet invalid"))
+			Expect(err.Error()).To(ContainSubstring("subnet ip is nil"))
 		})
 
 		It("create network with name", func() {
@@ -886,6 +892,25 @@ var _ = Describe("Config", func() {
 			Expect(err.Error()).To(Equal("default network podman cannot be removed"))
 		})
 
+		It("network create with same subnet", func() {
+			subnet := "10.0.0.0/24"
+			n, _ := types.ParseCIDR(subnet)
+			subnet2 := "10.10.0.0/24"
+			n2, _ := types.ParseCIDR(subnet2)
+			network := types.Network{Subnets: []types.Subnet{{Subnet: n}, {Subnet: n2}}}
+			network1, err := libpodNet.NetworkCreate(network)
+			Expect(err).To(BeNil())
+			Expect(network1.Subnets).To(HaveLen(2))
+			network = types.Network{Subnets: []types.Subnet{{Subnet: n}}}
+			_, err = libpodNet.NetworkCreate(network)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("subnet 10.0.0.0/24 is already used on the host or by another config"))
+			network = types.Network{Subnets: []types.Subnet{{Subnet: n2}}}
+			_, err = libpodNet.NetworkCreate(network)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("subnet 10.10.0.0/24 is already used on the host or by another config"))
+		})
+
 	})
 
 	Context("network load valid existing ones", func() {

libpod/network/cni/network.go

@@ -221,12 +221,7 @@ func getNetworkIDFromName(name string) string {
 }
 
 // getFreeIPv6NetworkSubnet returns a unused ipv4 subnet
-func (n *cniNetwork) getFreeIPv4NetworkSubnet() (*types.Subnet, error) {
+func (n *cniNetwork) getFreeIPv4NetworkSubnet(usedNetworks []*net.IPNet) (*types.Subnet, error) {
-	networks, err := n.getUsedSubnets()
-	if err != nil {
-		return nil, err
-	}
-
 	// the default podman network is 10.88.0.0/16
 	// start locking for free /24 networks
 	network := &net.IPNet{
@@ -236,12 +231,13 @@ func (n *cniNetwork) getFreeIPv4NetworkSubnet() (*types.Subnet, error) {
 
 	// TODO: make sure to not use public subnets
 	for {
-		if intersectsConfig := util.NetworkIntersectsWithNetworks(network, networks); !intersectsConfig {
+		if intersectsConfig := util.NetworkIntersectsWithNetworks(network, usedNetworks); !intersectsConfig {
 			logrus.Debugf("found free ipv4 network subnet %s", network.String())
 			return &types.Subnet{
 				Subnet: types.IPNet{IPNet: *network},
 			}, nil
 		}
+		var err error
 		network, err = util.NextSubnet(network)
 		if err != nil {
 			return nil, err
@@ -250,12 +246,7 @@ func (n *cniNetwork) getFreeIPv4NetworkSubnet() (*types.Subnet, error) {
 }
 
 // getFreeIPv6NetworkSubnet returns a unused ipv6 subnet
-func (n *cniNetwork) getFreeIPv6NetworkSubnet() (*types.Subnet, error) {
+func (n *cniNetwork) getFreeIPv6NetworkSubnet(usedNetworks []*net.IPNet) (*types.Subnet, error) {
-	networks, err := n.getUsedSubnets()
-	if err != nil {
-		return nil, err
-	}
-
 	// FIXME: Is 10000 fine as limit? We should prevent an endless loop.
 	for i := 0; i < 10000; i++ {
 		// RFC4193: Choose the ipv6 subnet random and NOT sequentially.
@@ -263,7 +254,7 @@ func (n *cniNetwork) getFreeIPv6NetworkSubnet() (*types.Subnet, error) {
 		if err != nil {
 			return nil, err
 		}
-		if intersectsConfig := util.NetworkIntersectsWithNetworks(&network, networks); !intersectsConfig {
+		if intersectsConfig := util.NetworkIntersectsWithNetworks(&network, usedNetworks); !intersectsConfig {
 			logrus.Debugf("found free ipv6 network subnet %s", network.String())
 			return &types.Subnet{
 				Subnet: types.IPNet{IPNet: network},
@@ -279,9 +270,8 @@ func (n *cniNetwork) getUsedSubnets() ([]*net.IPNet, error) {
 	// first, load all used subnets from network configs
 	subnets := make([]*net.IPNet, 0, len(n.networks))
 	for _, val := range n.networks {
-		for _, subnet := range val.libpodNet.Subnets {
+		for i := range val.libpodNet.Subnets {
-			// nolint:exportloopref
+			subnets = append(subnets, &val.libpodNet.Subnets[i].Subnet.IPNet)
-			subnets = append(subnets, &subnet.Subnet.IPNet)
 		}
 	}
 	// second, load networks from the current system