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.

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
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
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
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);
}
29 changes: 12 additions & 17 deletions src/main/java/org/opensearch/security/auth/BackendRegistry.java
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.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<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,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<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 matchesHostPatterns(ignoreHostsMatcher, address, hostResolverMode) || matchesCidrPatterns(clientBlockRegistry, address);
}

private boolean isBlocked(String authBackend, String userName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,6 +39,8 @@ public abstract class AbstractRateLimiter<ClientIdType> implements AuthFailureLi
protected final RateTracker<ClientIdType> rateTracker;
protected final List<String> ignoreHosts;
private WildcardMatcher ignoreHostMatcher;
// Cache for subnet matchers
private static final Map<String, SubnetUtils.SubnetInfo> SUBNET_CACHE = new ConcurrentHashMap<>();
cwperks marked this conversation as resolved.
Show resolved Hide resolved

public AbstractRateLimiter(Settings settings, Path configPath, Class<ClientIdType> clientIdType) {
this.ignoreHosts = settings.getAsList("ignore_hosts", Collections.emptyList());
Expand All @@ -50,6 +56,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 +77,16 @@ public WildcardMatcher getIgnoreHostsMatcher() {
return hostMatcher;
}

@Override
public SubnetUtils.SubnetInfo getSubnetForCidr(String cidr) {
return SUBNET_CACHE.computeIfAbsent(cidr, pattern -> {
shikharj05 marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand Down
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 @@ 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<String, SubnetInfo> cidrCache = new ConcurrentHashMap<>();
shikharj05 marked this conversation as resolved.
Show resolved Hide resolved

private SecurityUtils() {}

Expand Down Expand Up @@ -140,4 +149,35 @@ 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<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);
}

public static boolean matchesCidrPatterns(ClientBlockRegistry<InetAddress> clientBlockRegistry, InetAddress address) {
if (clientBlockRegistry == null || address == 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.

String hostAddress = address.getHostAddress();
return authFailureListener.getIgnoreHosts().parallelStream().filter(pattern -> pattern.indexOf('/') != -1).anyMatch(pattern -> {
shikharj05 marked this conversation as resolved.
Show resolved Hide resolved
try {
SubnetInfo subnetInfo = cidrCache.computeIfAbsent(pattern, authFailureListener::getSubnetForCidr);
cwperks marked this conversation as resolved.
Show resolved Hide resolved
return subnetInfo.isInRange(hostAddress);
} catch (IllegalArgumentException e) {
log.warn("Invalid CIDR pattern: {}", pattern);
return false;
}
});
}
}
88 changes: 0 additions & 88 deletions src/test/java/org/opensearch/security/UtilTests.java
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