From 1055f2c4d76e22b31af8ce9eb761af885bd6d702 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Tue, 26 Apr 2022 19:03:24 +0200 Subject: [PATCH] topdown/net: require prefix length for IPv6 in net.cidr_merge (#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 `[ "" ]`, with two, we'd panic. Fixes #4596. Signed-off-by: Stephan Renatus Signed-off-by: yongen.pan --- ast/parser.go | 6 +- ast/policy.go | 8 --- docs/content/policy-reference.md | 2 +- .../test-ipv6-with-and-without-prefix.yaml | 59 +++++++++++++++++++ topdown/cidr.go | 36 +++++------ 5 files changed, 79 insertions(+), 32 deletions(-) create mode 100644 test/cases/testdata/netcidrmerge/test-ipv6-with-and-without-prefix.yaml diff --git a/ast/parser.go b/ast/parser.go index 420978fbcd..eb888c19f1 100644 --- 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 index 26e747d8b4..b9f4adb1cd 100644 --- 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 index c3398539fd..a56d3d8dbe 100644 --- 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`: | ``output := net.cidr_contains_matches(cidrs, cidrs_or_ips)`` | `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`` | | ``net.cidr_intersects(cidr1, cidr2)`` | `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.| ✅ | | ``net.cidr_expand(cidr)`` | `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`` | -| ``net.cidr_merge(cidrs_or_ips)`` | `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`` | +| ``net.cidr_merge(cidrs_or_ips)`` | `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 new file mode 100644 index 0000000000..8642fc6e0a --- /dev/null +++ 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" \ No newline at end of file diff --git a/topdown/cidr.go b/topdown/cidr.go index 2123698c7c..e2dbfed479 100644 --- 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 {