From cda10847775cba29e9b6f3c972990ca10fe19850 Mon Sep 17 00:00:00 2001
From: Bryan Boreham <bjboreham@gmail.com>
Date: Thu, 26 May 2016 14:17:38 +0000
Subject: [PATCH] Check validity of IP subnets

Be more consistent about this - check in the same way whether at startup
or during allocation.
---
 ipam/allocator.go                 | 14 ++++++++++++++
 ipam/http.go                      | 12 +++++++-----
 prog/weaver/main.go               | 19 ++++---------------
 test/500_weave_multi_cidr_test.sh |  4 ++++
 4 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/ipam/allocator.go b/ipam/allocator.go
index 46b27184..0ca5ba46 100644
--- a/ipam/allocator.go
+++ b/ipam/allocator.go
@@ -111,6 +111,20 @@ func NewAllocator(config Config) *Allocator {
 	}
 }
 
+func ParseCIDRSubnet(cidrStr string) (cidr address.CIDR, err error) {
+	cidr, err = address.ParseCIDR(cidrStr)
+	if err != nil {
+		return
+	}
+	if !cidr.IsSubnet() {
+		err = fmt.Errorf("invalid subnet - bits after network prefix are not all zero: %s", cidrStr)
+	}
+	if cidr.Size() < MinSubnetSize {
+		err = fmt.Errorf("invalid subnet - smaller than minimum size %d: %s", MinSubnetSize, cidrStr)
+	}
+	return
+}
+
 // Start runs the allocator goroutine
 func (alloc *Allocator) Start() {
 	loadedPersistedData := alloc.loadPersistedData()
diff --git a/ipam/http.go b/ipam/http.go
index 95bbbf84..0d68052a 100644
--- a/ipam/http.go
+++ b/ipam/http.go
@@ -17,15 +17,17 @@ func badRequest(w http.ResponseWriter, err error) {
 }
 
 func parseCIDR(w http.ResponseWriter, cidrStr string, net bool) (address.CIDR, bool) {
-	cidr, err := address.ParseCIDR(cidrStr)
+	var cidr address.CIDR
+	var err error
+	if net {
+		cidr, err = ParseCIDRSubnet(cidrStr)
+	} else {
+		cidr, err = address.ParseCIDR(cidrStr)
+	}
 	if err != nil {
 		badRequest(w, err)
 		return address.CIDR{}, false
 	}
-	if net && !cidr.IsSubnet() {
-		badRequest(w, fmt.Errorf("Invalid subnet %s - bits after network prefix are not all zero", cidrStr))
-		return address.CIDR{}, false
-	}
 	return cidr, true
 }
 
diff --git a/prog/weaver/main.go b/prog/weaver/main.go
index adb3ec54..ce51e1e8 100644
--- a/prog/weaver/main.go
+++ b/prog/weaver/main.go
@@ -413,24 +413,13 @@ func createOverlay(datapathName string, ifaceName string, host string, port int,
 	return overlay, bridge
 }
 
-func parseAndCheckCIDR(cidrStr string) address.CIDR {
-	cidr, err := address.ParseCIDR(cidrStr)
-	checkFatal(err)
-
-	if !cidr.IsSubnet() {
-		Log.Fatalf("Invalid allocation range %s - bits after network prefix are not all zero", cidrStr)
-	}
-	if cidr.Size() < ipam.MinSubnetSize {
-		Log.Fatalf("Allocation range smaller than minimum size %d: %s", ipam.MinSubnetSize, cidrStr)
-	}
-	return cidr
-}
-
 func createAllocator(router *weave.NetworkRouter, config ipamConfig, db db.DB, isKnownPeer func(mesh.PeerName) bool) (*ipam.Allocator, address.CIDR) {
-	ipRange := parseAndCheckCIDR(config.IPRangeCIDR)
+	ipRange, err := ipam.ParseCIDRSubnet(config.IPRangeCIDR)
+	checkFatal(err)
 	defaultSubnet := ipRange
 	if config.IPSubnetCIDR != "" {
-		defaultSubnet = parseAndCheckCIDR(config.IPSubnetCIDR)
+		defaultSubnet, err = ipam.ParseCIDRSubnet(config.IPSubnetCIDR)
+		checkFatal(err)
 		if !ipRange.Range().Overlaps(defaultSubnet.Range()) {
 			Log.Fatalf("IP address allocation default subnet %s does not overlap with allocation range %s", defaultSubnet, ipRange)
 		}
diff --git a/test/500_weave_multi_cidr_test.sh b/test/500_weave_multi_cidr_test.sh
index d6f9ed5d..0d6479cc 100755
--- a/test/500_weave_multi_cidr_test.sh
+++ b/test/500_weave_multi_cidr_test.sh
@@ -122,4 +122,8 @@ assert_equal "$IPS"                                                      10.2.3.
 CID2=$(start_container $HOST1                                        net:10.2.3.0/24)
 assert_container_cidrs $HOST1 $CID2                                      10.2.3.1/24
 
+# Error conditions: host address not network, subnet too small
+assert_raises "start_container $HOST1                                net:10.2.3.2/30" 1
+assert_raises "start_container $HOST1                                net:10.2.3.2/31" 1
+
 end_suite