From 959af1e6abe6ea98e6ca591abc2bcfef26dd476b Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 2 Sep 2016 16:52:29 -0500 Subject: [PATCH] host-local: ensure requested IPs are within the given range And also make sure that RangeStart and RangeEnd are sane. --- plugins/ipam/host-local/allocator.go | 72 ++++++++++++++++---- plugins/ipam/host-local/allocator_test.go | 80 +++++++++++++++++++++++ 2 files changed, 140 insertions(+), 12 deletions(-) diff --git a/plugins/ipam/host-local/allocator.go b/plugins/ipam/host-local/allocator.go index e0915ed8..ad5f0d20 100644 --- a/plugins/ipam/host-local/allocator.go +++ b/plugins/ipam/host-local/allocator.go @@ -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 } diff --git a/plugins/ipam/host-local/allocator_test.go b/plugins/ipam/host-local/allocator_test.go index bb2b4f6f..9b7d19bf 100644 --- a/plugins/ipam/host-local/allocator_test.go +++ b/plugins/ipam/host-local/allocator_test.go @@ -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()) }) })