diff --git a/opa-builtins/opa-builtins-net/build.gradle.kts b/opa-builtins/opa-builtins-net/build.gradle.kts index a26b8f67..68c49f96 100644 --- a/opa-builtins/opa-builtins-net/build.gradle.kts +++ b/opa-builtins/opa-builtins-net/build.gradle.kts @@ -10,6 +10,9 @@ dependencies { api(project(":opa-evaluator")) implementation("com.github.seancfoley:ipaddress:5.6.2") + + testImplementation("org.junit.jupiter:junit-jupiter:6.1.2") + testRuntimeOnly("org.junit.platform:junit-platform-launcher:6.1.2") } java { @@ -17,3 +20,7 @@ java { languageVersion = JavaLanguageVersion.of(17) } } + +tasks.test { + useJUnitPlatform() +} diff --git a/opa-builtins/opa-builtins-net/src/main/java/io/github/open_policy_agent/opa/ast/builtin/impls/CidrBuiltins.java b/opa-builtins/opa-builtins-net/src/main/java/io/github/open_policy_agent/opa/ast/builtin/impls/CidrBuiltins.java index a24357b5..7ca4b91f 100644 --- a/opa-builtins/opa-builtins-net/src/main/java/io/github/open_policy_agent/opa/ast/builtin/impls/CidrBuiltins.java +++ b/opa-builtins/opa-builtins-net/src/main/java/io/github/open_policy_agent/opa/ast/builtin/impls/CidrBuiltins.java @@ -3,8 +3,8 @@ import inet.ipaddr.AddressStringException; import inet.ipaddr.IPAddress; import inet.ipaddr.IPAddressString; +import inet.ipaddr.IPAddressStringParameters; import inet.ipaddr.ipv4.IPv4Address; -import inet.ipaddr.ipv6.IPv6Address; import java.net.InetAddress; import java.net.UnknownHostException; import io.github.open_policy_agent.opa.ast.types.RegoArray; @@ -35,6 +35,18 @@ public class CidrBuiltins implements BuiltinProvider { + /** + * By default the parser accepts the empty string and resolves it to the loopback address, so + * net.cidr_contains("", "127.0.0.1") would answer true. Go's net.ParseCIDR/ParseIP reject it. + */ + private static final IPAddressStringParameters ADDRESS_PARAMS = + new IPAddressStringParameters.Builder().allowEmpty(false).toParams(); + + /** Build a parser for `str` that rejects the inputs Go rejects. */ + private static IPAddressString addressString(String str) { + return new IPAddressString(str, ADDRESS_PARAMS); + } + @Override public Map> builtins() { CidrBuiltins instance = new CidrBuiltins(); @@ -52,13 +64,13 @@ public Map> builti /** Parse an IP address or CIDR from a string. */ private IPAddress parseAddress(String str) throws AddressStringException { - return new IPAddressString(str).toAddress(); + return addressString(str).toAddress(); } /** Parse an IP address or CIDR, throwing a BuiltinError with the builtin name on failure. */ private IPAddress parseAddressOrThrow(String str, String builtinName) throws AddressStringException { - IPAddressString addrStr = new IPAddressString(str); + IPAddressString addrStr = addressString(str); if (!addrStr.isValid()) { throw new BuiltinError( builtinName + ": not a valid textual representation of an IP address or CIDR: " + str); @@ -224,21 +236,21 @@ private RegoString getCidrFromElement(RegoValue elem, int operandNum) { throw new BuiltinError( "net.cidr_contains_matches: operand " + operandNum - + " element must be string or non-empty array"); + + ": element must be string or non-empty array"); } RegoValue first = arr.getValue().get(0); if (!(first instanceof RegoString)) { throw new BuiltinError( "net.cidr_contains_matches: operand " + operandNum - + " element must be string or non-empty array"); + + ": element must be string or non-empty array"); } return (RegoString) first; } else { throw new BuiltinError( "net.cidr_contains_matches: operand " + operandNum - + " element must be string or non-empty array"); + + ": element must be string or non-empty array"); } } @@ -256,7 +268,12 @@ private RegoString getCidrFromElement(RegoValue elem, int operandNum) { public RegoValue expand(EvaluationContext ctx, RegoValue[] args) { try { String cidrStr = getArg(args, 0, RegoString.class).getValue(); - IPAddress addr = parseAddress(cidrStr); + IPAddressString addrStr = addressString(cidrStr); + if (!addrStr.isValid()) { + // Match Go's net.ParseCIDR wording rather than the parser's own diagnostic. + throw new BuiltinError("net.cidr_expand: invalid CIDR address: " + cidrStr); + } + IPAddress addr = addrStr.toAddress(); // Normalize to the network block (like Go's ip.Mask(ipNet.Mask)) IPAddress network = addr.toPrefixBlock(); @@ -288,13 +305,18 @@ public RegoValue expand(EvaluationContext ctx, RegoValue[] args) { public RegoValue isValid(EvaluationContext ctx, RegoValue[] args) { try { String cidrStr = getArg(args, 0, RegoString.class).getValue(); - IPAddressString addrStr = new IPAddressString(cidrStr); + IPAddressString addrStr = addressString(cidrStr); if (!addrStr.isValid()) { return RegoBoolean.FALSE; } - // Must be a valid CIDR (has a prefix or is a complete address) + // Go's net.ParseCIDR requires the prefix, so a bare address such as "192.168.1.2" is not + // valid CIDR notation even though it parses as an IP. + if (addrStr.getNetworkPrefixLength() == null) { + return RegoBoolean.FALSE; + } + addrStr.toAddress(); return RegoBoolean.TRUE; @@ -340,17 +362,23 @@ public RegoValue merge(EvaluationContext ctx, RegoValue[] args) { return new RegoSet(ctx.sortSets, new HashSet<>()); } - // Merge the addresses using the IPAddress library's merge functionality - List sortedAddresses = new ArrayList<>(addresses); - sortedAddresses.sort(Comparator.naturalOrder()); - - // Use merging from the first address's type - IPAddress[] merged = - sortedAddresses.get(0).mergeToPrefixBlocks(sortedAddresses.toArray(new IPAddress[0])); - + // mergeToPrefixBlocks cannot mix address versions, so merge each version separately and + // union the results. Go's implementation likewise merges v4 and v6 independently. Set resultSet = new HashSet<>(); - for (IPAddress addr : merged) { - resultSet.add(new RegoString(addr.toCanonicalString())); + for (boolean ipv6 : new boolean[] {false, true}) { + List versionAddresses = + addresses.stream().filter(a -> a.isIPv6() == ipv6).collect(Collectors.toList()); + if (versionAddresses.isEmpty()) { + continue; + } + versionAddresses.sort(Comparator.naturalOrder()); + IPAddress[] merged = + versionAddresses + .get(0) + .mergeToPrefixBlocks(versionAddresses.toArray(new IPAddress[0])); + for (IPAddress addr : merged) { + resultSet.add(new RegoString(addr.toCanonicalString())); + } } return new RegoSet(ctx.sortSets, resultSet); @@ -389,28 +417,36 @@ private List extractStringCollection(RegoValue input, String builtinName } private IPAddress parseIPOrCidr(String str) throws AddressStringException { - IPAddressString addrStr = new IPAddressString(str); - - // Try to parse as CIDR first - if (addrStr.isValid()) { - return addrStr.toAddress(); - } - - // Try as plain IP - addrStr = new IPAddressString(str); + IPAddressString addrStr = addressString(str); IPAddress addr = addrStr.toAddress(); - // If it's an IPv4 address without prefix, add default mask - if (addr instanceof IPv4Address && addr.getNetworkPrefixLength() == null) { - return addr.setPrefixLength(32, false); + if (addr.getNetworkPrefixLength() != null) { + return addr.toPrefixBlock(); } - // IPv6 addresses require a prefix length - if (addr instanceof IPv6Address && addr.getNetworkPrefixLength() == null) { - throw new AddressStringException(str, "IPv6 invalid: needs prefix length"); + // A bare address carries no prefix. Go applies net.IP.DefaultMask(), the classful default, + // so 192.0.2.112 widens to 192.0.2.0/24 rather than a /32. DefaultMask is undefined for + // IPv6, which is why a bare IPv6 address is an error instead. + if (addr.isIPv6()) { + // Thrown directly rather than as an AddressStringException, whose message would be + // decorated with the address and an "IP Address error" prefix. The evaluator prepends the + // builtin name, so it is not repeated here. + throw new BuiltinError("IPv6 invalid: needs prefix length"); } - return addr; + return addr.setPrefixLength(defaultClassfulPrefix(addr.toIPv4()), false).toPrefixBlock(); + } + + /** Go's net.IP.DefaultMask: /8 for class A, /16 for class B, /24 otherwise. */ + private int defaultClassfulPrefix(IPv4Address addr) { + int firstOctet = addr.getSegment(0).getSegmentValue(); + if (firstOctet < 0x80) { + return 8; + } + if (firstOctet < 0xC0) { + return 16; + } + return 24; } @OpaBuiltin( @@ -429,7 +465,7 @@ public RegoValue lookupIpAddr(EvaluationContext ctx, RegoValue[] args) { String hostname = getArg(args, 0, RegoString.class).getValue(); // Check if it's already an IP address - IPAddressString addrStr = new IPAddressString(hostname); + IPAddressString addrStr = addressString(hostname); if (addrStr.isIPAddress()) { // If it's an IP, just return it as-is Set resultSet = new HashSet<>(); @@ -442,7 +478,10 @@ public RegoValue lookupIpAddr(EvaluationContext ctx, RegoValue[] args) { Set resultSet = new HashSet<>(); for (InetAddress addr : addresses) { - resultSet.add(new RegoString(addr.getHostAddress())); + // InetAddress.getHostAddress renders IPv6 uncompressed (0:0:0:0:0:0:0:1); Go returns the + // canonical RFC 5952 form (::1), which is what policies compare against. + resultSet.add( + new RegoString(addressString(addr.getHostAddress()).getAddress().toCanonicalString())); } return new RegoSet(ctx.sortSets, resultSet); diff --git a/opa-builtins/opa-builtins-net/src/main/resources/META-INF/services/io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider b/opa-builtins/opa-builtins-net/src/main/resources/META-INF/services/io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider index 5f2e66e5..68abc25a 100644 --- a/opa-builtins/opa-builtins-net/src/main/resources/META-INF/services/io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider +++ b/opa-builtins/opa-builtins-net/src/main/resources/META-INF/services/io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider @@ -1 +1 @@ -#io.github.open_policy_agent.opa.ast.builtin.impls.CidrBuiltins +io.github.open_policy_agent.opa.ast.builtin.impls.CidrBuiltins diff --git a/opa-builtins/opa-builtins-net/src/test/java/io/github/open_policy_agent/opa/ast/builtin/impls/CidrBuiltinsTest.java b/opa-builtins/opa-builtins-net/src/test/java/io/github/open_policy_agent/opa/ast/builtin/impls/CidrBuiltinsTest.java new file mode 100644 index 00000000..65a1e7bb --- /dev/null +++ b/opa-builtins/opa-builtins-net/src/test/java/io/github/open_policy_agent/opa/ast/builtin/impls/CidrBuiltinsTest.java @@ -0,0 +1,168 @@ +package io.github.open_policy_agent.opa.ast.builtin.impls; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.github.open_policy_agent.opa.ast.builtin.BuiltinError; +import io.github.open_policy_agent.opa.ast.builtin.BuiltinProvider; +import io.github.open_policy_agent.opa.ast.types.RegoArray; +import io.github.open_policy_agent.opa.ast.types.RegoBoolean; +import io.github.open_policy_agent.opa.ast.types.RegoSet; +import io.github.open_policy_agent.opa.ast.types.RegoString; +import io.github.open_policy_agent.opa.ast.types.RegoValue; +import io.github.open_policy_agent.opa.rego.EvaluationContext; +import java.util.Arrays; +import java.util.List; +import java.util.ServiceLoader; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Test; + +class CidrBuiltinsTest { + + private final CidrBuiltins builtins = new CidrBuiltins(); + private final EvaluationContext ctx = new EvaluationContext.Builder().build(); + + /** + * The provider entry in META-INF/services was commented out, so none of these builtins were + * reachable for consumers even though the module was documented as supported. + */ + @Test + void providerIsDiscoverableViaServiceLoader() { + boolean found = + ServiceLoader.load(BuiltinProvider.class, CidrBuiltins.class.getClassLoader()).stream() + .anyMatch(p -> p.type().equals(CidrBuiltins.class)); + + assertTrue(found, "CidrBuiltins should be registered as a BuiltinProvider"); + } + + @Test + void registersAllNetBuiltins() { + assertTrue( + builtins + .builtins() + .keySet() + .containsAll( + List.of( + "net.cidr_contains", + "net.cidr_contains_matches", + "net.cidr_expand", + "net.cidr_intersects", + "net.cidr_is_valid", + "net.cidr_merge", + "net.lookup_ip_addr"))); + } + + // Go's net.ParseCIDR requires a prefix, so a bare address is not valid CIDR notation. + @Test + void cidrIsValidRequiresAPrefix() { + assertEquals(RegoBoolean.TRUE, isValid("192.168.1.0/24")); + assertEquals(RegoBoolean.TRUE, isValid("2002::1234:abcd:ffff:c0a8:101/64")); + assertEquals(RegoBoolean.FALSE, isValid("192.168.1.2")); + assertEquals(RegoBoolean.FALSE, isValid("")); + assertEquals(RegoBoolean.FALSE, isValid("there goes a string")); + } + + // A bare IPv4 address takes Go's classful DefaultMask, not a /32. + @Test + void mergeAppliesClassfulDefaultMaskToBareIpv4() { + assertEquals(List.of("192.0.128.0/23"), merge("192.0.128.0", "192.0.129.0")); + assertEquals( + List.of("192.0.2.0/24"), merge("192.0.2.112", "192.0.2.116/31", "192.0.2.118/31")); + } + + @Test + void mergeCollapsesOverlappingIpv6Prefixes() { + assertEquals( + List.of("2601:600:8a80:207e::/64"), + merge( + "2601:600:8a80:207e:a57d:7567:e2c9:e7b3/64", + "2601:600:8a80:207e:a57d:7567:e2c9:e7b3/128")); + } + + // mergeToPrefixBlocks cannot mix versions, so the two families are merged independently. + @Test + void mergeHandlesMixedIpv4AndIpv6() { + assertEquals( + List.of("192.0.2.0/23", "192.0.4.0/24", "fe80::/120"), + merge("fe80::/120", "192.0.2.0/24", "192.0.3.0/24", "192.0.4.0/25", "192.0.4.128/25")); + } + + @Test + void mergeRejectsBareIpv6() { + BuiltinError e = + assertThrows( + BuiltinError.class, () -> merge("2601:600:8a80:207e:a57d:7567:e2c9:e7b3")); + + // The evaluator prepends the builtin name, so the message must not repeat it. + assertEquals("eval_builtin_error: IPv6 invalid: needs prefix length", e.getMessage()); + } + + @Test + void mergeRejectsMalformedInput() { + for (String bad : List.of("not-an-address", "999.1.1.1", "192.168.1.1/33")) { + BuiltinError e = assertThrows(BuiltinError.class, () -> merge(bad), bad); + assertTrue(e.getMessage().contains(bad), e.getMessage()); + } + } + + // The parser accepts "" and resolves it to the loopback address unless allowEmpty(false) is + // set, which made net.cidr_contains("", "127.0.0.1") answer true. Go's parsers reject it. + @Test + void emptyStringIsNotAnAddress() { + assertEquals(RegoBoolean.FALSE, isValid("")); + assertThrows(BuiltinError.class, () -> merge("")); + assertThrows( + BuiltinError.class, + () -> builtins.expand(ctx, new RegoValue[] {new RegoString("")})); + assertThrows( + BuiltinError.class, + () -> builtins.contains(ctx, new RegoValue[] {new RegoString(""), new RegoString("127.0.0.1")})); + assertThrows( + BuiltinError.class, + () -> + builtins.intersects( + ctx, new RegoValue[] {new RegoString(""), new RegoString("127.0.0.0/8")})); + } + + @Test + void expandReportsGoStyleMessageForInvalidMask() { + BuiltinError e = + assertThrows( + BuiltinError.class, + () -> builtins.expand(ctx, new RegoValue[] {new RegoString("192.168.1.1/33")})); + + assertTrue( + e.getMessage().contains("net.cidr_expand: invalid CIDR address: 192.168.1.1/33"), + e.getMessage()); + } + + @Test + void containsMatchesUsesColonSeparatedOperandMessage() { + RegoValue[] args = { + new RegoArray(List.of(new RegoString("1.1.1.0/24"))), + new RegoArray(List.of(RegoBoolean.TRUE)) + }; + + BuiltinError e = assertThrows(BuiltinError.class, () -> builtins.containsMatches(ctx, args)); + + assertTrue( + e.getMessage() + .contains("net.cidr_contains_matches: operand 2: element must be string or non-empty array"), + e.getMessage()); + } + + private RegoValue isValid(String cidr) { + return builtins.isValid(ctx, new RegoValue[] {new RegoString(cidr)}); + } + + private List merge(String... addrs) { + RegoArray input = + new RegoArray(Arrays.stream(addrs).map(RegoString::new).collect(Collectors.toList())); + RegoSet result = (RegoSet) builtins.merge(ctx, new RegoValue[] {input}); + return result.getValue().stream() + .map(v -> ((RegoString) v).getValue()) + .sorted() + .collect(Collectors.toList()); + } +} diff --git a/opa-evaluator/src/test/resources/compliance/known-missing-builtins.txt b/opa-evaluator/src/test/resources/compliance/known-missing-builtins.txt index 9acde2d1..449d99fd 100644 --- a/opa-evaluator/src/test/resources/compliance/known-missing-builtins.txt +++ b/opa-evaluator/src/test/resources/compliance/known-missing-builtins.txt @@ -35,13 +35,6 @@ yaml.marshal yaml.unmarshal # opa-builtins-net -net.cidr_contains -net.cidr_contains_matches -net.cidr_expand -net.cidr_intersects -net.cidr_is_valid -net.cidr_merge -net.lookup_ip_addr # opa-builtins-regex regex.find_all_string_submatch_n