Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CIDR ranges in ignore_hosts setting. #5099

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+364 −105
Open

Add support for CIDR ranges in ignore_hosts setting. #5099

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9b598de
Add CIDR support in ignore_hosts setting
shikharj05 Feb 6, 2025
6603a89
Remove commented method/code
shikharj05 Feb 6, 2025
8aca1e6
Move IP matching to cidr matcher
shikharj05 Feb 10, 2025
6efe00b
Remove cache from AbstractRateLimiter, replace parallelStreams with s…
shikharj05 Feb 10, 2025
ac5cd05
Pre-compute subnetUtils instances
shikharj05 Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/org/opensearch/security/auth/AuthFailureListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> getIgnoreHosts();
shikharj05 marked this conversation as resolved.
Show resolved Hide resolved

void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request);

WildcardMatcher getIgnoreHostsMatcher();

SubnetInfo getSubnetForCidr(String cidr);
}
30 changes: 13 additions & 17 deletions src/main/java/org/opensearch/security/auth/BackendRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.matchesHostNamePatterns;
import static org.opensearch.security.support.SecurityUtils.matchesIpAndCidrPatterns;
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<AuthDomain> restAuthDomains;
private Set<AuthorizationBackend> restAuthorizers;

Expand Down Expand Up @@ -696,8 +697,7 @@ private boolean isBlocked(InetAddress address) {
}

for (ClientBlockRegistry<InetAddress> clientBlockRegistry : ipClientBlockRegistries) {
WildcardMatcher ignoreHostsMatcher = ((AuthFailureListener) clientBlockRegistry).getIgnoreHostsMatcher();
if (matchesHostPatterns(ignoreHostsMatcher, address, hostResolverMode)) {
if (matchesIgnoreHostPatterns(clientBlockRegistry, address, hostResolverMode)) {
return false;
}
if (clientBlockRegistry.isBlocked(address)) {
Expand All @@ -708,21 +708,17 @@ 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<InetAddress> clientBlockRegistry,
InetAddress address,
String hostResolverMode
) {
WildcardMatcher ignoreHostsMatcher = ((AuthFailureListener) clientBlockRegistry).getIgnoreHostsMatcher();
if (ignoreHostsMatcher == null || address == null) {
return false;
}
if (address != null) {
List<String> 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 matchesHostNamePatterns(ignoreHostsMatcher, address, hostResolverMode)
|| matchesIpAndCidrPatterns(clientBlockRegistry, address);
}

private boolean isBlocked(String authBackend, String userName) {
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.Collections;
import java.util.List;

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;
Expand Down Expand Up @@ -50,6 +52,11 @@ public AbstractRateLimiter(Settings settings, Path configPath, Class<ClientIdTyp
);
}

@Override
public List<String> getIgnoreHosts() {
return ignoreHosts;
}

@Override
public abstract void onAuthFailure(InetAddress remoteAddress, AuthCredentials authCredentials, Object request);

Expand All @@ -66,6 +73,14 @@ 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);
Expand Down
57 changes: 57 additions & 0 deletions src/main/java/org/opensearch/security/support/SecurityUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -46,6 +54,7 @@
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<String, SubnetInfo> cidrCache = new ConcurrentHashMap<>();
shikharj05 marked this conversation as resolved.
Show resolved Hide resolved

private SecurityUtils() {}

Expand Down Expand Up @@ -140,4 +149,52 @@
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<String> 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;

Check warning on line 169 in src/main/java/org/opensearch/security/support/SecurityUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/support/SecurityUtils.java#L167-L169 

Added lines #L167 - L169 were not covered by tests
}
}
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<InetAddress> clientBlockRegistry, InetAddress address) {
if (address == null || clientBlockRegistry == null) {
return false;
}

AuthFailureListener authFailureListener = (AuthFailureListener) clientBlockRegistry;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: As this method only works for AuthFailureListener instances, it should be probably an instance method of AuthFailureListener.

Alternatively: Have you considered to create a new class that encapsulates a combination of WildcardMatcher and CIDR matching?

This combination of matching is something what is not only useful for the ignore_hosts setting, but for quite a few functionalities. Having a reusable component for that would be nice.

final String hostAddress = address.getHostAddress();

return authFailureListener.getIgnoreHosts().stream().anyMatch(pattern -> {
// Handle CIDR patterns
if (pattern.indexOf('/') != -1) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think that using / is sufficient for identifying CIDRs. The ignore_hosts property supports the whole wildcard matcher feature set which also supports specifying regexes using the /regex/ syntax. This would also hit on these regexes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch. one of the options is to implement regex check for ipv4/ipv6 addresses to identify CIDRs specifically - will check other options and update the PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also keep it simple and get the last index of / as long as / is not the final char in the string

try {
SubnetInfo subnetInfo = cidrCache.computeIfAbsent(pattern, authFailureListener::getSubnetForCidr);
return subnetInfo.isInRange(hostAddress);
Copy link
Collaborator

@nibix nibix Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I see it correctly, SubnetUtils and SubnetInfo only support IPv4. If hostAddress is in IPv6 format, it will throw an IllegalArgumentException:

https://github.com/apache/commons-net/blob/aeaa37e9753ce9920c91d22493e074781ef69ff3/src/main/java/org/apache/commons/net/util/SubnetUtils.java#L395C19-L395C43

I guess, we also need to support IPv6?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will add support for ipv6.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a recommendation: I have used in other projects this library:

https://seancfoley.github.io/IPAddress/

It supports both IPv4 and IPv6, is super flexible and has a clean API.

} 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);
});
}
}
88 changes: 0 additions & 88 deletions src/test/java/org/opensearch/security/UtilTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
);
}
}
Loading
Loading