topdown/net: require prefix length for IPv6 in net.cidr_merge (open-p…

…olicy-agent#4613) There are no default prefixes in IPv6, so if an IPv6 without a prefix is fed into net.cidr_merge, we'll return a non-halt error now. Before, we'd fail in various ways if a prefix-less IPv6 was fed into `net.cidr_merge`. With only one, we'd return `[ "<nil>" ]`, with two, we'd panic. Fixes open-policy-agent#4596. Signed-off-by: Stephan Renatus <[email protected]> Signed-off-by: yongen.pan <[email protected]>
rokkiter · Apr 26, 2022 · 1055f2c · 1055f2c
1 parent 619867c
commit 1055f2c
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 32 deletions.
diff --git a/ast/parser.go b/ast/parser.go
@@ -819,11 +819,7 @@ func (p *Parser) parseLiteral() (expr *Expr) {
 			return nil
 		}
 		return p.parseEvery()
-:wq	case tokens.In:
-		if negated {
-			p.illegal("illegal negation of 'in'")
-			return nil
-		}
+
 	default:
 		s := p.save()
 		expr := p.parseExpr()

diff --git a/ast/policy.go b/ast/policy.go
@@ -225,14 +225,6 @@ type (
 		Body     Body      `json:"body"`
 	}
 
-	In struct {
-		Location *Location `json:"-"`
-		Key      *Term     `json:"key"`
-		Value    *Term     `json:"value"`
-		Domain   *Term     `json:"domain"`
-		Body     Body      `json:"body"`
-	}
-
 	// With represents a modifier on an expression.
 	With struct {
 		Location *Location `json:"-"`

diff --git a/docs/content/policy-reference.md b/docs/content/policy-reference.md
@@ -1015,7 +1015,7 @@ The table below shows examples of calling `http.send`:
 | <span class="opa-keep-it-together">``output := net.cidr_contains_matches(cidrs, cidrs_or_ips)``</span> | `output` is a `set` of tuples identifying matches where `cidrs_or_ips` are contained within `cidrs`. This function is similar to `net.cidr_contains` except it allows callers to pass collections of CIDRs or IPs as arguments and returns the matches (as opposed to a boolean result indicating a match between two CIDRs/IPs.) See below for examples. | ``SDK-dependent`` |
 | <span class="opa-keep-it-together">``net.cidr_intersects(cidr1, cidr2)``</span> | `output` is `true` if `cidr1` (e.g. `192.168.0.0/16`) overlaps with `cidr2` (e.g. `192.168.1.0/24`) and false otherwise. Supports both IPv4 and IPv6 notations.| ✅ |
 | <span class="opa-keep-it-together">``net.cidr_expand(cidr)``</span> | `output` is the set of hosts in `cidr`  (e.g., `net.cidr_expand("192.168.0.0/30")` generates 4 hosts: `{"192.168.0.0", "192.168.0.1", "192.168.0.2", "192.168.0.3"}` | ``SDK-dependent`` |
-| <span class="opa-keep-it-together">``net.cidr_merge(cidrs_or_ips)``</span> | `output` is the smallest possible set of CIDRs obtained after merging the provided list of IP addresses and subnets in `cidrs_or_ips`  (e.g., `net.cidr_merge(["192.0.128.0/24", "192.0.129.0/24"])` generates `{"192.0.128.0/23"}`. This function merges adjacent subnets where possible, those contained within others and also removes any duplicates. Supports both IPv4 and IPv6 notations. | ``SDK-dependent`` |
+| <span class="opa-keep-it-together">``net.cidr_merge(cidrs_or_ips)``</span> | `output` is the smallest possible set of CIDRs obtained after merging the provided list of IP addresses and subnets in `cidrs_or_ips`  (e.g., `net.cidr_merge(["192.0.128.0/24", "192.0.129.0/24"])` generates `{"192.0.128.0/23"}`. This function merges adjacent subnets where possible, those contained within others and also removes any duplicates. Supports both IPv4 and IPv6 notations. IPv6 inputs need a prefix length (e.g. "/128"). | ``SDK-dependent`` |
 
 #### Notes on Name Resolution (`net.lookup_ip_addr`)
 

diff --git a/test/cases/testdata/netcidrmerge/test-ipv6-with-and-without-prefix.yaml b/test/cases/testdata/netcidrmerge/test-ipv6-with-and-without-prefix.yaml
@@ -0,0 +1,59 @@
+cases:
+- note: netcidrmerge/cidr ipv6 with prefix
+  modules:
+  - |
+    package test
+
+    p = x {
+      net.cidr_merge(["2601:600:8a80:207e:a57d:7567:e2c9:e7b3/128"], x)
+    }
+  query: data.test.p = x
+  want_result:
+  - x:
+    - "2601:600:8a80:207e:a57d:7567:e2c9:e7b3/128"
+- note: netcidrmerge/cidr ipv6 with prefix, same twice
+  modules:
+  - |
+    package test
+
+    p = x {
+      net.cidr_merge(["2601:600:8a80:207e:a57d:7567:e2c9:e7b3/128", "2601:600:8a80:207e:a57d:7567:e2c9:e7b3/128"], x)
+    }
+  query: data.test.p = x
+  want_result:
+  - x:
+    - "2601:600:8a80:207e:a57d:7567:e2c9:e7b3/128"
+- note: netcidrmerge/cidr ipv6 with prefix, two different prefixes
+  modules:
+  - |
+    package test
+
+    p = x {
+      net.cidr_merge(["2601:600:8a80:207e:a57d:7567:e2c9:e7b3/64", "2601:600:8a80:207e:a57d:7567:e2c9:e7b3/128"], x)
+    }
+  query: data.test.p = x
+  want_result:
+  - x:
+    - "2601:600:8a80:207e::/64"
+- note: netcidrmerge/cidr ipv6 without prefix
+  modules:
+  - |
+    package test
+
+    p = x {
+      net.cidr_merge(["2601:600:8a80:207e:a57d:7567:e2c9:e7b3"], x)
+    }
+  query: data.test.p = x
+  strict_error: true
+  want_error: "eval_builtin_error: net.cidr_merge: IPv6 invalid: needs prefix length"
+- note: netcidrmerge/cidr ipv6 without prefix, same twice
+  modules:
+  - |
+    package test
+
+    p = x {
+      net.cidr_merge(["2601:600:8a80:207e:a57d:7567:e2c9:e7b3", "2601:600:8a80:207e:a57d:7567:e2c9:e7b3"], x)
+    }
+  query: data.test.p = x
+  strict_error: true
+  want_error: "eval_builtin_error: net.cidr_merge: IPv6 invalid: needs prefix length"
diff --git a/topdown/cidr.go b/topdown/cidr.go
@@ -73,7 +73,7 @@ func builtinNetCIDRIntersects(a, b ast.Value) (ast.Value, error) {
 	}
 
 	// If either net contains the others starting IP they are overlapping
-	cidrsOverlap := (cidrnetA.Contains(cidrnetB.IP) || cidrnetB.Contains(cidrnetA.IP))
+	cidrsOverlap := cidrnetA.Contains(cidrnetB.IP) || cidrnetB.Contains(cidrnetA.IP)
 
 	return ast.Boolean(cidrsOverlap), nil
 }
@@ -332,25 +332,25 @@ func evalNetCIDRMerge(networks []*net.IPNet) []*net.IPNet {
 }
 
 func generateIPNet(term *ast.Term) (*net.IPNet, error) {
-	switch e := term.Value.(type) {
-	case ast.String:
-		network := &net.IPNet{}
-		// try to parse element as an IP first, fall back to CIDR
-		ip := net.ParseIP(string(e))
-		if ip != nil {
-			network.IP = ip
-			network.Mask = ip.DefaultMask()
-		} else {
-			var err error
-			_, network, err = net.ParseCIDR(string(e))
-			if err != nil {
-				return nil, err
-			}
-		}
-		return network, nil
-	default:
+	e, ok := term.Value.(ast.String)
+	if !ok {
 		return nil, errors.New("element must be string")
 	}
+
+	// try to parse element as an IP first, fall back to CIDR
+	ip := net.ParseIP(string(e))
+	if ip == nil {
+		_, network, err := net.ParseCIDR(string(e))
+		return network, err
+	}
+
+	if ip.To4() != nil {
+		return &net.IPNet{
+			IP:   ip,
+			Mask: ip.DefaultMask(),
+		}, nil
+	}
+	return nil, errors.New("IPv6 invalid: needs prefix length")
 }
 
 func mergeCIDRs(ranges cidrBlockRanges) cidrBlockRanges {