Merge pull request #2321 from weaveworks/issues/2282-check-subnets

Check validity of IP subnets

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

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
 }
 

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

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