From 9b598de20c0ec2bb0af546a49a5a7922a3b4c3bb Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Fri, 7 Feb 2025 01:14:17 +0530 Subject: [PATCH 01/13] Add CIDR support in ignore_hosts setting Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- build.gradle | 1 + .../security/auth/AuthFailureListener.java | 7 + .../security/auth/BackendRegistry.java | 29 ++-- .../auth/limiting/AbstractRateLimiter.java | 21 +++ .../security/support/SecurityUtils.java | 63 +++++++ .../org/opensearch/security/UtilTests.java | 88 ---------- .../security/support/SecurityUtilsTest.java | 159 ++++++++++++++++++ 7 files changed, 263 insertions(+), 105 deletions(-) diff --git a/build.gradle b/build.gradle index 2de842572d..f7107aee97 100644 --- a/build.gradle +++ b/build.gradle @@ -589,6 +589,7 @@ dependencies { implementation 'com.nimbusds:nimbus-jose-jwt:9.48' implementation 'com.rfksystems:blake2b:2.0.0' implementation 'com.password4j:password4j:1.8.2' + implementation "commons-net:commons-net:3.11.1" // Action privileges: check tables and compact collections implementation 'com.selectivem.collections:special-collections-complete:1.4.0' diff --git a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java index cbc76cc2e0..c843b34121 100644 --- a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java +++ b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java @@ -18,12 +18,19 @@ package org.opensearch.security.auth; import java.net.InetAddress; +import java.util.List; + +import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.opensearch.security.support.WildcardMatcher; import org.opensearch.security.user.AuthCredentials; public interface AuthFailureListener { + List getIgnoreHosts(); + void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request); WildcardMatcher getIgnoreHostsMatcher(); + + SubnetInfo getSubnetForCidr(String cidr); } diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index b527b8fca2..ea0943241b 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -28,7 +28,6 @@ import java.net.InetAddress; import java.net.InetSocketAddress; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -76,11 +75,13 @@ import static org.apache.http.HttpStatus.SC_FORBIDDEN; import static org.apache.http.HttpStatus.SC_SERVICE_UNAVAILABLE; import static org.apache.http.HttpStatus.SC_UNAUTHORIZED; +import static org.opensearch.security.support.SecurityUtils.matchesCidrPatterns; +import static org.opensearch.security.support.SecurityUtils.matchesHostPatterns; import static com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.SAML_TYPE; public class BackendRegistry { - protected final Logger log = LogManager.getLogger(this.getClass()); + protected static final Logger log = LogManager.getLogger(BackendRegistry.class); private SortedSet restAuthDomains; private Set restAuthorizers; @@ -696,8 +697,7 @@ private boolean isBlocked(InetAddress address) { } for (ClientBlockRegistry clientBlockRegistry : ipClientBlockRegistries) { - WildcardMatcher ignoreHostsMatcher = ((AuthFailureListener) clientBlockRegistry).getIgnoreHostsMatcher(); - if (matchesHostPatterns(ignoreHostsMatcher, address, hostResolverMode)) { + if (matchesIgnoreHostPatterns(clientBlockRegistry, address, hostResolverMode)) { return false; } if (clientBlockRegistry.isBlocked(address)) { @@ -708,21 +708,16 @@ private boolean isBlocked(InetAddress address) { return false; } - public static boolean matchesHostPatterns(WildcardMatcher hostMatcher, InetAddress address, String hostResolverMode) { - if (hostMatcher == null) { + private static boolean matchesIgnoreHostPatterns( + ClientBlockRegistry clientBlockRegistry, + InetAddress address, + String hostResolverMode + ) { + WildcardMatcher ignoreHostsMatcher = ((AuthFailureListener) clientBlockRegistry).getIgnoreHostsMatcher(); + if (ignoreHostsMatcher == null || address == null) { return false; } - if (address != null) { - List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); - if (hostResolverMode != null - && (hostResolverMode.equalsIgnoreCase("ip-hostname") || hostResolverMode.equalsIgnoreCase("ip-hostname-lookup"))) { - final String hostName = address.getHostName(); - valuesToCheck.add(hostName); - } - - return valuesToCheck.stream().anyMatch(hostMatcher); - } - return false; + return matchesHostPatterns(ignoreHostsMatcher, address, hostResolverMode) || matchesCidrPatterns(clientBlockRegistry, address); } private boolean isBlocked(String authBackend, String userName) { diff --git a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java index a0028fc1d6..9a8968b38d 100644 --- a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java +++ b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java @@ -21,6 +21,10 @@ import java.nio.file.Path; import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import org.apache.commons.net.util.SubnetUtils; import org.opensearch.common.settings.Settings; import org.opensearch.security.auth.AuthFailureListener; @@ -35,6 +39,8 @@ public abstract class AbstractRateLimiter implements AuthFailureLi protected final RateTracker rateTracker; protected final List ignoreHosts; private WildcardMatcher ignoreHostMatcher; + // Cache for subnet matchers + private static final Map SUBNET_CACHE = new ConcurrentHashMap<>(); public AbstractRateLimiter(Settings settings, Path configPath, Class clientIdType) { this.ignoreHosts = settings.getAsList("ignore_hosts", Collections.emptyList()); @@ -50,6 +56,11 @@ public AbstractRateLimiter(Settings settings, Path configPath, Class getIgnoreHosts() { + return ignoreHosts; + } + @Override public abstract void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request); @@ -66,6 +77,16 @@ public WildcardMatcher getIgnoreHostsMatcher() { return hostMatcher; } + @Override + public SubnetUtils.SubnetInfo getSubnetForCidr(String cidr) { + return SUBNET_CACHE.computeIfAbsent(cidr, pattern -> { + SubnetUtils utils = new SubnetUtils(pattern); + // Include network and broadcast addresses + utils.setInclusiveHostCount(true); + return utils.getInfo(); + }); + } + @Override public boolean isBlocked(ClientIdType clientId) { return clientBlockRegistry.isBlocked(clientId); diff --git a/src/main/java/org/opensearch/security/support/SecurityUtils.java b/src/main/java/org/opensearch/security/support/SecurityUtils.java index 5686f0076e..685e7643ad 100644 --- a/src/main/java/org/opensearch/security/support/SecurityUtils.java +++ b/src/main/java/org/opensearch/security/support/SecurityUtils.java @@ -26,16 +26,24 @@ package org.opensearch.security.support; +import java.net.InetAddress; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Base64; +import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.common.settings.Settings; +import org.opensearch.security.auth.AuthFailureListener; +import org.opensearch.security.auth.blocking.ClientBlockRegistry; import org.opensearch.security.tools.Hasher; public final class SecurityUtils { @@ -46,6 +54,7 @@ public final class SecurityUtils { static final Pattern ENVBC_PATTERN = Pattern.compile("\\$\\{envbc" + ENV_PATTERN_SUFFIX); static final Pattern ENVBASE64_PATTERN = Pattern.compile("\\$\\{envbase64" + ENV_PATTERN_SUFFIX); public static Locale EN_Locale = forEN(); + private static final Map cidrCache = new ConcurrentHashMap<>(); private SecurityUtils() {} @@ -140,4 +149,58 @@ private static String resolveEnvVar(String envVarName, String mode, boolean bc, return bc ? Hasher.hash(envVarValue.toCharArray(), settings) : envVarValue; } } + + public static boolean matchesHostPatterns(WildcardMatcher hostMatcher, InetAddress address, String hostResolverMode) { + if (hostMatcher == null || address == null) { + return false; + } + List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); + if (hostResolverMode != null + && (hostResolverMode.equalsIgnoreCase("ip-hostname") || hostResolverMode.equalsIgnoreCase("ip-hostname-lookup"))) { + final String hostName = address.getHostName(); + valuesToCheck.add(hostName); + } + + return valuesToCheck.stream().anyMatch(hostMatcher); + } + + /* + public static boolean matchesCidrPatterns(ClientBlockRegistry clientBlockRegistry, InetAddress address) { + if (clientBlockRegistry == null || address == null) { + return false; + } + AuthFailureListener authFailureListener = (AuthFailureListener) clientBlockRegistry; + List patterns = authFailureListener.getIgnoreHosts(); + for (String pattern : patterns) { + if (pattern.contains("/")) { + try { + SubnetInfo subnetInfo = authFailureListener.getSubnetForCidr(pattern); + if (subnetInfo.isInRange(address.getHostAddress())) { + return true; + } + } catch (IllegalArgumentException e) { + log.warn("Invalid CIDR pattern: {}", pattern); + // do not break loop, continue through other patterns + } + } + } + return false; + } + */ + public static boolean matchesCidrPatterns(ClientBlockRegistry clientBlockRegistry, InetAddress address) { + if (clientBlockRegistry == null || address == null) { + return false; + } + AuthFailureListener authFailureListener = (AuthFailureListener) clientBlockRegistry; + String hostAddress = address.getHostAddress(); + return authFailureListener.getIgnoreHosts().parallelStream().filter(pattern -> pattern.indexOf('/') != -1).anyMatch(pattern -> { + try { + SubnetInfo subnetInfo = cidrCache.computeIfAbsent(pattern, authFailureListener::getSubnetForCidr); + return subnetInfo.isInRange(hostAddress); + } catch (IllegalArgumentException e) { + log.warn("Invalid CIDR pattern: {}", pattern); + return false; + } + }); + } } diff --git a/src/test/java/org/opensearch/security/UtilTests.java b/src/test/java/org/opensearch/security/UtilTests.java index fcfd26576f..15361b027c 100644 --- a/src/test/java/org/opensearch/security/UtilTests.java +++ b/src/test/java/org/opensearch/security/UtilTests.java @@ -26,15 +26,11 @@ package org.opensearch.security; -import java.net.InetAddress; -import java.net.UnknownHostException; -import java.util.List; import java.util.Map; import org.junit.Test; import org.opensearch.common.settings.Settings; -import org.opensearch.security.auth.BackendRegistry; import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.hasher.PasswordHasherFactory; import org.opensearch.security.support.ConfigConstants; @@ -226,88 +222,4 @@ public void testNoEnvReplace() { ); } } - - @Test - public void testHostMatching() throws UnknownHostException { - assertThat(BackendRegistry.matchesHostPatterns(null, null, "ip-only"), is(false)); - assertThat(BackendRegistry.matchesHostPatterns(null, null, null), is(false)); - assertThat(BackendRegistry.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.1")), null, "ip-only"), is(false)); - assertThat(BackendRegistry.matchesHostPatterns(null, InetAddress.getByName("127.0.0.1"), "ip-only"), is(false)); - assertThat( - BackendRegistry.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.1")), InetAddress.getByName("127.0.0.1"), "ip-only"), - is(true) - ); - assertThat( - BackendRegistry.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.*")), InetAddress.getByName("127.0.0.1"), "ip-only"), - is(true) - ); - assertThat( - BackendRegistry.matchesHostPatterns( - WildcardMatcher.from(List.of("127.0.0.1")), - InetAddress.getByName("localhost"), - "ip-hostname" - ), - is(true) - ); - assertThat( - BackendRegistry.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.1")), InetAddress.getByName("localhost"), "ip-only"), - is(true) - ); - assertThat( - BackendRegistry.matchesHostPatterns( - WildcardMatcher.from(List.of("127.0.0.1")), - InetAddress.getByName("localhost"), - "ip-hostname" - ), - is(true) - ); - assertThat( - BackendRegistry.matchesHostPatterns( - WildcardMatcher.from(List.of("127.0.0.1")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(false) - ); - assertThat( - BackendRegistry.matchesHostPatterns( - WildcardMatcher.from(List.of("example.org")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(true) - ); - assertThat( - BackendRegistry.matchesHostPatterns( - WildcardMatcher.from(List.of("example.org")), - InetAddress.getByName("example.org"), - "ip-only" - ), - is(false) - ); - assertThat( - BackendRegistry.matchesHostPatterns( - WildcardMatcher.from(List.of("*example.org")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(true) - ); - assertThat( - BackendRegistry.matchesHostPatterns( - WildcardMatcher.from(List.of("example.*")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(true) - ); - assertThat( - BackendRegistry.matchesHostPatterns( - WildcardMatcher.from(List.of("opensearch.org")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(false) - ); - } } diff --git a/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java b/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java index f0645d4958..12f5c9fbff 100644 --- a/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java +++ b/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java @@ -10,14 +10,21 @@ */ package org.opensearch.security.support; +import java.net.InetAddress; +import java.net.UnknownHostException; import java.util.Collection; import java.util.List; import java.util.function.Predicate; import org.junit.Test; +import org.opensearch.common.settings.Settings; +import org.opensearch.security.auth.blocking.ClientBlockRegistry; +import org.opensearch.security.auth.limiting.AddressBasedRateLimiter; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.opensearch.security.support.SecurityUtils.ENVBASE64_PATTERN; import static org.opensearch.security.support.SecurityUtils.ENVBC_PATTERN; import static org.opensearch.security.support.SecurityUtils.ENV_PATTERN; @@ -70,4 +77,156 @@ private void checkKeysWithPredicate(Collection keys, String predicateNam ); }); } + + @Test + public void testHostMatching() throws UnknownHostException { + assertThat(SecurityUtils.matchesHostPatterns(null, null, "ip-only"), is(false)); + assertThat(SecurityUtils.matchesHostPatterns(null, null, null), is(false)); + assertThat(SecurityUtils.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.1")), null, "ip-only"), is(false)); + assertThat(SecurityUtils.matchesHostPatterns(null, InetAddress.getByName("127.0.0.1"), "ip-only"), is(false)); + assertThat( + SecurityUtils.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.1")), InetAddress.getByName("127.0.0.1"), "ip-only"), + is(true) + ); + assertThat( + SecurityUtils.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.*")), InetAddress.getByName("127.0.0.1"), "ip-only"), + is(true) + ); + assertThat( + SecurityUtils.matchesHostPatterns( + WildcardMatcher.from(List.of("127.0.0.1")), + InetAddress.getByName("localhost"), + "ip-hostname" + ), + is(true) + ); + assertThat( + SecurityUtils.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.1")), InetAddress.getByName("localhost"), "ip-only"), + is(true) + ); + assertThat( + SecurityUtils.matchesHostPatterns( + WildcardMatcher.from(List.of("127.0.0.1")), + InetAddress.getByName("localhost"), + "ip-hostname" + ), + is(true) + ); + assertThat( + SecurityUtils.matchesHostPatterns( + WildcardMatcher.from(List.of("127.0.0.1")), + InetAddress.getByName("example.org"), + "ip-hostname" + ), + is(false) + ); + assertThat( + SecurityUtils.matchesHostPatterns( + WildcardMatcher.from(List.of("example.org")), + InetAddress.getByName("example.org"), + "ip-hostname" + ), + is(true) + ); + assertThat( + SecurityUtils.matchesHostPatterns( + WildcardMatcher.from(List.of("example.org")), + InetAddress.getByName("example.org"), + "ip-only" + ), + is(false) + ); + assertThat( + SecurityUtils.matchesHostPatterns( + WildcardMatcher.from(List.of("*example.org")), + InetAddress.getByName("example.org"), + "ip-hostname" + ), + is(true) + ); + assertThat( + SecurityUtils.matchesHostPatterns( + WildcardMatcher.from(List.of("example.*")), + InetAddress.getByName("example.org"), + "ip-hostname" + ), + is(true) + ); + assertThat( + SecurityUtils.matchesHostPatterns( + WildcardMatcher.from(List.of("opensearch.org")), + InetAddress.getByName("example.org"), + "ip-hostname" + ), + is(false) + ); + } + + @Test + public void testMatchesCidrPatternNullValues() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24").build(); + InetAddress address = InetAddress.getByName("192.168.1.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + + assertThat(SecurityUtils.matchesCidrPatterns(null, address), is(false)); + assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, null), is(false)); + } + + @Test + public void testMatchesCidrPatternIpOnly() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0").build(); + InetAddress address = InetAddress.getByName("192.168.1.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + + assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(false)); + } + + @Test + public void testMatchesCidrPatternHostOnly() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "example.com").build(); + InetAddress address = InetAddress.getByName("192.168.1.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + + assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(false)); + } + + @Test + public void testMatchesCidrPatternSingleValidCidrMatch() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24").build(); + InetAddress address = InetAddress.getByName("192.168.1.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(true)); + } + + @Test + public void testMatchesCidrPatternSingleValidCidrNoMatch() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24").build(); + InetAddress address = InetAddress.getByName("10.0.0.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(false)); + } + + @Test + public void testMatchesCidrPatternMultipleValidCidrsMatch() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24,10.0.0.0/8").build(); + InetAddress address = InetAddress.getByName("192.168.1.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(true)); + } + + @Test + public void testMatchesCidrPatternInvalidCidrWithValidAddressAndInvalidCidrLast() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24, invalid/cidr").build(); + InetAddress address = InetAddress.getByName("192.168.1.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(true)); + } + + @Test + public void testMatchesCidrPatternInvalidCidrWithValidAddressAndInvalidCidrFirst() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "invalid/cidr, 192.168.1.0/24").build(); + InetAddress address = InetAddress.getByName("192.168.1.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(true)); + } } From 6603a890dee8782fa7aed60064554a0cbf6aa774 Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Fri, 7 Feb 2025 01:17:31 +0530 Subject: [PATCH 02/13] Remove commented method/code Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../security/support/SecurityUtils.java | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/src/main/java/org/opensearch/security/support/SecurityUtils.java b/src/main/java/org/opensearch/security/support/SecurityUtils.java index 685e7643ad..a170c6dcec 100644 --- a/src/main/java/org/opensearch/security/support/SecurityUtils.java +++ b/src/main/java/org/opensearch/security/support/SecurityUtils.java @@ -164,29 +164,6 @@ public static boolean matchesHostPatterns(WildcardMatcher hostMatcher, InetAddre return valuesToCheck.stream().anyMatch(hostMatcher); } - /* - public static boolean matchesCidrPatterns(ClientBlockRegistry clientBlockRegistry, InetAddress address) { - if (clientBlockRegistry == null || address == null) { - return false; - } - AuthFailureListener authFailureListener = (AuthFailureListener) clientBlockRegistry; - List patterns = authFailureListener.getIgnoreHosts(); - for (String pattern : patterns) { - if (pattern.contains("/")) { - try { - SubnetInfo subnetInfo = authFailureListener.getSubnetForCidr(pattern); - if (subnetInfo.isInRange(address.getHostAddress())) { - return true; - } - } catch (IllegalArgumentException e) { - log.warn("Invalid CIDR pattern: {}", pattern); - // do not break loop, continue through other patterns - } - } - } - return false; - } - */ public static boolean matchesCidrPatterns(ClientBlockRegistry clientBlockRegistry, InetAddress address) { if (clientBlockRegistry == null || address == null) { return false; From 8aca1e68243ab526b1f810f82a51813970b0258b Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Mon, 10 Feb 2025 14:42:56 +0530 Subject: [PATCH 03/13] Move IP matching to cidr matcher Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../security/auth/BackendRegistry.java | 7 +- .../security/support/SecurityUtils.java | 51 ++++++---- .../security/support/SecurityUtilsTest.java | 93 ++++++++++++------- 3 files changed, 98 insertions(+), 53 deletions(-) diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index ea0943241b..ae0d6c3d7c 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -75,8 +75,8 @@ import static org.apache.http.HttpStatus.SC_FORBIDDEN; import static org.apache.http.HttpStatus.SC_SERVICE_UNAVAILABLE; import static org.apache.http.HttpStatus.SC_UNAUTHORIZED; -import static org.opensearch.security.support.SecurityUtils.matchesCidrPatterns; -import static org.opensearch.security.support.SecurityUtils.matchesHostPatterns; +import static org.opensearch.security.support.SecurityUtils.matchesHostNamePatterns; +import static org.opensearch.security.support.SecurityUtils.matchesIpAndCidrPatterns; import static com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.SAML_TYPE; public class BackendRegistry { @@ -717,7 +717,8 @@ private static boolean matchesIgnoreHostPatterns( if (ignoreHostsMatcher == null || address == null) { return false; } - return matchesHostPatterns(ignoreHostsMatcher, address, hostResolverMode) || matchesCidrPatterns(clientBlockRegistry, address); + return matchesHostNamePatterns(ignoreHostsMatcher, address, hostResolverMode) + || matchesIpAndCidrPatterns(clientBlockRegistry, address); } private boolean isBlocked(String authBackend, String userName) { diff --git a/src/main/java/org/opensearch/security/support/SecurityUtils.java b/src/main/java/org/opensearch/security/support/SecurityUtils.java index a170c6dcec..f4ab7f7ad9 100644 --- a/src/main/java/org/opensearch/security/support/SecurityUtils.java +++ b/src/main/java/org/opensearch/security/support/SecurityUtils.java @@ -150,34 +150,51 @@ private static String resolveEnvVar(String envVarName, String mode, boolean bc, } } - public static boolean matchesHostPatterns(WildcardMatcher hostMatcher, InetAddress address, String hostResolverMode) { - if (hostMatcher == null || address == null) { + // This is used to match host names - for e.g. *.example.local to dev.example.local + public static boolean matchesHostNamePatterns(WildcardMatcher hostMatcher, InetAddress address, String hostResolverMode) { + if (address == null || hostMatcher == null) { return false; } - List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); + + // Only proceed with hostname checks if hostResolverMode is configured if (hostResolverMode != null && (hostResolverMode.equalsIgnoreCase("ip-hostname") || hostResolverMode.equalsIgnoreCase("ip-hostname-lookup"))) { - final String hostName = address.getHostName(); - valuesToCheck.add(hostName); + try { + List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); + final String hostName = address.getHostName(); + valuesToCheck.add(hostName); + return valuesToCheck.parallelStream().anyMatch(hostMatcher); + } catch (Exception e) { + log.warn("Failed to resolve hostname for {}: {}", address.getHostAddress(), e.getMessage()); + return false; + } } - - return valuesToCheck.stream().anyMatch(hostMatcher); + return false; } - public static boolean matchesCidrPatterns(ClientBlockRegistry clientBlockRegistry, InetAddress address) { - if (clientBlockRegistry == null || address == null) { + // This is used to match IP addresses - for e.g. 192.168.0.1 to 192.168.0.0/16 + public static boolean matchesIpAndCidrPatterns(ClientBlockRegistry clientBlockRegistry, InetAddress address) { + if (address == null || clientBlockRegistry == null) { return false; } + AuthFailureListener authFailureListener = (AuthFailureListener) clientBlockRegistry; - String hostAddress = address.getHostAddress(); - return authFailureListener.getIgnoreHosts().parallelStream().filter(pattern -> pattern.indexOf('/') != -1).anyMatch(pattern -> { - try { - SubnetInfo subnetInfo = cidrCache.computeIfAbsent(pattern, authFailureListener::getSubnetForCidr); - return subnetInfo.isInRange(hostAddress); - } catch (IllegalArgumentException e) { - log.warn("Invalid CIDR pattern: {}", pattern); - return false; + final String hostAddress = address.getHostAddress(); + + return authFailureListener.getIgnoreHosts().parallelStream().anyMatch(pattern -> { + // Handle CIDR patterns + if (pattern.indexOf('/') != -1) { + try { + SubnetInfo subnetInfo = cidrCache.computeIfAbsent(pattern, authFailureListener::getSubnetForCidr); + return subnetInfo.isInRange(hostAddress); + } catch (IllegalArgumentException e) { + log.warn("Invalid CIDR pattern: {}", pattern); + return false; + } } + + // Handle both direct IPs and wildcard IP patterns using WildcardMatcher + return authFailureListener.getIgnoreHostsMatcher().test(hostAddress); }); } } diff --git a/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java b/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java index 12f5c9fbff..70e86f5444 100644 --- a/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java +++ b/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java @@ -79,21 +79,31 @@ private void checkKeysWithPredicate(Collection keys, String predicateNam } @Test - public void testHostMatching() throws UnknownHostException { - assertThat(SecurityUtils.matchesHostPatterns(null, null, "ip-only"), is(false)); - assertThat(SecurityUtils.matchesHostPatterns(null, null, null), is(false)); - assertThat(SecurityUtils.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.1")), null, "ip-only"), is(false)); - assertThat(SecurityUtils.matchesHostPatterns(null, InetAddress.getByName("127.0.0.1"), "ip-only"), is(false)); + public void testHostNameMatching() throws UnknownHostException { + assertThat(SecurityUtils.matchesHostNamePatterns(null, null, "ip-only"), is(false)); + assertThat(SecurityUtils.matchesHostNamePatterns(null, null, null), is(false)); + assertThat(SecurityUtils.matchesHostNamePatterns(WildcardMatcher.from(List.of("127.0.0.1")), null, "ip-only"), is(false)); + assertThat(SecurityUtils.matchesHostNamePatterns(null, InetAddress.getByName("127.0.0.1"), "ip-only"), is(false)); + + // "ip-only" modes are handled by matchesIpAndCidrPatterns assertThat( - SecurityUtils.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.1")), InetAddress.getByName("127.0.0.1"), "ip-only"), - is(true) + SecurityUtils.matchesHostNamePatterns( + WildcardMatcher.from(List.of("127.0.0.1")), + InetAddress.getByName("127.0.0.1"), + "ip-only" + ), + is(false) ); assertThat( - SecurityUtils.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.*")), InetAddress.getByName("127.0.0.1"), "ip-only"), - is(true) + SecurityUtils.matchesHostNamePatterns( + WildcardMatcher.from(List.of("127.0.0.*")), + InetAddress.getByName("127.0.0.1"), + "ip-only" + ), + is(false) ); assertThat( - SecurityUtils.matchesHostPatterns( + SecurityUtils.matchesHostNamePatterns( WildcardMatcher.from(List.of("127.0.0.1")), InetAddress.getByName("localhost"), "ip-hostname" @@ -101,11 +111,7 @@ public void testHostMatching() throws UnknownHostException { is(true) ); assertThat( - SecurityUtils.matchesHostPatterns(WildcardMatcher.from(List.of("127.0.0.1")), InetAddress.getByName("localhost"), "ip-only"), - is(true) - ); - assertThat( - SecurityUtils.matchesHostPatterns( + SecurityUtils.matchesHostNamePatterns( WildcardMatcher.from(List.of("127.0.0.1")), InetAddress.getByName("localhost"), "ip-hostname" @@ -113,7 +119,7 @@ public void testHostMatching() throws UnknownHostException { is(true) ); assertThat( - SecurityUtils.matchesHostPatterns( + SecurityUtils.matchesHostNamePatterns( WildcardMatcher.from(List.of("127.0.0.1")), InetAddress.getByName("example.org"), "ip-hostname" @@ -121,7 +127,7 @@ public void testHostMatching() throws UnknownHostException { is(false) ); assertThat( - SecurityUtils.matchesHostPatterns( + SecurityUtils.matchesHostNamePatterns( WildcardMatcher.from(List.of("example.org")), InetAddress.getByName("example.org"), "ip-hostname" @@ -129,7 +135,7 @@ public void testHostMatching() throws UnknownHostException { is(true) ); assertThat( - SecurityUtils.matchesHostPatterns( + SecurityUtils.matchesHostNamePatterns( WildcardMatcher.from(List.of("example.org")), InetAddress.getByName("example.org"), "ip-only" @@ -137,7 +143,7 @@ public void testHostMatching() throws UnknownHostException { is(false) ); assertThat( - SecurityUtils.matchesHostPatterns( + SecurityUtils.matchesHostNamePatterns( WildcardMatcher.from(List.of("*example.org")), InetAddress.getByName("example.org"), "ip-hostname" @@ -145,7 +151,7 @@ public void testHostMatching() throws UnknownHostException { is(true) ); assertThat( - SecurityUtils.matchesHostPatterns( + SecurityUtils.matchesHostNamePatterns( WildcardMatcher.from(List.of("example.*")), InetAddress.getByName("example.org"), "ip-hostname" @@ -153,7 +159,7 @@ public void testHostMatching() throws UnknownHostException { is(true) ); assertThat( - SecurityUtils.matchesHostPatterns( + SecurityUtils.matchesHostNamePatterns( WildcardMatcher.from(List.of("opensearch.org")), InetAddress.getByName("example.org"), "ip-hostname" @@ -168,17 +174,38 @@ public void testMatchesCidrPatternNullValues() throws UnknownHostException { InetAddress address = InetAddress.getByName("192.168.1.1"); ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesCidrPatterns(null, address), is(false)); - assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, null), is(false)); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(null, address), is(false)); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, null), is(false)); } @Test - public void testMatchesCidrPatternIpOnly() throws UnknownHostException { + public void testMatchesCidrPatternIpOnlyNoMatch() throws UnknownHostException { Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0").build(); InetAddress address = InetAddress.getByName("192.168.1.1"); ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(false)); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(false)); + } + + @Test + public void testMatchesCidrPatternIpOnlyMatch() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.1").build(); + InetAddress address = InetAddress.getByName("192.168.1.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); + } + + @Test + public void testMatchesCidrPatternIpAndCidrMatch() throws UnknownHostException { + Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.1/32,192.168.0.0/16").build(); + InetAddress address = InetAddress.getByName("192.168.1.1"); + ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); + + // match CIDR 192.168.0.0/16 + address = InetAddress.getByName("192.168.0.2"); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); } @Test @@ -187,23 +214,23 @@ public void testMatchesCidrPatternHostOnly() throws UnknownHostException { InetAddress address = InetAddress.getByName("192.168.1.1"); ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(false)); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(false)); } @Test - public void testMatchesCidrPatternSingleValidCidrMatch() throws UnknownHostException { + public void testMatchesCidrPatternSingleValidIpAndCidrMatch() throws UnknownHostException { Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24").build(); InetAddress address = InetAddress.getByName("192.168.1.1"); ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(true)); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); } @Test - public void testMatchesCidrPatternSingleValidCidrNoMatch() throws UnknownHostException { + public void testMatchesCidrPatternSingleValidIpAndCidrNoMatch() throws UnknownHostException { Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24").build(); InetAddress address = InetAddress.getByName("10.0.0.1"); ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(false)); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(false)); } @Test @@ -211,7 +238,7 @@ public void testMatchesCidrPatternMultipleValidCidrsMatch() throws UnknownHostEx Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24,10.0.0.0/8").build(); InetAddress address = InetAddress.getByName("192.168.1.1"); ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(true)); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); } @Test @@ -219,7 +246,7 @@ public void testMatchesCidrPatternInvalidCidrWithValidAddressAndInvalidCidrLast( Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24, invalid/cidr").build(); InetAddress address = InetAddress.getByName("192.168.1.1"); ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(true)); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); } @Test @@ -227,6 +254,6 @@ public void testMatchesCidrPatternInvalidCidrWithValidAddressAndInvalidCidrFirst Settings settings = Settings.builder().put("ignore_hosts", "invalid/cidr, 192.168.1.0/24").build(); InetAddress address = InetAddress.getByName("192.168.1.1"); ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesCidrPatterns(clientBlockRegistry, address), is(true)); + assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); } } From 6efe00b6c1b978730dd9a0d6ef6e9d17ab2daec3 Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Mon, 10 Feb 2025 14:53:07 +0530 Subject: [PATCH 04/13] Remove cache from AbstractRateLimiter, replace parallelStreams with streams Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../auth/limiting/AbstractRateLimiter.java | 14 ++++---------- .../opensearch/security/support/SecurityUtils.java | 4 ++-- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java index 9a8968b38d..1dd661f0e4 100644 --- a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java +++ b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java @@ -21,8 +21,6 @@ import java.nio.file.Path; import java.util.Collections; import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.net.util.SubnetUtils; @@ -39,8 +37,6 @@ public abstract class AbstractRateLimiter implements AuthFailureLi protected final RateTracker rateTracker; protected final List ignoreHosts; private WildcardMatcher ignoreHostMatcher; - // Cache for subnet matchers - private static final Map SUBNET_CACHE = new ConcurrentHashMap<>(); public AbstractRateLimiter(Settings settings, Path configPath, Class clientIdType) { this.ignoreHosts = settings.getAsList("ignore_hosts", Collections.emptyList()); @@ -79,12 +75,10 @@ public WildcardMatcher getIgnoreHostsMatcher() { @Override public SubnetUtils.SubnetInfo getSubnetForCidr(String cidr) { - return SUBNET_CACHE.computeIfAbsent(cidr, pattern -> { - SubnetUtils utils = new SubnetUtils(pattern); - // Include network and broadcast addresses - utils.setInclusiveHostCount(true); - return utils.getInfo(); - }); + SubnetUtils utils = new SubnetUtils(cidr); + // Include network and broadcast addresses + utils.setInclusiveHostCount(true); + return utils.getInfo(); } @Override diff --git a/src/main/java/org/opensearch/security/support/SecurityUtils.java b/src/main/java/org/opensearch/security/support/SecurityUtils.java index f4ab7f7ad9..277102ecc8 100644 --- a/src/main/java/org/opensearch/security/support/SecurityUtils.java +++ b/src/main/java/org/opensearch/security/support/SecurityUtils.java @@ -163,7 +163,7 @@ public static boolean matchesHostNamePatterns(WildcardMatcher hostMatcher, InetA List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); final String hostName = address.getHostName(); valuesToCheck.add(hostName); - return valuesToCheck.parallelStream().anyMatch(hostMatcher); + return valuesToCheck.stream().anyMatch(hostMatcher); } catch (Exception e) { log.warn("Failed to resolve hostname for {}: {}", address.getHostAddress(), e.getMessage()); return false; @@ -181,7 +181,7 @@ public static boolean matchesIpAndCidrPatterns(ClientBlockRegistry AuthFailureListener authFailureListener = (AuthFailureListener) clientBlockRegistry; final String hostAddress = address.getHostAddress(); - return authFailureListener.getIgnoreHosts().parallelStream().anyMatch(pattern -> { + return authFailureListener.getIgnoreHosts().stream().anyMatch(pattern -> { // Handle CIDR patterns if (pattern.indexOf('/') != -1) { try { From ac5cd05e6d5f21be6c3fc1f8f3a8bc57517f219e Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:39:44 +0530 Subject: [PATCH 05/13] Pre-compute subnetUtils instances Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../security/auth/AuthFailureListener.java | 5 +- .../auth/limiting/AbstractRateLimiter.java | 19 +++-- .../security/support/SecurityUtils.java | 26 +++--- .../security/util/IPAddressUtils.java | 79 +++++++++++++++++++ 4 files changed, 107 insertions(+), 22 deletions(-) create mode 100644 src/main/java/org/opensearch/security/util/IPAddressUtils.java diff --git a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java index c843b34121..88b70617fa 100644 --- a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java +++ b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java @@ -19,8 +19,9 @@ import java.net.InetAddress; import java.util.List; +import java.util.Map; -import org.apache.commons.net.util.SubnetUtils.SubnetInfo; +import org.apache.commons.net.util.SubnetUtils; import org.opensearch.security.support.WildcardMatcher; import org.opensearch.security.user.AuthCredentials; @@ -32,5 +33,5 @@ public interface AuthFailureListener { WildcardMatcher getIgnoreHostsMatcher(); - SubnetInfo getSubnetForCidr(String cidr); + Map getSubnetUtilsMatcherMap(); } diff --git a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java index 1dd661f0e4..3309781499 100644 --- a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java +++ b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java @@ -21,6 +21,7 @@ import java.nio.file.Path; import java.util.Collections; import java.util.List; +import java.util.Map; import org.apache.commons.net.util.SubnetUtils; @@ -30,6 +31,7 @@ import org.opensearch.security.auth.blocking.HeapBasedClientBlockRegistry; import org.opensearch.security.support.WildcardMatcher; import org.opensearch.security.user.AuthCredentials; +import org.opensearch.security.util.IPAddressUtils; import org.opensearch.security.util.ratetracking.RateTracker; public abstract class AbstractRateLimiter implements AuthFailureListener, ClientBlockRegistry { @@ -37,6 +39,7 @@ public abstract class AbstractRateLimiter implements AuthFailureLi protected final RateTracker rateTracker; protected final List ignoreHosts; private WildcardMatcher ignoreHostMatcher; + protected final Map subnetUtilsMatcherMap; public AbstractRateLimiter(Settings settings, Path configPath, Class clientIdType) { this.ignoreHosts = settings.getAsList("ignore_hosts", Collections.emptyList()); @@ -50,6 +53,9 @@ public AbstractRateLimiter(Settings settings, Path configPath, Class getIgnoreHosts() { return ignoreHosts; } + @Override + public Map getSubnetUtilsMatcherMap() { + return subnetUtilsMatcherMap; + } + @Override public abstract void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request); @@ -73,14 +84,6 @@ public WildcardMatcher getIgnoreHostsMatcher() { return hostMatcher; } - @Override - public SubnetUtils.SubnetInfo getSubnetForCidr(String cidr) { - SubnetUtils utils = new SubnetUtils(cidr); - // Include network and broadcast addresses - utils.setInclusiveHostCount(true); - return utils.getInfo(); - } - @Override public boolean isBlocked(ClientIdType clientId) { return clientBlockRegistry.isBlocked(clientId); diff --git a/src/main/java/org/opensearch/security/support/SecurityUtils.java b/src/main/java/org/opensearch/security/support/SecurityUtils.java index 277102ecc8..27861fc367 100644 --- a/src/main/java/org/opensearch/security/support/SecurityUtils.java +++ b/src/main/java/org/opensearch/security/support/SecurityUtils.java @@ -37,6 +37,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.net.util.SubnetUtils; import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -181,20 +182,21 @@ public static boolean matchesIpAndCidrPatterns(ClientBlockRegistry AuthFailureListener authFailureListener = (AuthFailureListener) clientBlockRegistry; final String hostAddress = address.getHostAddress(); - return authFailureListener.getIgnoreHosts().stream().anyMatch(pattern -> { - // Handle CIDR patterns - if (pattern.indexOf('/') != -1) { - try { - SubnetInfo subnetInfo = cidrCache.computeIfAbsent(pattern, authFailureListener::getSubnetForCidr); - return subnetInfo.isInRange(hostAddress); - } catch (IllegalArgumentException e) { - log.warn("Invalid CIDR pattern: {}", pattern); - return false; + for (String pattern : authFailureListener.getIgnoreHosts()) { + // Handle CIDR patterns - SubnetUtilsMatcherMap holds a SubnetInfo object for valid IPv4 patterns + if (authFailureListener.getSubnetUtilsMatcherMap().containsKey(pattern)) { + SubnetUtils.SubnetInfo subnetInfo = authFailureListener.getSubnetUtilsMatcherMap().get(pattern); + if (subnetInfo != null && subnetInfo.isInRange(hostAddress)) { + return true; + } + } else { + // Handle both direct IPs and wildcard IP patterns + if (authFailureListener.getIgnoreHostsMatcher().test(hostAddress)) { + return true; } } + } + return false; - // Handle both direct IPs and wildcard IP patterns using WildcardMatcher - return authFailureListener.getIgnoreHostsMatcher().test(hostAddress); - }); } } diff --git a/src/main/java/org/opensearch/security/util/IPAddressUtils.java b/src/main/java/org/opensearch/security/util/IPAddressUtils.java new file mode 100644 index 0000000000..1259b1999b --- /dev/null +++ b/src/main/java/org/opensearch/security/util/IPAddressUtils.java @@ -0,0 +1,79 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.util; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import org.apache.commons.net.util.SubnetUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.security.support.SecurityUtils; + +public class IPAddressUtils { + + protected final static Logger log = LogManager.getLogger(SecurityUtils.class); + private static final String IP_ADDRESS = "(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})"; + private static final String SLASH_FORMAT = IP_ADDRESS + "/(\\d{1,2})"; // 0 -> 32 + private static final Pattern IPV4_CIDR_PATTERN = Pattern.compile(SLASH_FORMAT); + + /** + * Creates a map of CIDR strings to their corresponding SubnetInfo objects + * + * @param hosts List of CIDR patterns to process + * @return Map of valid CIDR strings to their SubnetInfo objects + */ + public static Map createSubnetUtils(List hosts) { + if (hosts == null || hosts.isEmpty()) { + return Collections.emptyMap(); + } + + return hosts.stream() + .filter(Objects::nonNull) + .collect( + Collectors.toMap( + cidr -> cidr, + IPAddressUtils::getSubnetForCidr, + (existing, replacement) -> existing, + () -> new HashMap<>(hosts.size()) + ) + ); + } + + /** + * Creates a SubnetInfo object for a given CIDR pattern + * + * @param cidr CIDR pattern to process + * @return SubnetInfo object for the given CIDR + */ + private static SubnetUtils.SubnetInfo getSubnetForCidr(String cidr) { + SubnetUtils utils = new SubnetUtils(cidr); + utils.setInclusiveHostCount(true); + return utils.getInfo(); + } + + /** + * Validates if a given string matches IPv4 CIDR pattern + * + * @param cidr String to validate + * @return true if the string is a valid IPv4 CIDR pattern, false otherwise + */ + public static boolean isValidIpv4Cidr(String cidr) { + return cidr != null && IPV4_CIDR_PATTERN.matcher(cidr).matches(); + } +} From f8e68af1a31bbde907a752bc09728949851dae6e Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Mon, 3 Mar 2025 21:54:57 +0530 Subject: [PATCH 06/13] Add a new HostAndCidrMatcher for ignore-hosts cidr support Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- build.gradle | 2 +- .../security/auth/AuthFailureListener.java | 8 +- .../security/auth/BackendRegistry.java | 10 +- .../auth/limiting/AbstractRateLimiter.java | 23 +-- .../security/support/HostAndCidrMatcher.java | 104 ++++++++++ .../security/support/SecurityUtils.java | 59 ------ .../security/util/IPAddressUtils.java | 79 -------- .../support/HostAndCidrMatcherTest.java | 187 ++++++++++++++++++ .../security/support/SecurityUtilsTest.java | 186 ----------------- 9 files changed, 303 insertions(+), 355 deletions(-) create mode 100644 src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java delete mode 100644 src/main/java/org/opensearch/security/util/IPAddressUtils.java create mode 100644 src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java diff --git a/build.gradle b/build.gradle index 4dcf557a47..2208fb9921 100644 --- a/build.gradle +++ b/build.gradle @@ -589,7 +589,7 @@ dependencies { implementation 'com.nimbusds:nimbus-jose-jwt:9.48' implementation 'com.rfksystems:blake2b:2.0.0' implementation 'com.password4j:password4j:1.8.2' - implementation "commons-net:commons-net:3.11.1" + implementation "com.github.seancfoley:ipaddress:5.5.1" // Action privileges: check tables and compact collections implementation 'com.selectivem.collections:special-collections-complete:1.4.0' diff --git a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java index 88b70617fa..b770abd29a 100644 --- a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java +++ b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java @@ -19,11 +19,8 @@ import java.net.InetAddress; import java.util.List; -import java.util.Map; -import org.apache.commons.net.util.SubnetUtils; - -import org.opensearch.security.support.WildcardMatcher; +import org.opensearch.security.support.HostAndCidrMatcher; import org.opensearch.security.user.AuthCredentials; public interface AuthFailureListener { @@ -31,7 +28,6 @@ public interface AuthFailureListener { void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request); - WildcardMatcher getIgnoreHostsMatcher(); + HostAndCidrMatcher getIgnoreHostsMatcher(); - Map getSubnetUtilsMatcherMap(); } diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index ae0d6c3d7c..83c42321f1 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -65,7 +65,7 @@ import org.opensearch.security.http.XFFResolver; import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.support.ConfigConstants; -import org.opensearch.security.support.WildcardMatcher; +import org.opensearch.security.support.HostAndCidrMatcher; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; import org.opensearch.threadpool.ThreadPool; @@ -75,8 +75,6 @@ import static org.apache.http.HttpStatus.SC_FORBIDDEN; import static org.apache.http.HttpStatus.SC_SERVICE_UNAVAILABLE; import static org.apache.http.HttpStatus.SC_UNAUTHORIZED; -import static org.opensearch.security.support.SecurityUtils.matchesHostNamePatterns; -import static org.opensearch.security.support.SecurityUtils.matchesIpAndCidrPatterns; import static com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.SAML_TYPE; public class BackendRegistry { @@ -713,12 +711,12 @@ private static boolean matchesIgnoreHostPatterns( InetAddress address, String hostResolverMode ) { - WildcardMatcher ignoreHostsMatcher = ((AuthFailureListener) clientBlockRegistry).getIgnoreHostsMatcher(); + HostAndCidrMatcher ignoreHostsMatcher = ((AuthFailureListener) clientBlockRegistry).getIgnoreHostsMatcher(); if (ignoreHostsMatcher == null || address == null) { return false; } - return matchesHostNamePatterns(ignoreHostsMatcher, address, hostResolverMode) - || matchesIpAndCidrPatterns(clientBlockRegistry, address); + return ignoreHostsMatcher.matches(address, hostResolverMode); + } private boolean isBlocked(String authBackend, String userName) { diff --git a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java index 3309781499..2c9a7dfdd9 100644 --- a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java +++ b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java @@ -21,25 +21,20 @@ import java.nio.file.Path; import java.util.Collections; import java.util.List; -import java.util.Map; - -import org.apache.commons.net.util.SubnetUtils; import org.opensearch.common.settings.Settings; import org.opensearch.security.auth.AuthFailureListener; import org.opensearch.security.auth.blocking.ClientBlockRegistry; import org.opensearch.security.auth.blocking.HeapBasedClientBlockRegistry; -import org.opensearch.security.support.WildcardMatcher; +import org.opensearch.security.support.HostAndCidrMatcher; import org.opensearch.security.user.AuthCredentials; -import org.opensearch.security.util.IPAddressUtils; import org.opensearch.security.util.ratetracking.RateTracker; public abstract class AbstractRateLimiter implements AuthFailureListener, ClientBlockRegistry { protected final ClientBlockRegistry clientBlockRegistry; protected final RateTracker rateTracker; protected final List ignoreHosts; - private WildcardMatcher ignoreHostMatcher; - protected final Map subnetUtilsMatcherMap; + private HostAndCidrMatcher ignoreHostMatcher; public AbstractRateLimiter(Settings settings, Path configPath, Class clientIdType) { this.ignoreHosts = settings.getAsList("ignore_hosts", Collections.emptyList()); @@ -53,9 +48,6 @@ public AbstractRateLimiter(Settings settings, Path configPath, Class getIgnoreHosts() { return ignoreHosts; } - @Override - public Map getSubnetUtilsMatcherMap() { - return subnetUtilsMatcherMap; - } - @Override public abstract void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request); @Override - public WildcardMatcher getIgnoreHostsMatcher() { + public HostAndCidrMatcher getIgnoreHostsMatcher() { if (this.ignoreHostMatcher != null) { return this.ignoreHostMatcher; } - WildcardMatcher hostMatcher = WildcardMatcher.NONE; + HostAndCidrMatcher hostMatcher = new HostAndCidrMatcher(Collections.emptyList()); if (this.ignoreHosts != null && !this.ignoreHosts.isEmpty()) { - hostMatcher = WildcardMatcher.from(this.ignoreHosts); + hostMatcher = new HostAndCidrMatcher(this.ignoreHosts); } this.ignoreHostMatcher = hostMatcher; return hostMatcher; diff --git a/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java new file mode 100644 index 0000000000..3e07b814f1 --- /dev/null +++ b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java @@ -0,0 +1,104 @@ +package org.opensearch.security.support; + +import java.net.InetAddress; +import java.util.ArrayList; +import java.util.List; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import inet.ipaddr.IPAddressString; + +/** + * A utility class that performs matching of IP addresses against hostname patterns and CIDR ranges. + * This matcher supports both wildcard hostname patterns (e.g., *.example.com) and CIDR notation (e.g., 192.168.1.0/24). + */ +public class HostAndCidrMatcher { + private static final String IP_HOSTNAME = "ip-hostname"; + private static final String IP_HOSTNAME_LOOKUP = "ip-hostname-lookup"; + + protected final Logger log = LogManager.getLogger(HostAndCidrMatcher.class); + private final WildcardMatcher hostMatcher; + private final List cidrMatchers; + + /** + * Constructs a new matcher with the specified host patterns. + * + * @param hostPatterns A list of patterns that can include both hostname wildcards + * (e.g., *.example.com) and CIDR ranges (e.g., 192.168.1.0/24). + * Must not be null. + * @throws IllegalArgumentException if hostPatterns is null + */ + public HostAndCidrMatcher(List hostPatterns) { + if (hostPatterns == null) { + throw new IllegalArgumentException("Host patterns cannot be null"); + } + + this.hostMatcher = WildcardMatcher.from(hostPatterns); + this.cidrMatchers = hostPatterns.stream().map(IPAddressString::new).filter(IPAddressString::isIPAddress).toList(); + } + + /** + * Checks if the provided IP address matches any of the configured CIDR ranges. + * + * @param address The IP address to check. Can be either IPv4 or IPv6. + * @return true if the address matches any configured CIDR range, false otherwise + * or if the address is null + */ + public boolean matchesCidr(InetAddress address) { + if (address == null || cidrMatchers == null) { + return false; + } + + try { + IPAddressString addressString = new IPAddressString(address.getHostAddress()); + return cidrMatchers.stream().anyMatch(cidrAddress -> cidrAddress.contains(addressString)); + } catch (Exception e) { + log.warn("Failed to process IP address {}: {}", address, e.getMessage()); + return false; + } + } + + /** + * Checks if the provided IP address matches any of the configured hostname patterns. + * This method can perform DNS lookups depending on the hostResolverMode. + * + * @param address The IP address to check + * @param hostResolverMode The resolution mode. Must be either "ip-hostname" or + * "ip-hostname-lookup" to enable hostname matching + * @return true if the address matches any configured hostname pattern, false otherwise, + * if the address is null, or if the resolver mode is invalid + * @implNote This method may perform DNS lookups which could impact performance + */ + public boolean matchesHostname(InetAddress address, String hostResolverMode) { + if (address == null || hostMatcher == null || hostResolverMode == null) { + return false; + } + + if (!hostResolverMode.equalsIgnoreCase(IP_HOSTNAME) && !hostResolverMode.equalsIgnoreCase(IP_HOSTNAME_LOOKUP)) { + return false; + } + + try { + List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); + final String hostName = address.getHostName(); // potential blocking call + valuesToCheck.add(hostName); + return valuesToCheck.stream().anyMatch(hostMatcher); + } catch (Exception e) { + log.warn("Failed to resolve hostname for {}: {}", address.getHostAddress(), e.getMessage()); + return false; + } + } + + /** + * Checks if the provided IP address matches either hostname patterns or CIDR ranges. + * + * @param address The IP address to check + * @param hostResolverMode The resolution mode for hostname matching + * @return true if the address matches either hostname patterns or CIDR ranges, + * false otherwise + */ + public boolean matches(InetAddress address, String hostResolverMode) { + return matchesHostname(address, hostResolverMode) || matchesCidr(address); + } +} diff --git a/src/main/java/org/opensearch/security/support/SecurityUtils.java b/src/main/java/org/opensearch/security/support/SecurityUtils.java index 27861fc367..5686f0076e 100644 --- a/src/main/java/org/opensearch/security/support/SecurityUtils.java +++ b/src/main/java/org/opensearch/security/support/SecurityUtils.java @@ -26,25 +26,16 @@ package org.opensearch.security.support; -import java.net.InetAddress; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; import java.util.Base64; -import java.util.List; import java.util.Locale; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.apache.commons.net.util.SubnetUtils; -import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.common.settings.Settings; -import org.opensearch.security.auth.AuthFailureListener; -import org.opensearch.security.auth.blocking.ClientBlockRegistry; import org.opensearch.security.tools.Hasher; public final class SecurityUtils { @@ -55,7 +46,6 @@ public final class SecurityUtils { static final Pattern ENVBC_PATTERN = Pattern.compile("\\$\\{envbc" + ENV_PATTERN_SUFFIX); static final Pattern ENVBASE64_PATTERN = Pattern.compile("\\$\\{envbase64" + ENV_PATTERN_SUFFIX); public static Locale EN_Locale = forEN(); - private static final Map cidrCache = new ConcurrentHashMap<>(); private SecurityUtils() {} @@ -150,53 +140,4 @@ private static String resolveEnvVar(String envVarName, String mode, boolean bc, return bc ? Hasher.hash(envVarValue.toCharArray(), settings) : envVarValue; } } - - // This is used to match host names - for e.g. *.example.local to dev.example.local - public static boolean matchesHostNamePatterns(WildcardMatcher hostMatcher, InetAddress address, String hostResolverMode) { - if (address == null || hostMatcher == null) { - return false; - } - - // Only proceed with hostname checks if hostResolverMode is configured - if (hostResolverMode != null - && (hostResolverMode.equalsIgnoreCase("ip-hostname") || hostResolverMode.equalsIgnoreCase("ip-hostname-lookup"))) { - try { - List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); - final String hostName = address.getHostName(); - valuesToCheck.add(hostName); - return valuesToCheck.stream().anyMatch(hostMatcher); - } catch (Exception e) { - log.warn("Failed to resolve hostname for {}: {}", address.getHostAddress(), e.getMessage()); - return false; - } - } - return false; - } - - // This is used to match IP addresses - for e.g. 192.168.0.1 to 192.168.0.0/16 - public static boolean matchesIpAndCidrPatterns(ClientBlockRegistry clientBlockRegistry, InetAddress address) { - if (address == null || clientBlockRegistry == null) { - return false; - } - - AuthFailureListener authFailureListener = (AuthFailureListener) clientBlockRegistry; - final String hostAddress = address.getHostAddress(); - - for (String pattern : authFailureListener.getIgnoreHosts()) { - // Handle CIDR patterns - SubnetUtilsMatcherMap holds a SubnetInfo object for valid IPv4 patterns - if (authFailureListener.getSubnetUtilsMatcherMap().containsKey(pattern)) { - SubnetUtils.SubnetInfo subnetInfo = authFailureListener.getSubnetUtilsMatcherMap().get(pattern); - if (subnetInfo != null && subnetInfo.isInRange(hostAddress)) { - return true; - } - } else { - // Handle both direct IPs and wildcard IP patterns - if (authFailureListener.getIgnoreHostsMatcher().test(hostAddress)) { - return true; - } - } - } - return false; - - } } diff --git a/src/main/java/org/opensearch/security/util/IPAddressUtils.java b/src/main/java/org/opensearch/security/util/IPAddressUtils.java deleted file mode 100644 index 1259b1999b..0000000000 --- a/src/main/java/org/opensearch/security/util/IPAddressUtils.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.util; - -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.regex.Pattern; -import java.util.stream.Collectors; - -import org.apache.commons.net.util.SubnetUtils; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; - -import org.opensearch.security.support.SecurityUtils; - -public class IPAddressUtils { - - protected final static Logger log = LogManager.getLogger(SecurityUtils.class); - private static final String IP_ADDRESS = "(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})"; - private static final String SLASH_FORMAT = IP_ADDRESS + "/(\\d{1,2})"; // 0 -> 32 - private static final Pattern IPV4_CIDR_PATTERN = Pattern.compile(SLASH_FORMAT); - - /** - * Creates a map of CIDR strings to their corresponding SubnetInfo objects - * - * @param hosts List of CIDR patterns to process - * @return Map of valid CIDR strings to their SubnetInfo objects - */ - public static Map createSubnetUtils(List hosts) { - if (hosts == null || hosts.isEmpty()) { - return Collections.emptyMap(); - } - - return hosts.stream() - .filter(Objects::nonNull) - .collect( - Collectors.toMap( - cidr -> cidr, - IPAddressUtils::getSubnetForCidr, - (existing, replacement) -> existing, - () -> new HashMap<>(hosts.size()) - ) - ); - } - - /** - * Creates a SubnetInfo object for a given CIDR pattern - * - * @param cidr CIDR pattern to process - * @return SubnetInfo object for the given CIDR - */ - private static SubnetUtils.SubnetInfo getSubnetForCidr(String cidr) { - SubnetUtils utils = new SubnetUtils(cidr); - utils.setInclusiveHostCount(true); - return utils.getInfo(); - } - - /** - * Validates if a given string matches IPv4 CIDR pattern - * - * @param cidr String to validate - * @return true if the string is a valid IPv4 CIDR pattern, false otherwise - */ - public static boolean isValidIpv4Cidr(String cidr) { - return cidr != null && IPV4_CIDR_PATTERN.matcher(cidr).matches(); - } -} diff --git a/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java new file mode 100644 index 0000000000..ec05e822ae --- /dev/null +++ b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java @@ -0,0 +1,187 @@ +package org.opensearch.security.support; + +import java.net.InetAddress; +import java.util.Arrays; +import java.util.Collections; + +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class HostAndCidrMatcherTest { + + private static final String OPENSEARCH_DOMAIN = "*.opensearch.org"; + private static final String OPENSEARCH_WWW = "www.opensearch.org"; + private static final String EXAMPLE_DOMAIN = "*.example.com"; + private static final String EXAMPLE_WWW = "www.example.com"; + + // CIDR ranges + private static final String PRIVATE_CLASS_A_CIDR = "10.0.0.0/8"; + private static final String PRIVATE_CLASS_B_CIDR = "172.16.0.0/12"; + private static final String PRIVATE_CLASS_C_CIDR = "192.168.1.0/24"; + private static final String IPV6_DOCUMENTATION_CIDR = "2001:db8::/32"; + + // IP addresses within the CIDR ranges + private static final String PRIVATE_CLASS_A_IP = "10.10.10.10"; + private static final String PRIVATE_CLASS_B_IP = "172.16.1.1"; + private static final String PRIVATE_CLASS_C_IP = "192.168.1.100"; + private static final String IPV6_DOCUMENTATION_IP = "2001:db8:1:2::"; + + private HostAndCidrMatcher matcher; + + @Test(expected = IllegalArgumentException.class) + public void constructorShouldThrowExceptionForNullInput() { + new HostAndCidrMatcher(null); + } + + @Test + public void shouldReturnFalseForEmptyResolverMode() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); + InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); + assertFalse(matcher.matchesHostname(address, "")); + } + + @Test + public void shouldReturnFalseForInvalidResolverMode() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); + InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); + assertFalse(matcher.matchesHostname(address, "invalid-mode")); + } + + @Test + public void shouldReturnFalseForNullResolverMode() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); + InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); + assertFalse(matcher.matchesHostname(address, null)); + } + + @Test + public void shouldReturnFalseForWrongCaseResolverMode() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); + InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); + assertFalse(matcher.matchesHostname(address, "IP_HOSTNAME")); + } + + @Test + public void shouldMatchIpv4WithinCidrRange() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR, PRIVATE_CLASS_A_CIDR)); + InetAddress address = InetAddress.getByName(PRIVATE_CLASS_C_IP); + assertTrue(matcher.matchesCidr(address)); + } + + @Test + public void shouldNotMatchIpv4OutsideCidrRange() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR, PRIVATE_CLASS_A_CIDR)); + InetAddress address = InetAddress.getByName("192.168.2.100"); + assertFalse(matcher.matchesCidr(address)); + } + + @Test + public void shouldMatchIpv6WithinCidrRange() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(IPV6_DOCUMENTATION_CIDR)); + InetAddress address = InetAddress.getByName(IPV6_DOCUMENTATION_IP); + assertTrue(matcher.matchesCidr(address)); + } + + @Test + public void shouldHandleNullAddressInCidrMatching() { + matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR)); + assertFalse(matcher.matchesCidr(null)); + } + + @Test + public void shouldMatchValidHostnamePattern() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN, OPENSEARCH_WWW)); + InetAddress address = InetAddress.getByName(EXAMPLE_WWW); + assertTrue(matcher.matchesHostname(address, "ip-hostname")); + } + + @Test + public void shouldHandleNullAddressInHostnameMatching() { + matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN)); + assertFalse(matcher.matchesHostname(null, "ip-hostname")); + } + + @Test + public void shouldMatchWhenIpMatchesCidrOnly() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN, PRIVATE_CLASS_C_CIDR)); + InetAddress address = InetAddress.getByName(PRIVATE_CLASS_C_IP); + assertTrue(matcher.matches(address, "ip-hostname")); + } + + @Test + public void shouldMatchWhenHostnameMatchesOnly() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN, PRIVATE_CLASS_C_CIDR)); + InetAddress address = InetAddress.getByName(EXAMPLE_WWW); + assertTrue(matcher.matches(address, "ip-hostname")); + } + + @Test + public void shouldNotMatchWhenNeitherMatches() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN, PRIVATE_CLASS_C_CIDR)); + InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); + assertFalse(matcher.matches(address, "ip-hostname")); + } + + @Test + public void shouldHandleEmptyPatternList() throws Exception { + matcher = new HostAndCidrMatcher(Collections.emptyList()); + InetAddress address = InetAddress.getByName(PRIVATE_CLASS_C_IP); + assertFalse(matcher.matches(address, "ip-hostname")); + } + + @Test + public void shouldHandleInvalidCidrNotation() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList("invalid/cidr/notation")); + InetAddress address = InetAddress.getByName(PRIVATE_CLASS_C_IP); + assertFalse(matcher.matchesCidr(address)); + } + + @Test(expected = Exception.class) + public void shouldHandleMalformedIpAddresses() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR)); + InetAddress address = InetAddress.getByName("invalid.ip.address"); + matcher.matchesCidr(address); + } + + @Test + public void shouldMatchIpHostnameLookupMode() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); + InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); + assertTrue(matcher.matchesHostname(address, "ip-hostname-lookup")); + } + + @Test + public void shouldHandleMultipleCidrRanges() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_A_CIDR, PRIVATE_CLASS_B_CIDR, PRIVATE_CLASS_C_CIDR)); + + InetAddress address1 = InetAddress.getByName(PRIVATE_CLASS_A_IP); + InetAddress address2 = InetAddress.getByName(PRIVATE_CLASS_B_IP); + InetAddress address3 = InetAddress.getByName(PRIVATE_CLASS_C_IP); + + assertTrue(matcher.matchesCidr(address1)); + assertTrue(matcher.matchesCidr(address2)); + assertTrue(matcher.matchesCidr(address3)); + } + + @Test + public void shouldHandleMultipleHostnamePatterns() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN, OPENSEARCH_DOMAIN)); + InetAddress address1 = InetAddress.getByName(EXAMPLE_WWW); + InetAddress address2 = InetAddress.getByName(OPENSEARCH_WWW); + + assertTrue(matcher.matchesHostname(address1, "ip-hostname")); + assertTrue(matcher.matchesHostname(address2, "ip-hostname")); + } + + @Test + public void shouldHandleMixedIpv4AndIpv6Patterns() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR, IPV6_DOCUMENTATION_CIDR, EXAMPLE_DOMAIN)); + InetAddress ipv4Address = InetAddress.getByName(PRIVATE_CLASS_C_IP); + InetAddress ipv6Address = InetAddress.getByName(IPV6_DOCUMENTATION_IP); + + assertTrue(matcher.matchesCidr(ipv4Address)); + assertTrue(matcher.matchesCidr(ipv6Address)); + } +} diff --git a/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java b/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java index 70e86f5444..f0645d4958 100644 --- a/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java +++ b/src/test/java/org/opensearch/security/support/SecurityUtilsTest.java @@ -10,21 +10,14 @@ */ package org.opensearch.security.support; -import java.net.InetAddress; -import java.net.UnknownHostException; import java.util.Collection; import java.util.List; import java.util.function.Predicate; import org.junit.Test; -import org.opensearch.common.settings.Settings; -import org.opensearch.security.auth.blocking.ClientBlockRegistry; -import org.opensearch.security.auth.limiting.AddressBasedRateLimiter; - import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.opensearch.security.support.SecurityUtils.ENVBASE64_PATTERN; import static org.opensearch.security.support.SecurityUtils.ENVBC_PATTERN; import static org.opensearch.security.support.SecurityUtils.ENV_PATTERN; @@ -77,183 +70,4 @@ private void checkKeysWithPredicate(Collection keys, String predicateNam ); }); } - - @Test - public void testHostNameMatching() throws UnknownHostException { - assertThat(SecurityUtils.matchesHostNamePatterns(null, null, "ip-only"), is(false)); - assertThat(SecurityUtils.matchesHostNamePatterns(null, null, null), is(false)); - assertThat(SecurityUtils.matchesHostNamePatterns(WildcardMatcher.from(List.of("127.0.0.1")), null, "ip-only"), is(false)); - assertThat(SecurityUtils.matchesHostNamePatterns(null, InetAddress.getByName("127.0.0.1"), "ip-only"), is(false)); - - // "ip-only" modes are handled by matchesIpAndCidrPatterns - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("127.0.0.1")), - InetAddress.getByName("127.0.0.1"), - "ip-only" - ), - is(false) - ); - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("127.0.0.*")), - InetAddress.getByName("127.0.0.1"), - "ip-only" - ), - is(false) - ); - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("127.0.0.1")), - InetAddress.getByName("localhost"), - "ip-hostname" - ), - is(true) - ); - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("127.0.0.1")), - InetAddress.getByName("localhost"), - "ip-hostname" - ), - is(true) - ); - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("127.0.0.1")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(false) - ); - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("example.org")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(true) - ); - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("example.org")), - InetAddress.getByName("example.org"), - "ip-only" - ), - is(false) - ); - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("*example.org")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(true) - ); - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("example.*")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(true) - ); - assertThat( - SecurityUtils.matchesHostNamePatterns( - WildcardMatcher.from(List.of("opensearch.org")), - InetAddress.getByName("example.org"), - "ip-hostname" - ), - is(false) - ); - } - - @Test - public void testMatchesCidrPatternNullValues() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24").build(); - InetAddress address = InetAddress.getByName("192.168.1.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - - assertThat(SecurityUtils.matchesIpAndCidrPatterns(null, address), is(false)); - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, null), is(false)); - } - - @Test - public void testMatchesCidrPatternIpOnlyNoMatch() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0").build(); - InetAddress address = InetAddress.getByName("192.168.1.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(false)); - } - - @Test - public void testMatchesCidrPatternIpOnlyMatch() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.1").build(); - InetAddress address = InetAddress.getByName("192.168.1.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); - } - - @Test - public void testMatchesCidrPatternIpAndCidrMatch() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.1/32,192.168.0.0/16").build(); - InetAddress address = InetAddress.getByName("192.168.1.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); - - // match CIDR 192.168.0.0/16 - address = InetAddress.getByName("192.168.0.2"); - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); - } - - @Test - public void testMatchesCidrPatternHostOnly() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "example.com").build(); - InetAddress address = InetAddress.getByName("192.168.1.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(false)); - } - - @Test - public void testMatchesCidrPatternSingleValidIpAndCidrMatch() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24").build(); - InetAddress address = InetAddress.getByName("192.168.1.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); - } - - @Test - public void testMatchesCidrPatternSingleValidIpAndCidrNoMatch() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24").build(); - InetAddress address = InetAddress.getByName("10.0.0.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(false)); - } - - @Test - public void testMatchesCidrPatternMultipleValidCidrsMatch() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24,10.0.0.0/8").build(); - InetAddress address = InetAddress.getByName("192.168.1.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); - } - - @Test - public void testMatchesCidrPatternInvalidCidrWithValidAddressAndInvalidCidrLast() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "192.168.1.0/24, invalid/cidr").build(); - InetAddress address = InetAddress.getByName("192.168.1.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); - } - - @Test - public void testMatchesCidrPatternInvalidCidrWithValidAddressAndInvalidCidrFirst() throws UnknownHostException { - Settings settings = Settings.builder().put("ignore_hosts", "invalid/cidr, 192.168.1.0/24").build(); - InetAddress address = InetAddress.getByName("192.168.1.1"); - ClientBlockRegistry clientBlockRegistry = new AddressBasedRateLimiter(settings, null); - assertThat(SecurityUtils.matchesIpAndCidrPatterns(clientBlockRegistry, address), is(true)); - } } From d2cab5c9e0d1ec9bc9abfcd07f47bd76bc5cf1d4 Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Mon, 3 Mar 2025 21:57:12 +0530 Subject: [PATCH 07/13] Remove unused method Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../org/opensearch/security/auth/AuthFailureListener.java | 2 -- .../security/auth/limiting/AbstractRateLimiter.java | 5 ----- 2 files changed, 7 deletions(-) diff --git a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java index b770abd29a..297adbfafd 100644 --- a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java +++ b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java @@ -18,13 +18,11 @@ package org.opensearch.security.auth; import java.net.InetAddress; -import java.util.List; import org.opensearch.security.support.HostAndCidrMatcher; import org.opensearch.security.user.AuthCredentials; public interface AuthFailureListener { - List getIgnoreHosts(); void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request); diff --git a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java index 2c9a7dfdd9..a266109c66 100644 --- a/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java +++ b/src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java @@ -50,11 +50,6 @@ public AbstractRateLimiter(Settings settings, Path configPath, Class getIgnoreHosts() { - return ignoreHosts; - } - @Override public abstract void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request); From 1ef938da3ce33028895da327f5b94b79fa6861d6 Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Mon, 3 Mar 2025 22:08:03 +0530 Subject: [PATCH 08/13] Use assertThat instead of assertTrue/assertFalse Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../support/HostAndCidrMatcherTest.java | 59 +++++++++---------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java index ec05e822ae..c976d40395 100644 --- a/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java +++ b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java @@ -6,8 +6,8 @@ import org.junit.Test; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; public class HostAndCidrMatcherTest { @@ -39,103 +39,103 @@ public void constructorShouldThrowExceptionForNullInput() { public void shouldReturnFalseForEmptyResolverMode() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); - assertFalse(matcher.matchesHostname(address, "")); + assertThat(matcher.matchesHostname(address, ""), is(false)); } @Test public void shouldReturnFalseForInvalidResolverMode() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); - assertFalse(matcher.matchesHostname(address, "invalid-mode")); + assertThat(matcher.matchesHostname(address, "invalid-mode"), is(false)); } @Test public void shouldReturnFalseForNullResolverMode() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); - assertFalse(matcher.matchesHostname(address, null)); + assertThat(matcher.matchesHostname(address, null), is(false)); } @Test public void shouldReturnFalseForWrongCaseResolverMode() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); - assertFalse(matcher.matchesHostname(address, "IP_HOSTNAME")); + assertThat(matcher.matchesHostname(address, "IP_HOSTNAME"), is(false)); } @Test public void shouldMatchIpv4WithinCidrRange() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR, PRIVATE_CLASS_A_CIDR)); InetAddress address = InetAddress.getByName(PRIVATE_CLASS_C_IP); - assertTrue(matcher.matchesCidr(address)); + assertThat(matcher.matchesCidr(address), is(true)); } @Test public void shouldNotMatchIpv4OutsideCidrRange() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR, PRIVATE_CLASS_A_CIDR)); InetAddress address = InetAddress.getByName("192.168.2.100"); - assertFalse(matcher.matchesCidr(address)); + assertThat(matcher.matchesCidr(address), is(false)); } @Test public void shouldMatchIpv6WithinCidrRange() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(IPV6_DOCUMENTATION_CIDR)); InetAddress address = InetAddress.getByName(IPV6_DOCUMENTATION_IP); - assertTrue(matcher.matchesCidr(address)); + assertThat(matcher.matchesCidr(address), is(true)); } @Test public void shouldHandleNullAddressInCidrMatching() { matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR)); - assertFalse(matcher.matchesCidr(null)); + assertThat(matcher.matchesCidr(null), is(false)); } @Test public void shouldMatchValidHostnamePattern() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN, OPENSEARCH_WWW)); InetAddress address = InetAddress.getByName(EXAMPLE_WWW); - assertTrue(matcher.matchesHostname(address, "ip-hostname")); + assertThat(matcher.matchesHostname(address, "ip-hostname"), is(true)); } @Test public void shouldHandleNullAddressInHostnameMatching() { matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN)); - assertFalse(matcher.matchesHostname(null, "ip-hostname")); + assertThat(matcher.matchesHostname(null, "ip-hostname"), is(false)); } @Test public void shouldMatchWhenIpMatchesCidrOnly() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN, PRIVATE_CLASS_C_CIDR)); InetAddress address = InetAddress.getByName(PRIVATE_CLASS_C_IP); - assertTrue(matcher.matches(address, "ip-hostname")); + assertThat(matcher.matches(address, "ip-hostname"), is(true)); } @Test public void shouldMatchWhenHostnameMatchesOnly() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN, PRIVATE_CLASS_C_CIDR)); InetAddress address = InetAddress.getByName(EXAMPLE_WWW); - assertTrue(matcher.matches(address, "ip-hostname")); + assertThat(matcher.matches(address, "ip-hostname"), is(true)); } @Test public void shouldNotMatchWhenNeitherMatches() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(EXAMPLE_DOMAIN, PRIVATE_CLASS_C_CIDR)); InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); - assertFalse(matcher.matches(address, "ip-hostname")); + assertThat(matcher.matches(address, "ip-hostname"), is(false)); } @Test public void shouldHandleEmptyPatternList() throws Exception { matcher = new HostAndCidrMatcher(Collections.emptyList()); InetAddress address = InetAddress.getByName(PRIVATE_CLASS_C_IP); - assertFalse(matcher.matches(address, "ip-hostname")); + assertThat(matcher.matches(address, "ip-hostname"), is(false)); } @Test public void shouldHandleInvalidCidrNotation() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList("invalid/cidr/notation")); InetAddress address = InetAddress.getByName(PRIVATE_CLASS_C_IP); - assertFalse(matcher.matchesCidr(address)); + assertThat(matcher.matchesCidr(address), is(false)); } @Test(expected = Exception.class) @@ -149,20 +149,19 @@ public void shouldHandleMalformedIpAddresses() throws Exception { public void shouldMatchIpHostnameLookupMode() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); InetAddress address = InetAddress.getByName(OPENSEARCH_WWW); - assertTrue(matcher.matchesHostname(address, "ip-hostname-lookup")); + assertThat(matcher.matchesHostname(address, "ip-hostname-lookup"), is(true)); } @Test public void shouldHandleMultipleCidrRanges() throws Exception { - matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_A_CIDR, PRIVATE_CLASS_B_CIDR, PRIVATE_CLASS_C_CIDR)); + matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR, PRIVATE_CLASS_A_CIDR, PRIVATE_CLASS_B_CIDR)); + InetAddress address1 = InetAddress.getByName(PRIVATE_CLASS_C_IP); + InetAddress address2 = InetAddress.getByName(PRIVATE_CLASS_A_IP); + InetAddress address3 = InetAddress.getByName(PRIVATE_CLASS_B_IP); - InetAddress address1 = InetAddress.getByName(PRIVATE_CLASS_A_IP); - InetAddress address2 = InetAddress.getByName(PRIVATE_CLASS_B_IP); - InetAddress address3 = InetAddress.getByName(PRIVATE_CLASS_C_IP); - - assertTrue(matcher.matchesCidr(address1)); - assertTrue(matcher.matchesCidr(address2)); - assertTrue(matcher.matchesCidr(address3)); + assertThat(matcher.matchesCidr(address1), is(true)); + assertThat(matcher.matchesCidr(address2), is(true)); + assertThat(matcher.matchesCidr(address3), is(true)); } @Test @@ -171,8 +170,8 @@ public void shouldHandleMultipleHostnamePatterns() throws Exception { InetAddress address1 = InetAddress.getByName(EXAMPLE_WWW); InetAddress address2 = InetAddress.getByName(OPENSEARCH_WWW); - assertTrue(matcher.matchesHostname(address1, "ip-hostname")); - assertTrue(matcher.matchesHostname(address2, "ip-hostname")); + assertThat(matcher.matchesHostname(address1, "ip-hostname"), is(true)); + assertThat(matcher.matchesHostname(address2, "ip-hostname"), is(true)); } @Test @@ -181,7 +180,7 @@ public void shouldHandleMixedIpv4AndIpv6Patterns() throws Exception { InetAddress ipv4Address = InetAddress.getByName(PRIVATE_CLASS_C_IP); InetAddress ipv6Address = InetAddress.getByName(IPV6_DOCUMENTATION_IP); - assertTrue(matcher.matchesCidr(ipv4Address)); - assertTrue(matcher.matchesCidr(ipv6Address)); + assertThat(matcher.matchesCidr(ipv4Address), is(true)); + assertThat(matcher.matchesCidr(ipv6Address), is(true)); } } From e5fbd78d9787e357d05c85db79b519a048568876 Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Mon, 3 Mar 2025 22:29:47 +0530 Subject: [PATCH 09/13] Always check host-pattern, add more tests Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../security/support/HostAndCidrMatcher.java | 25 ++++++++--------- .../support/HostAndCidrMatcherTest.java | 28 +++++++++++++++++++ 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java index 3e07b814f1..381f9edf9b 100644 --- a/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java +++ b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java @@ -71,23 +71,22 @@ public boolean matchesCidr(InetAddress address) { * @implNote This method may perform DNS lookups which could impact performance */ public boolean matchesHostname(InetAddress address, String hostResolverMode) { - if (address == null || hostMatcher == null || hostResolverMode == null) { + if (address == null || hostMatcher == null) { return false; } - if (!hostResolverMode.equalsIgnoreCase(IP_HOSTNAME) && !hostResolverMode.equalsIgnoreCase(IP_HOSTNAME_LOOKUP)) { - return false; - } - - try { - List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); - final String hostName = address.getHostName(); // potential blocking call - valuesToCheck.add(hostName); - return valuesToCheck.stream().anyMatch(hostMatcher); - } catch (Exception e) { - log.warn("Failed to resolve hostname for {}: {}", address.getHostAddress(), e.getMessage()); - return false; + List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); + if (hostResolverMode != null + && (hostResolverMode.equalsIgnoreCase(IP_HOSTNAME) || hostResolverMode.equalsIgnoreCase(IP_HOSTNAME_LOOKUP))) { + try { + final String hostName = address.getHostName(); // potential blocking call + valuesToCheck.add(hostName); + } catch (Exception e) { + log.warn("Failed to resolve hostname for {}: {}", address.getHostAddress(), e.getMessage()); + return false; + } } + return valuesToCheck.stream().anyMatch(hostMatcher); } /** diff --git a/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java index c976d40395..0973358117 100644 --- a/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java +++ b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java @@ -63,6 +63,34 @@ public void shouldReturnFalseForWrongCaseResolverMode() throws Exception { assertThat(matcher.matchesHostname(address, "IP_HOSTNAME"), is(false)); } + @Test + public void shouldReturnTrueForExactMatch() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList("127.0.0.1")); + InetAddress address = InetAddress.getByName("127.0.0.1"); + assertThat(matcher.matchesHostname(address, "ip-only"), is(true)); + } + + @Test + public void shouldReturnTrueForIpPatternMatch() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList("127.0.0.*")); + InetAddress address = InetAddress.getByName("127.0.0.1"); + assertThat(matcher.matchesHostname(address, "ip-only"), is(true)); + } + + @Test + public void shouldReturnFalseForHostMatchWithIpResolve() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList("127.0.0.1")); + InetAddress address = InetAddress.getByName("localhost"); + assertThat(matcher.matchesHostname(address, "ip-only"), is(true)); + } + + @Test + public void shouldReturnTrueForHostMatchWithIpResolve() throws Exception { + matcher = new HostAndCidrMatcher(Arrays.asList("127.0.0.1")); + InetAddress address = InetAddress.getByName("localhost"); + assertThat(matcher.matchesHostname(address, "ip-hostname"), is(true)); + } + @Test public void shouldMatchIpv4WithinCidrRange() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR, PRIVATE_CLASS_A_CIDR)); From df78c3094a3d604c426cf11ea7839267de6d4e94 Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Mon, 3 Mar 2025 22:32:33 +0530 Subject: [PATCH 10/13] Add license header to new files Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../opensearch/security/auth/AuthFailureListener.java | 1 - .../security/support/HostAndCidrMatcher.java | 11 +++++++++++ .../security/support/HostAndCidrMatcherTest.java | 11 +++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java index 297adbfafd..9f23391275 100644 --- a/src/main/java/org/opensearch/security/auth/AuthFailureListener.java +++ b/src/main/java/org/opensearch/security/auth/AuthFailureListener.java @@ -23,7 +23,6 @@ import org.opensearch.security.user.AuthCredentials; public interface AuthFailureListener { - void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request); HostAndCidrMatcher getIgnoreHostsMatcher(); diff --git a/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java index 381f9edf9b..099c462862 100644 --- a/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java +++ b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java @@ -1,3 +1,14 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + package org.opensearch.security.support; import java.net.InetAddress; diff --git a/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java index 0973358117..ac73b6c1a9 100644 --- a/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java +++ b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java @@ -1,3 +1,14 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + package org.opensearch.security.support; import java.net.InetAddress; From d15652fde9862e2732569ba227424a0e63189644 Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Thu, 6 Mar 2025 15:04:51 +0530 Subject: [PATCH 11/13] Adding HostResolverMode as enum, minor changes Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../security/securityconf/ConfigModelV7.java | 6 +++-- .../security/support/HostAndCidrMatcher.java | 11 ++++----- .../security/support/HostResolverMode.java | 16 +++++++++++++ .../support/HostAndCidrMatcherTest.java | 7 ------ .../support/HostResolverModeTest.java | 24 +++++++++++++++++++ 5 files changed, 48 insertions(+), 16 deletions(-) create mode 100644 src/main/java/org/opensearch/security/support/HostResolverMode.java create mode 100644 src/test/java/org/opensearch/security/support/HostResolverModeTest.java diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java index 31bf626c7b..9d6dafc857 100644 --- a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java @@ -52,6 +52,7 @@ import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.securityconf.impl.v7.TenantV7; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.support.HostResolverMode; import org.opensearch.security.support.WildcardMatcher; import org.opensearch.security.user.User; @@ -330,7 +331,8 @@ private Set map(final User user, final TransportAddress caller) { } if (caller.address() != null - && (hostResolverMode.equalsIgnoreCase("ip-hostname") || hostResolverMode.equalsIgnoreCase("ip-hostname-lookup"))) { + && (hostResolverMode.equalsIgnoreCase(HostResolverMode.IP_HOSTNAME.getValue()) + || hostResolverMode.equalsIgnoreCase(HostResolverMode.IP_HOSTNAME_LOOKUP.getValue()))) { final String hostName = caller.address().getHostString(); for (String p : WildcardMatcher.getAllMatchingPatterns(hostMatchers, hostName)) { @@ -338,7 +340,7 @@ private Set map(final User user, final TransportAddress caller) { } } - if (caller.address() != null && hostResolverMode.equalsIgnoreCase("ip-hostname-lookup")) { + if (caller.address() != null && hostResolverMode.equalsIgnoreCase(HostResolverMode.IP_HOSTNAME_LOOKUP.getValue())) { final String resolvedHostName = caller.address().getHostName(); diff --git a/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java index 099c462862..b1693237f9 100644 --- a/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java +++ b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java @@ -25,9 +25,6 @@ * This matcher supports both wildcard hostname patterns (e.g., *.example.com) and CIDR notation (e.g., 192.168.1.0/24). */ public class HostAndCidrMatcher { - private static final String IP_HOSTNAME = "ip-hostname"; - private static final String IP_HOSTNAME_LOOKUP = "ip-hostname-lookup"; - protected final Logger log = LogManager.getLogger(HostAndCidrMatcher.class); private final WildcardMatcher hostMatcher; private final List cidrMatchers; @@ -66,7 +63,7 @@ public boolean matchesCidr(InetAddress address) { return cidrMatchers.stream().anyMatch(cidrAddress -> cidrAddress.contains(addressString)); } catch (Exception e) { log.warn("Failed to process IP address {}: {}", address, e.getMessage()); - return false; + throw new RuntimeException("Invalid Address format used"); } } @@ -75,8 +72,7 @@ public boolean matchesCidr(InetAddress address) { * This method can perform DNS lookups depending on the hostResolverMode. * * @param address The IP address to check - * @param hostResolverMode The resolution mode. Must be either "ip-hostname" or - * "ip-hostname-lookup" to enable hostname matching + * @param hostResolverMode The resolution mode. Must be one of {@link HostResolverMode} to enable hostname matching * @return true if the address matches any configured hostname pattern, false otherwise, * if the address is null, or if the resolver mode is invalid * @implNote This method may perform DNS lookups which could impact performance @@ -88,7 +84,8 @@ public boolean matchesHostname(InetAddress address, String hostResolverMode) { List valuesToCheck = new ArrayList<>(List.of(address.getHostAddress())); if (hostResolverMode != null - && (hostResolverMode.equalsIgnoreCase(IP_HOSTNAME) || hostResolverMode.equalsIgnoreCase(IP_HOSTNAME_LOOKUP))) { + && (hostResolverMode.equalsIgnoreCase(HostResolverMode.IP_HOSTNAME.getValue()) + || hostResolverMode.equalsIgnoreCase(HostResolverMode.IP_HOSTNAME_LOOKUP.getValue()))) { try { final String hostName = address.getHostName(); // potential blocking call valuesToCheck.add(hostName); diff --git a/src/main/java/org/opensearch/security/support/HostResolverMode.java b/src/main/java/org/opensearch/security/support/HostResolverMode.java new file mode 100644 index 0000000000..c4a3ed9351 --- /dev/null +++ b/src/main/java/org/opensearch/security/support/HostResolverMode.java @@ -0,0 +1,16 @@ +package org.opensearch.security.support; + +public enum HostResolverMode { + IP_HOSTNAME("ip-hostname"), + IP_HOSTNAME_LOOKUP("ip-hostname-lookup"); + + private final String value; + + HostResolverMode(String value) { + this.value = value; + } + + public String getValue() { + return value; + } +} diff --git a/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java index ac73b6c1a9..d351c532b0 100644 --- a/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java +++ b/src/test/java/org/opensearch/security/support/HostAndCidrMatcherTest.java @@ -177,13 +177,6 @@ public void shouldHandleInvalidCidrNotation() throws Exception { assertThat(matcher.matchesCidr(address), is(false)); } - @Test(expected = Exception.class) - public void shouldHandleMalformedIpAddresses() throws Exception { - matcher = new HostAndCidrMatcher(Arrays.asList(PRIVATE_CLASS_C_CIDR)); - InetAddress address = InetAddress.getByName("invalid.ip.address"); - matcher.matchesCidr(address); - } - @Test public void shouldMatchIpHostnameLookupMode() throws Exception { matcher = new HostAndCidrMatcher(Arrays.asList(OPENSEARCH_DOMAIN)); diff --git a/src/test/java/org/opensearch/security/support/HostResolverModeTest.java b/src/test/java/org/opensearch/security/support/HostResolverModeTest.java new file mode 100644 index 0000000000..aee86bd216 --- /dev/null +++ b/src/test/java/org/opensearch/security/support/HostResolverModeTest.java @@ -0,0 +1,24 @@ +package org.opensearch.security.support; + +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +public class HostResolverModeTest { + + @Test + public void testIpHostnameValue() { + assertThat(HostResolverMode.IP_HOSTNAME.getValue(), is("ip-hostname")); + } + + @Test + public void testIpHostnameLookupValue() { + assertThat(HostResolverMode.IP_HOSTNAME_LOOKUP.getValue(), is("ip-hostname-lookup")); + } + + @Test + public void testEnumCount() { + assertThat(HostResolverMode.values().length, is(2)); + } +} From 2dfc3f4834965afc0001b0a770030a9d61b8cb0c Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Thu, 6 Mar 2025 15:13:00 +0530 Subject: [PATCH 12/13] Adding license header to new files Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../opensearch/security/support/HostResolverMode.java | 11 +++++++++++ .../security/support/HostResolverModeTest.java | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/main/java/org/opensearch/security/support/HostResolverMode.java b/src/main/java/org/opensearch/security/support/HostResolverMode.java index c4a3ed9351..00ce6e9117 100644 --- a/src/main/java/org/opensearch/security/support/HostResolverMode.java +++ b/src/main/java/org/opensearch/security/support/HostResolverMode.java @@ -1,3 +1,14 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + package org.opensearch.security.support; public enum HostResolverMode { diff --git a/src/test/java/org/opensearch/security/support/HostResolverModeTest.java b/src/test/java/org/opensearch/security/support/HostResolverModeTest.java index aee86bd216..bee18e5242 100644 --- a/src/test/java/org/opensearch/security/support/HostResolverModeTest.java +++ b/src/test/java/org/opensearch/security/support/HostResolverModeTest.java @@ -1,3 +1,14 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + package org.opensearch.security.support; import org.junit.Test; From e8ff312f5ac5129e23dfbb288c4f31e70807acb8 Mon Sep 17 00:00:00 2001 From: shikharj05 <8859327+shikharj05@users.noreply.github.com> Date: Thu, 6 Mar 2025 15:24:56 +0530 Subject: [PATCH 13/13] Throw original exception in HostAndCidrMatcher.matchesCidr Signed-off-by: shikharj05 <8859327+shikharj05@users.noreply.github.com> --- .../opensearch/security/support/HostAndCidrMatcher.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java index b1693237f9..caffa6b871 100644 --- a/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java +++ b/src/main/java/org/opensearch/security/support/HostAndCidrMatcher.java @@ -58,13 +58,8 @@ public boolean matchesCidr(InetAddress address) { return false; } - try { - IPAddressString addressString = new IPAddressString(address.getHostAddress()); - return cidrMatchers.stream().anyMatch(cidrAddress -> cidrAddress.contains(addressString)); - } catch (Exception e) { - log.warn("Failed to process IP address {}: {}", address, e.getMessage()); - throw new RuntimeException("Invalid Address format used"); - } + IPAddressString addressString = new IPAddressString(address.getHostAddress()); + return cidrMatchers.stream().anyMatch(cidrAddress -> cidrAddress.contains(addressString)); } /**