topdown/net: require prefix length for IPv6 in net.cidr_merge #4613

srenatus · 2022-04-22T12:43:21Z

There are no default prefixes in IPv6, so if an IPv6 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 #4596.

ashutosh-narkar · 2022-04-25T23:59:53Z

topdown/cidr.go

+	case net.IPv4len:
+		network.Mask = ip.DefaultMask()
+	case net.IPv6len:
+		network.Mask = net.CIDRMask(128, 128) // "/128"


If only IPv4 addresses have default masks, should we assume this or would it be better to return an error if one is not provided ?

Yeah good point. I'll change this to return a (non-halt) error if the IPv6 input doesn't come with a prefix and can't be mapped to an IPv4 (which would have a default mask).

srenatus · 2022-04-26T11:07:26Z

@ashutosh-narkar updated -- please take another look :)

ashutosh-narkar · 2022-04-26T15:35:19Z

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. For prefix-less IPv6 inputs, the prefix `/128` is assumed. | ``SDK-dependent`` |


Can we update this note as it's no longer valid.

Of course! Thanks for paying attention

ashutosh-narkar

LGTM. Just the doc needs to be updated.

There are no default prefixes in IPv6, so if an IPv6 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]>

…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]>

# This is the 1st commit message: finalizing changes for formatting with sprintf Signed-off-by: Damien Burks <[email protected]> # This is the commit message open-policy-agent#2: updating changes to allow for multiple format strings Signed-off-by: Damien Burks <[email protected]> # This is the commit message open-policy-agent#3: fixing golint issues Signed-off-by: Damien Burks <[email protected]> # This is the commit message open-policy-agent#4: fixing golint issues Signed-off-by: Damien Burks <[email protected]> # This is the commit message open-policy-agent#5: making recommended change: package level variable Signed-off-by: Damien Burks <[email protected]> # This is the commit message open-policy-agent#6: adding support for explicit argument indexes Signed-off-by: Damien Burks <[email protected]> # This is the commit message open-policy-agent#7: format: don't add 'in' keyword import when 'every' is there (open-policy-agent#4607) Also ensure that added imports have a location set. Previously, `opa fmt` on the added test file would have panicked because the import hadn't had a location. Fixes open-policy-agent#4606. Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#8: ast+topdown+planner: allow for mocking built-in functions via "with" (open-policy-agent#4540) With this change, we can replace calls to built-in functions via `with`. The replacement can either be a value -- which will be used as the return value for every call to the mocked built-in -- or a reference to a non-built-in function -- when the results need to depend on the call's arguments. Compiler, topdown, and planner have been adapted in this change. The included docs changes describe the replacement options further. Fixes first part of open-policy-agent#4449. (Missing are non-built-in functions as mock targets.) Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#9: build(deps): bump google.golang.org/grpc from 1.45.0 to 1.46.0 (open-policy-agent#4617) # This is the commit message open-policy-agent#10: docs/policy-testing: use assignment operator in mocks (open-policy-agent#4618) Additionally, simplify one test example. Signed-off-by: Anders Eknert <[email protected]> # This is the commit message open-policy-agent#11: cmd/capabilities: expose capabilities through CLI (open-policy-agent#4588) There is a new command argument "capabilities". With this, it is possible to print the current capabilities version, show all capabilities versions & print any capabilities version, without the need of a file. Moreover, for the other commands which use the --capabilities flag, it is possible to give only the version number, without specifying a file. However, there are no breaking changes for those who use the capabilities file as an input for the flag. Unit tests were also written, in order to test the new argument and the changes made in ast. Fixes: open-policy-agent#4236 Signed-off-by: IoannisMatzaris <[email protected]> # This is the commit message open-policy-agent#12: format,eval: don't use source locations when formatting PE output (open-policy-agent#4611) * format: allow ignoreing source locations * cmd/eval: format disregarding source locations for partial result Before, we'd see this output: ``` $ opa eval -p -fsource 'time.clock(input.x)==time.clock(input.y)' # Query 1 time.clock(time.clock(input.x), input.y) ``` Now, we get the proper answer: `time.clock(input.y, time.clock(input.x))`. Note that it's a _display_ issue; the JSON output of PE has not been affected. Fixes open-policy-agent#4609. Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#13: build(deps): bump github/codeql-action from 1 to 2 (open-policy-agent#4621) Bumps [github/codeql-action](https://github.com/github/codeql-action) from 1 to 2. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@v1...v2) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> # This is the commit message open-policy-agent#14: status: Remove activeRevision label on all but one metric (open-policy-agent#4600) Having one activeRevision label on each of the prometheus metrics emitted by the status plugin has proven to be problematic with a large number of bundles. So with this change, 1. we keep the activeRevision label (just on) the last_success_bundle_activation metric. 2. the gauge gets reset, so we only keep the last active_revision (instead of keeping them all and therefore avoiding the situation where the /metrics output grows indefinitely) Fixes open-policy-agent#4584. Signed-off-by: cmuraru <[email protected]> # This is the commit message open-policy-agent#15: website: add playground button to navbar (open-policy-agent#4622) Addressing one tiny bit of open-policy-agent#4614. Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#16: topdown/net: require prefix length for IPv6 in net.cidr_merge (open-policy-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]> # This is the commit message open-policy-agent#17: Dockerfile: add source annotation (open-policy-agent#4626) `org.opencontainers.image.source` URL to get source code for building the image (string) https://github.com/opencontainers/image-spec/blob/main/annotations.md Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#18: build(deps): bump github.com/fsnotify/fsnotify v1.5.2 -> v1.5.4 (open-policy-agent#4628) https://github.com/fsnotify/fsnotify/releases/tag/v1.5.4 Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#19: docs: update version in kubernetes examples (open-policy-agent#4627) Signed-off-by: yongen.pan <[email protected]> # This is the commit message open-policy-agent#20: bundle/status: Include bundle type in status information OPA has support for Delta Bundles. The status object already contains valuable information such as last activation timestamp but does not specify if the bundle was a canonical snapshot or delta. This change updates the bundle.Status object to include the bundle type string: either "snapshot" or "delta". This can be useful for status endpoints to differentiate between the bundle types. Issue: 4477 Signed-off-by: Bryan Fulton <[email protected]> # This is the commit message open-policy-agent#21: ast+topdown+planner: replacement of non-built-in functions via 'with' (open-policy-agent#4616) Follow-up to open-policy-agent#4540 We can now mock functions that are user-defined: package test f(_) = 1 { input.x = "x" } p = y { y := f(1) with f as 2 } ...following the same scoping rules as laid out for built-in mocks. The replacement can be a value (replacing all calls), or a built-in, or another non-built-in function. Also addresses bugs in the previous slice: * topdown/evalCall: account for empty rules result from indexer * topdown/eval: capture value replacement in PE could panic Note: in PE, we now drop 'with' for function mocks of any kind: These are always fully replaced in the saved support modules, so this should be OK. When keeping them, we'd also have to either copy the existing definitions into the support module; or create a function stub in it. Fixes open-policy-agent#4449. Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#22: format: keep whitespaces for multiple indented same-line withs (open-policy-agent#4635) Fixes open-policy-agent#4634. Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#23: downloader: support for downloading bundles from an OCI registry (open-policy-agent#4558) Initial support for open-policy-agent#4518. Configuration uses the 'services' config for registries, via the "type: oci" field. Bundles configured to pull from that service will then use OCI. ``` services: ghcr-registry: url: https://ghcr.io type: oci bundles: authz: service: ghcr-registry resource: ghcr.io/${ORGANIZATION}/${REPOSITORY}:${TAG} persist: true polling: min_delay_seconds: 60 max_delay_seconds: 120 persistence_directory: ${PERSISTENCE_PATH} ``` Service credentials are supported: if you want to pull from a private registry, use ``` services: ghcr-registry: url: https://ghcr.io type: oci credentials: bearer: token: ${GH_PAT} ``` If no `persistence_directory` is configured, the data is stored in a directory under /tmp. See docs/devel/OCI.md for manual steps to test this feature with some OCI registry (like ghcr.io). Signed-off-by: carabasdaniel <[email protected]> # This is the commit message open-policy-agent#24: Prepare v0.40.0 Release (open-policy-agent#4631) Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#25: Prepare v0.41.0 development (open-policy-agent#4636) Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#26: docs: Adding example for `rego.metadata.role()` usage (open-policy-agent#4640) Signed-off-by: Johan Fylling <[email protected]> # This is the commit message open-policy-agent#27: build(deps): bump oras.land/oras-go from 1.1.0 to 1.1.1 (open-policy-agent#4643) Bumps [oras.land/oras-go](https://github.com/oras-project/oras-go) from 1.1.0 to 1.1.1. - [Release notes](https://github.com/oras-project/oras-go/releases) - [Commits](oras-project/oras-go@v1.1.0...v1.1.1) --- updated-dependencies: - dependency-name: oras.land/oras-go dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> # This is the commit message open-policy-agent#28: build(deps): bump OpenTelemetry 1.6.3 -> 1.7.0 (open-policy-agent#4649) https://github.com/open-telemetry/opentelemetry-go/releases/tag/v1.7.0 https://github.com/open-telemetry/opentelemetry-go-contrib/releases/tag/v1.7.0 Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#29: build(deps): bump github.com/containerd/containerd from 1.6.2 to 1.6.3 (open-policy-agent#4654) Bumps [github.com/containerd/containerd](https://github.com/containerd/containerd) from 1.6.2 to 1.6.3. - [Release notes](https://github.com/containerd/containerd/releases) - [Changelog](https://github.com/containerd/containerd/blob/main/RELEASES.md) - [Commits](containerd/containerd@v1.6.2...v1.6.3) --- updated-dependencies: - dependency-name: github.com/containerd/containerd dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> # This is the commit message open-policy-agent#30: Update k8s examples to the latest schema (open-policy-agent#4655) Signed-off-by: Víctor Martínez Bevià <[email protected]> # This is the commit message open-policy-agent#31: Fix incorrect padding claims (open-policy-agent#4657) Signed-off-by: Anders Eknert <[email protected]> # This is the commit message open-policy-agent#32: build(deps): bump github.com/containerd/containerd from 1.6.3 to 1.6.4 (open-policy-agent#4662) Bumps [github.com/containerd/containerd](https://github.com/containerd/containerd) from 1.6.3 to 1.6.4. - [Release notes](https://github.com/containerd/containerd/releases) - [Changelog](https://github.com/containerd/containerd/blob/main/RELEASES.md) - [Commits](containerd/containerd@v1.6.3...v1.6.4) --- updated-dependencies: - dependency-name: github.com/containerd/containerd dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> # This is the commit message open-policy-agent#33: build(deps): bump docker/setup-qemu-action from 1 to 2 (open-policy-agent#4668) # This is the commit message open-policy-agent#34: build(deps): bump docker/setup-buildx-action from 1 to 2 (open-policy-agent#4669) Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 1 to 2. - [Release notes](https://github.com/docker/setup-buildx-action/releases) - [Commits](docker/setup-buildx-action@v1...v2) --- updated-dependencies: - dependency-name: docker/setup-buildx-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> # This is the commit message open-policy-agent#35: build(deps): github.com/bytecodealliance/wasmtime-go 0.35.0 -> 0.36.0 (open-policy-agent#4652) * build(deps): bump wasmtime-go: 0.35.0 -> 0.36.0 * internal/wasm: adapt to using epoch-based interruption Looks like we don't get frames for this. Also, there is currentlty no better way than comparing the message, as the trap code isn't surfaced (yet). Fixes open-policy-agent#4663. Signed-off-by: Stephan Renatus <[email protected]> # This is the commit message open-policy-agent#36: ecosystem: Add Sansshell (open-policy-agent#4674) Signed-off-by: James Chacon <[email protected]> # This is the commit message open-policy-agent#37: topdown: Add units.parse builtin (open-policy-agent#4676) This function works on all base decimal and binary SI units of the set: m, K/Ki, M/Mi, G/Gi, T/Ti, P/Pi, and E/Ei Note: Unlike `units.parse_bytes`, this function is case sensitive. Fixes open-policy-agent#1802. Signed-off-by: Philip Conrad <[email protected]> # This is the commit message open-policy-agent#38: docs/contrib-code: Add capabilities step to built-in functions tutorial (open-policy-agent#4677) Signed-off-by: Philip Conrad <[email protected]>

srenatus requested a review from ashutosh-narkar April 22, 2022 12:43

srenatus force-pushed the sr/builtins/cidr-ipv6 branch from fb3cddb to 84dfb23 Compare April 22, 2022 12:44

ashutosh-narkar reviewed Apr 25, 2022

View reviewed changes

srenatus marked this pull request as draft April 26, 2022 07:20

srenatus force-pushed the sr/builtins/cidr-ipv6 branch from 09a6689 to eb9fe4f Compare April 26, 2022 11:06

srenatus changed the title ~~topdown/net: default to "/128" prefix for IPv6 in net.cidr_merge~~ topdown/net: require prefix length for IPv6 in net.cidr_merge Apr 26, 2022

srenatus marked this pull request as ready for review April 26, 2022 11:07

srenatus requested a review from ashutosh-narkar April 26, 2022 11:07

ashutosh-narkar reviewed Apr 26, 2022

View reviewed changes

ashutosh-narkar previously approved these changes Apr 26, 2022

View reviewed changes

srenatus dismissed ashutosh-narkar’s stale review via 64bd125 April 26, 2022 16:56

srenatus force-pushed the sr/builtins/cidr-ipv6 branch from eb9fe4f to 64bd125 Compare April 26, 2022 16:56

srenatus force-pushed the sr/builtins/cidr-ipv6 branch from 64bd125 to c541ad0 Compare April 26, 2022 17:02

srenatus merged commit b481f00 into open-policy-agent:main Apr 26, 2022

srenatus deleted the sr/builtins/cidr-ipv6 branch April 26, 2022 17:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

topdown/net: require prefix length for IPv6 in net.cidr_merge #4613

topdown/net: require prefix length for IPv6 in net.cidr_merge #4613

srenatus commented Apr 22, 2022 •

edited

Loading

ashutosh-narkar Apr 25, 2022

srenatus Apr 26, 2022

srenatus commented Apr 26, 2022

ashutosh-narkar Apr 26, 2022

srenatus Apr 26, 2022

ashutosh-narkar left a comment

topdown/net: require prefix length for IPv6 in net.cidr_merge #4613

topdown/net: require prefix length for IPv6 in net.cidr_merge #4613

Conversation

srenatus commented Apr 22, 2022 • edited Loading

ashutosh-narkar Apr 25, 2022

Choose a reason for hiding this comment

srenatus Apr 26, 2022

Choose a reason for hiding this comment

srenatus commented Apr 26, 2022

ashutosh-narkar Apr 26, 2022

Choose a reason for hiding this comment

srenatus Apr 26, 2022

Choose a reason for hiding this comment

ashutosh-narkar left a comment

Choose a reason for hiding this comment

srenatus commented Apr 22, 2022 •

edited

Loading