host-local: ensure requested IPs are within the given range
authorDan Williams <dcbw@redhat.com>
Fri, 2 Sep 2016 21:52:29 +0000 (16:52 -0500)
committerDan Williams <dcbw@redhat.com>
Fri, 2 Sep 2016 21:52:29 +0000 (16:52 -0500)
And also make sure that RangeStart and RangeEnd are sane.

plugins/ipam/host-local/allocator.go
plugins/ipam/host-local/allocator_test.go

index e0915ed..ad5f0d2 100644 (file)
@@ -48,25 +48,76 @@ func NewIPAllocator(conf *IPAMConfig, store backend.Store) (*IPAllocator, error)
        start = ip.NextIP(start)
 
        if conf.RangeStart != nil {
-               if err := validateRangeIP(conf.RangeStart, (*net.IPNet)(&conf.Subnet)); err != nil {
+               if err := validateRangeIP(conf.RangeStart, (*net.IPNet)(&conf.Subnet), nil, nil); err != nil {
                        return nil, err
                }
                start = conf.RangeStart
        }
        if conf.RangeEnd != nil {
-               if err := validateRangeIP(conf.RangeEnd, (*net.IPNet)(&conf.Subnet)); err != nil {
+               if err := validateRangeIP(conf.RangeEnd, (*net.IPNet)(&conf.Subnet), start, nil); err != nil {
                        return nil, err
                }
-               // RangeEnd is inclusive
                end = conf.RangeEnd
        }
        return &IPAllocator{start, end, conf, store}, nil
 }
 
-func validateRangeIP(ip net.IP, ipnet *net.IPNet) error {
+func canonicalizeIP(ip net.IP) (net.IP, error) {
+       if ip.To4() != nil {
+               return ip.To4(), nil
+       } else if ip.To16() != nil {
+               return ip.To16(), nil
+       }
+       return nil, fmt.Errorf("IP %s not v4 nor v6", ip)
+}
+
+// Ensures @ip is within @ipnet, and (if given) inclusive of @start and @end
+func validateRangeIP(ip net.IP, ipnet *net.IPNet, start net.IP, end net.IP) error {
+       var err error
+
+       // Make sure we can compare IPv4 addresses directly
+       ip, err = canonicalizeIP(ip)
+       if err != nil {
+               return err
+       }
+
        if !ipnet.Contains(ip) {
                return fmt.Errorf("%s not in network: %s", ip, ipnet)
        }
+
+       if start != nil {
+               start, err = canonicalizeIP(start)
+               if err != nil {
+                       return err
+               }
+               if len(ip) != len(start) {
+                       return fmt.Errorf("%s %d not same size IP address as start %s %d", ip, len(ip), start, len(start))
+               }
+               for i := 0; i < len(ip); i++ {
+                       if ip[i] > start[i] {
+                               break
+                       } else if ip[i] < start[i] {
+                               return fmt.Errorf("%s outside of network %s with start %s", ip, ipnet, start)
+                       }
+               }
+       }
+
+       if end != nil {
+               end, err = canonicalizeIP(end)
+               if err != nil {
+                       return err
+               }
+               if len(ip) != len(end) {
+                       return fmt.Errorf("%s %d not same size IP address as end %s %d", ip, len(ip), end, len(end))
+               }
+               for i := 0; i < len(ip); i++ {
+                       if ip[i] < end[i] {
+                               break
+                       } else if ip[i] > end[i] {
+                               return fmt.Errorf("%s outside of network %s with end %s", ip, ipnet, end)
+                       }
+               }
+       }
        return nil
 }
 
@@ -94,7 +145,7 @@ func (a *IPAllocator) Get(id string) (*types.IPConfig, error) {
                        IP:   a.conf.Subnet.IP,
                        Mask: a.conf.Subnet.Mask,
                }
-               err := validateRangeIP(requestedIP, &subnet)
+               err := validateRangeIP(requestedIP, &subnet, a.start, a.end)
                if err != nil {
                        return nil, err
                }
@@ -148,12 +199,9 @@ func networkRange(ipnet *net.IPNet) (net.IP, net.IP, error) {
        if ipnet.IP == nil {
                return nil, nil, fmt.Errorf("missing field %q in IPAM configuration", "subnet")
        }
-       ip := ipnet.IP.To4()
-       if ip == nil {
-               ip = ipnet.IP.To16()
-               if ip == nil {
-                       return nil, nil, fmt.Errorf("IP not v4 nor v6")
-               }
+       ip, err := canonicalizeIP(ipnet.IP)
+       if err != nil {
+               return nil, nil, fmt.Errorf("IP not v4 nor v6")
        }
 
        if len(ip) != len(ipnet.Mask) {
@@ -188,7 +236,7 @@ func (a *IPAllocator) getSearchRange() (net.IP, net.IP) {
                        IP:   a.conf.Subnet.IP,
                        Mask: a.conf.Subnet.Mask,
                }
-               err := validateRangeIP(lastReservedIP, &subnet)
+               err := validateRangeIP(lastReservedIP, &subnet, a.start, a.end)
                if err == nil {
                        startFromLastReservedIP = true
                }
index bb2b4f6..9b7d19b 100644 (file)
@@ -207,6 +207,86 @@ var _ = Describe("host-local ip allocator", func() {
                                Expect(err).ToNot(HaveOccurred())
                                Expect(res.IP.IP.String()).To(Equal(requestedIP.String()))
                        })
+
+                       It("must return an error when the requested IP is after RangeEnd", func() {
+                               subnet, err := types.ParseCIDR("192.168.1.0/24")
+                               Expect(err).ToNot(HaveOccurred())
+                               ipmap := map[string]string{}
+                               conf := IPAMConfig{
+                                       Name:     "test",
+                                       Type:     "host-local",
+                                       Subnet:   types.IPNet{IP: subnet.IP, Mask: subnet.Mask},
+                                       Args:     &IPAMArgs{IP: net.ParseIP("192.168.1.50")},
+                                       RangeEnd: net.ParseIP("192.168.1.20"),
+                               }
+                               store := fakestore.NewFakeStore(ipmap, nil)
+                               alloc, _ := NewIPAllocator(&conf, store)
+                               _, err = alloc.Get("ID")
+                               Expect(err).To(HaveOccurred())
+                       })
+
+                       It("must return an error when the requested IP is before RangeStart", func() {
+                               subnet, err := types.ParseCIDR("192.168.1.0/24")
+                               Expect(err).ToNot(HaveOccurred())
+                               ipmap := map[string]string{}
+                               conf := IPAMConfig{
+                                       Name:       "test",
+                                       Type:       "host-local",
+                                       Subnet:     types.IPNet{IP: subnet.IP, Mask: subnet.Mask},
+                                       Args:       &IPAMArgs{IP: net.ParseIP("192.168.1.3")},
+                                       RangeStart: net.ParseIP("192.168.1.10"),
+                               }
+                               store := fakestore.NewFakeStore(ipmap, nil)
+                               alloc, _ := NewIPAllocator(&conf, store)
+                               _, err = alloc.Get("ID")
+                               Expect(err).To(HaveOccurred())
+                       })
+               })
+
+               It("RangeStart must be in the given subnet", func() {
+                       subnet, err := types.ParseCIDR("192.168.1.0/24")
+                       Expect(err).ToNot(HaveOccurred())
+
+                       conf := IPAMConfig{
+                               Name:       "test",
+                               Type:       "host-local",
+                               Subnet:     types.IPNet{IP: subnet.IP, Mask: subnet.Mask},
+                               RangeStart: net.ParseIP("10.0.0.1"),
+                       }
+                       store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP(""))
+                       _, err = NewIPAllocator(&conf, store)
+                       Expect(err).To(HaveOccurred())
+               })
+
+               It("RangeEnd must be in the given subnet", func() {
+                       subnet, err := types.ParseCIDR("192.168.1.0/24")
+                       Expect(err).ToNot(HaveOccurred())
+
+                       conf := IPAMConfig{
+                               Name:     "test",
+                               Type:     "host-local",
+                               Subnet:   types.IPNet{IP: subnet.IP, Mask: subnet.Mask},
+                               RangeEnd: net.ParseIP("10.0.0.1"),
+                       }
+                       store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP(""))
+                       _, err = NewIPAllocator(&conf, store)
+                       Expect(err).To(HaveOccurred())
+               })
+
+               It("RangeEnd must be after RangeStart in the given subnet", func() {
+                       subnet, err := types.ParseCIDR("192.168.1.0/24")
+                       Expect(err).ToNot(HaveOccurred())
+
+                       conf := IPAMConfig{
+                               Name:       "test",
+                               Type:       "host-local",
+                               Subnet:     types.IPNet{IP: subnet.IP, Mask: subnet.Mask},
+                               RangeStart: net.ParseIP("192.168.1.10"),
+                               RangeEnd:   net.ParseIP("192.168.1.3"),
+                       }
+                       store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP(""))
+                       _, err = NewIPAllocator(&conf, store)
+                       Expect(err).To(HaveOccurred())
                })
        })