From 6a7462be3e0c78fde57b859e62784b1cb38bc558 Mon Sep 17 00:00:00 2001 From: jaymode Date: Wed, 16 Sep 2015 12:44:13 -0400 Subject: [PATCH] update the IPFilter to always allow traffic from a bound address This change updates the IPFilter to always allow traffic from the bound addresses of the node even if they have been explicitly disabled. This behavior can be disabled through a setting but that could be dangerous if the blocking rule is added via a persistent setting stored in the cluster state. Closes elastic/elasticsearch#487 Original commit: elastic/x-pack-elasticsearch@4c1cf9455fe13d75c63aa930d5b25515d7eb5dcf --- .../agent/exporter/HttpESExporterUtils.java | 33 ++++++--- .../shield/transport/filter/IPFilter.java | 64 ++++++++--------- .../transport/filter/ShieldIpFilterRule.java | 60 ++++++++++++++-- .../audit/index/IndexAuditTrailTests.java | 7 +- .../IndexAuditTrailUpdateMappingTests.java | 3 +- .../audit/logfile/LoggingAuditTrailTests.java | 2 +- .../shield/authc/AnonymousUserTests.java | 2 +- .../authc/pki/PkiAuthenticationTests.java | 6 +- .../authc/pki/PkiOptionalClientAuthTests.java | 2 +- .../transport/filter/IPFilterTests.java | 34 ++++++--- .../filter/IpFilteringIntegrationTests.java | 4 +- .../filter/IpFilteringUpdateTests.java | 16 ++++- .../filter/ShieldIpFilterRuleTests.java | 72 +++++++++++++++++++ .../IPFilterNettyUpstreamHandlerTests.java | 5 +- .../transport/ssl/SslIntegrationTests.java | 6 +- .../transport/ssl/SslMultiPortTests.java | 10 +-- 16 files changed, 241 insertions(+), 85 deletions(-) create mode 100644 shield/src/test/java/org/elasticsearch/shield/transport/filter/ShieldIpFilterRuleTests.java diff --git a/marvel/src/main/java/org/elasticsearch/marvel/agent/exporter/HttpESExporterUtils.java b/marvel/src/main/java/org/elasticsearch/marvel/agent/exporter/HttpESExporterUtils.java index f1db07afb3c..29d428673ac 100644 --- a/marvel/src/main/java/org/elasticsearch/marvel/agent/exporter/HttpESExporterUtils.java +++ b/marvel/src/main/java/org/elasticsearch/marvel/agent/exporter/HttpESExporterUtils.java @@ -12,12 +12,15 @@ import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.InetSocketTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.net.*; import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -28,24 +31,34 @@ public class HttpESExporterUtils { static final String VERSION_FIELD = "number"; public static String[] extractHostsFromAddress(BoundTransportAddress boundAddress, ESLogger logger) { - if (boundAddress == null || boundAddress.boundAddress() == null) { + if (boundAddress == null || boundAddress.boundAddresses() == null) { logger.debug("local http server is not yet started. can't connect"); return null; } - if (boundAddress.boundAddress().uniqueAddressTypeId() != 1) { - logger.error("local node is not bound via the http transport. can't connect"); - return null; + TransportAddress[] boundAddresses = boundAddress.boundAddresses(); + List hosts = new ArrayList<>(boundAddresses.length); + for (TransportAddress transportAddress : boundAddresses) { + if (transportAddress.uniqueAddressTypeId() == 1) { + InetSocketTransportAddress address = (InetSocketTransportAddress) transportAddress; + InetSocketAddress inetSocketAddress = address.address(); + InetAddress inetAddress = inetSocketAddress.getAddress(); + if (inetAddress == null) { + logger.error("failed to extract the ip address of from transport address [{}]", transportAddress); + continue; + } + hosts.add(NetworkAddress.formatAddress(inetSocketAddress)); + } else { + logger.error("local node http transport is not bound via a InetSocketTransportAddress. address is [{}] with typeId [{}]", transportAddress, transportAddress.uniqueAddressTypeId()); + } } - InetSocketTransportAddress address = (InetSocketTransportAddress) boundAddress.boundAddress(); - InetSocketAddress inetSocketAddress = address.address(); - InetAddress inetAddress = inetSocketAddress.getAddress(); - if (inetAddress == null) { - logger.error("failed to extract the ip address of current node."); + + if (hosts.isEmpty()) { + logger.error("could not extract any hosts from bound address. can't connect"); return null; } - return new String[]{ NetworkAddress.formatAddress(inetSocketAddress) }; + return hosts.toArray(new String[hosts.size()]); } public static URL parseHostWithPath(String host, String path) throws URISyntaxException, MalformedURLException { diff --git a/shield/src/main/java/org/elasticsearch/shield/transport/filter/IPFilter.java b/shield/src/main/java/org/elasticsearch/shield/transport/filter/IPFilter.java index f7a72f3f1e5..5af4cf576c2 100644 --- a/shield/src/main/java/org/elasticsearch/shield/transport/filter/IPFilter.java +++ b/shield/src/main/java/org/elasticsearch/shield/transport/filter/IPFilter.java @@ -15,8 +15,8 @@ import org.elasticsearch.common.component.LifecycleListener; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.inject.internal.Nullable; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.transport.InetSocketTransportAddress; -import org.elasticsearch.common.util.ArrayUtils; +import org.elasticsearch.common.transport.BoundTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.node.settings.NodeSettingsService; import org.elasticsearch.shield.audit.AuditTrail; @@ -66,6 +66,7 @@ public class IPFilter extends AbstractLifecycleComponent { private NodeSettingsService nodeSettingsService; private final AuditTrail auditTrail; private final Transport transport; + private final boolean alwaysAllowBoundAddresses; private Map rules = Collections.EMPTY_MAP; private HttpServerTransport httpServerTransport = null; @@ -75,6 +76,7 @@ public class IPFilter extends AbstractLifecycleComponent { this.nodeSettingsService = nodeSettingsService; this.auditTrail = auditTrail; this.transport = transport; + this.alwaysAllowBoundAddresses = settings.getAsBoolean("shield.filter.always_allow_bound_address", true); } @Override @@ -144,27 +146,30 @@ public class IPFilter extends AbstractLifecycleComponent { Map profileRules = new HashMap<>(); if (isHttpFilterEnabled && httpServerTransport != null && httpServerTransport.lifecycleState() == Lifecycle.State.STARTED) { - InetAddress localAddress = ((InetSocketTransportAddress) this.httpServerTransport.boundAddress().boundAddress()).address().getAddress(); + TransportAddress[] localAddresses = this.httpServerTransport.boundAddress().boundAddresses(); String[] httpAllowed = settings.getAsArray("shield.http.filter.allow", settings.getAsArray("transport.profiles.default.shield.filter.allow", settings.getAsArray("shield.transport.filter.allow"))); - String[] httpDdenied = settings.getAsArray("shield.http.filter.deny", settings.getAsArray("transport.profiles.default.shield.filter.deny", settings.getAsArray("shield.transport.filter.deny"))); - profileRules.put(HTTP_PROFILE_NAME, ArrayUtils.concat(parseValue(httpAllowed, true, localAddress), parseValue(httpDdenied, false, localAddress), ShieldIpFilterRule.class)); + String[] httpDenied = settings.getAsArray("shield.http.filter.deny", settings.getAsArray("transport.profiles.default.shield.filter.deny", settings.getAsArray("shield.transport.filter.deny"))); + profileRules.put(HTTP_PROFILE_NAME, createRules(httpAllowed, httpDenied, localAddresses)); } if (isIpFilterEnabled && this.transport.lifecycleState() == Lifecycle.State.STARTED) { - InetAddress localAddress = ((InetSocketTransportAddress) this.transport.boundAddress().boundAddress()).address().getAddress(); + TransportAddress[] localAddresses = this.transport.boundAddress().boundAddresses(); String[] allowed = settings.getAsArray("shield.transport.filter.allow"); String[] denied = settings.getAsArray("shield.transport.filter.deny"); - profileRules.put("default", ArrayUtils.concat(parseValue(allowed, true, localAddress), parseValue(denied, false, localAddress), ShieldIpFilterRule.class)); + profileRules.put("default", createRules(allowed, denied, localAddresses)); Map groupedSettings = settings.getGroups("transport.profiles."); for (Map.Entry entry : groupedSettings.entrySet()) { String profile = entry.getKey(); + BoundTransportAddress profileBoundTransportAddress = transport.profileBoundAddresses().get(profile); + if (profileBoundTransportAddress == null) { + // this could happen if a user updates the settings dynamically with a new profile + logger.warn("skipping ip filter rules for profile [{}] since the profile is not bound to any addresses", profile); + continue; + } Settings profileSettings = entry.getValue().getByPrefix("shield.filter."); - profileRules.put(profile, ArrayUtils.concat( - parseValue(profileSettings.getAsArray("allow"), true, localAddress), - parseValue(profileSettings.getAsArray("deny"), false, localAddress), - ShieldIpFilterRule.class)); + profileRules.put(profile, createRules(profileSettings.getAsArray("allow"), profileSettings.getAsArray("deny"), profileBoundTransportAddress.boundAddresses())); } } @@ -172,32 +177,23 @@ public class IPFilter extends AbstractLifecycleComponent { return ImmutableMap.copyOf(profileRules); } - private ShieldIpFilterRule[] parseValue(String[] values, boolean isAllowRule, InetAddress localAddress) { + private ShieldIpFilterRule[] createRules(String[] allow, String[] deny, TransportAddress[] boundAddresses) { List rules = new ArrayList<>(); - for (int i = 0; i < values.length; i++) { - - // never ever deny on localhost, do not even add this rule - if (!isAllowRule && isLocalAddress(localAddress, values[i])) { - logger.warn("Configuration setting not applied to reject connections on [{}]. local connections are always allowed!", values[i]); - continue; - } - - rules.add(new ShieldIpFilterRule(isAllowRule, values[i])); + // if we are always going to allow the bound addresses, then the rule for them should be the first rule in the list + if (alwaysAllowBoundAddresses) { + assert boundAddresses != null && boundAddresses.length > 0; + rules.add(new ShieldIpFilterRule(true, boundAddresses)); } - return rules.toArray(new ShieldIpFilterRule[]{}); - } - /** - * Checks if a user provided address is the same address that we are bound to. This is to prevent denying - * connections from the machine we are running on - * - * @param localAddress the InetAddress that this node is bound to. This should come from the transport - * @param address the address that is being evaluated to be blocked - * @return true if the address is not the same as the localAddress - */ - private boolean isLocalAddress(InetAddress localAddress, String address) { - // FIXME add the correct behavior, see https://github.com/elastic/x-plugins/issues/487 - return false; + // add all rules to the same list. Allow takes precedence so they must come first! + for (String value : allow) { + rules.add(new ShieldIpFilterRule(true, value)); + } + for (String value : deny) { + rules.add(new ShieldIpFilterRule(false, value)); + } + + return rules.toArray(new ShieldIpFilterRule[rules.size()]); } private class ApplySettings implements NodeSettingsService.Listener { diff --git a/shield/src/main/java/org/elasticsearch/shield/transport/filter/ShieldIpFilterRule.java b/shield/src/main/java/org/elasticsearch/shield/transport/filter/ShieldIpFilterRule.java index 3cefd84fb6e..652d6bff274 100644 --- a/shield/src/main/java/org/elasticsearch/shield/transport/filter/ShieldIpFilterRule.java +++ b/shield/src/main/java/org/elasticsearch/shield/transport/filter/ShieldIpFilterRule.java @@ -7,12 +7,16 @@ package org.elasticsearch.shield.transport.filter; import com.google.common.net.InetAddresses; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.common.network.NetworkAddress; +import org.elasticsearch.common.transport.InetSocketTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; import org.jboss.netty.handler.ipfilter.IpFilterRule; import org.jboss.netty.handler.ipfilter.IpSubnetFilterRule; import org.jboss.netty.handler.ipfilter.PatternRule; import java.net.InetAddress; import java.net.UnknownHostException; +import java.util.Arrays; /** * decorator class to have a useful toString() method for an IpFilterRule @@ -38,6 +42,7 @@ public class ShieldIpFilterRule implements IpFilterRule { }; public static final ShieldIpFilterRule DENY_ALL = new ShieldIpFilterRule(true, "deny_all") { + @Override public boolean contains(InetAddress inetAddress) { return true; @@ -62,6 +67,11 @@ public class ShieldIpFilterRule implements IpFilterRule { this.ruleSpec = ruleSpec; } + ShieldIpFilterRule(boolean isAllowRule, TransportAddress... addresses) { + this.ruleSpec = getRuleSpec(addresses); + this.ipFilterRule = getRule(isAllowRule, ruleSpec); + } + @Override public boolean contains(InetAddress inetAddress) { return ipFilterRule.contains(inetAddress); @@ -90,12 +100,21 @@ public class ShieldIpFilterRule implements IpFilterRule { return builder.toString(); } - private static IpFilterRule getRule(boolean isAllowRule, String value) { - if ("_all".equals(value)) { + static IpFilterRule getRule(boolean isAllowRule, String value) { + String[] values = value.split(","); + int allRuleIndex = Arrays.binarySearch(values, 0, values.length, "_all"); + if (allRuleIndex >= 0) { + // all rule was found. It should be the only rule! + if (values.length != 1) { + throw new IllegalArgumentException("rules that specify _all may not have other values!"); + } return isAllowRule ? ACCEPT_ALL : DENY_ALL; } if (value.contains("/")) { + if (values.length != 1) { + throw new IllegalArgumentException("multiple subnet filters cannot be specified in a single rule!"); + } try { return new IpSubnetFilterRule(isAllowRule, value); } catch (UnknownHostException e) { @@ -103,9 +122,40 @@ public class ShieldIpFilterRule implements IpFilterRule { } } - boolean isInetAddress = InetAddresses.isInetAddress(value); - String prefix = isInetAddress ? "i:" : "n:"; - return new PatternRule(isAllowRule, prefix + value); + boolean firstAdded = false; + StringBuilder ruleSpec = new StringBuilder(); + for (String singleValue : values) { + if (firstAdded) { + ruleSpec.append(","); + } else { + firstAdded = true; + } + + boolean isInetAddress = InetAddresses.isInetAddress(singleValue); + if (isInetAddress) { + ruleSpec.append("i:"); + } else { + ruleSpec.append("n:"); + } + ruleSpec.append(singleValue); + } + + return new PatternRule(isAllowRule, ruleSpec.toString()); } + static String getRuleSpec(TransportAddress... addresses) { + StringBuilder ruleSpec = new StringBuilder(); + boolean firstAdded = false; + for (TransportAddress transportAddress : addresses) { + if (firstAdded) { + ruleSpec.append(","); + } else { + firstAdded = true; + } + + assert transportAddress instanceof InetSocketTransportAddress; + ruleSpec.append(NetworkAddress.formatAddress(((InetSocketTransportAddress) transportAddress).address().getAddress())); + } + return ruleSpec.toString(); + } } diff --git a/shield/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailTests.java b/shield/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailTests.java index 70409ada554..b4340c97e6c 100644 --- a/shield/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailTests.java @@ -16,10 +16,7 @@ import org.elasticsearch.cluster.ClusterService; import org.elasticsearch.common.inject.util.Providers; import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.transport.BoundTransportAddress; -import org.elasticsearch.common.transport.DummyTransportAddress; -import org.elasticsearch.common.transport.InetSocketTransportAddress; -import org.elasticsearch.common.transport.LocalTransportAddress; +import org.elasticsearch.common.transport.*; import org.elasticsearch.env.Environment; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.cache.IndexCacheModule; @@ -197,7 +194,7 @@ public class IndexAuditTrailTests extends ShieldIntegTestCase { when(authService.authenticate(mock(RestRequest.class))).thenThrow(new UnsupportedOperationException("")); when(authService.authenticate("_action", new LocalHostMockMessage(), user.user())).thenThrow(new UnsupportedOperationException("")); Transport transport = mock(Transport.class); - when(transport.boundAddress()).thenReturn(new BoundTransportAddress(DummyTransportAddress.INSTANCE, DummyTransportAddress.INSTANCE)); + when(transport.boundAddress()).thenReturn(new BoundTransportAddress(new TransportAddress[] { DummyTransportAddress.INSTANCE }, DummyTransportAddress.INSTANCE)); Environment env = new Environment(settings); threadPool = new ThreadPool("index audit trail tests"); diff --git a/shield/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailUpdateMappingTests.java b/shield/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailUpdateMappingTests.java index 17fc07d1a1c..f23d35f74d1 100644 --- a/shield/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailUpdateMappingTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailUpdateMappingTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.common.inject.util.Providers; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.DummyTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.env.Environment; import org.elasticsearch.shield.authc.AuthenticationService; import org.elasticsearch.test.*; @@ -48,7 +49,7 @@ public class IndexAuditTrailUpdateMappingTests extends ShieldIntegTestCase { AuthenticationService authService = mock(AuthenticationService.class); Settings settings = Settings.builder().put("shield.audit.index.rollover", rollover.name().toLowerCase(Locale.ENGLISH)).put("path.home", createTempDir()).build(); Transport transport = mock(Transport.class); - when(transport.boundAddress()).thenReturn(new BoundTransportAddress(DummyTransportAddress.INSTANCE, DummyTransportAddress.INSTANCE)); + when(transport.boundAddress()).thenReturn(new BoundTransportAddress(new TransportAddress[] { DummyTransportAddress.INSTANCE }, DummyTransportAddress.INSTANCE)); Environment env = new Environment(settings); IndexAuditTrail auditor = new IndexAuditTrail(settings, new IndexAuditUserHolder(), env, authService, transport, Providers.of(client()), threadPool, mock(ClusterService.class)); diff --git a/shield/src/test/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrailTests.java b/shield/src/test/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrailTests.java index 471b3b6985f..f425ef5cd37 100644 --- a/shield/src/test/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrailTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrailTests.java @@ -109,7 +109,7 @@ public class LoggingAuditTrailTests extends ESTestCase { .put("shield.audit.logfile.prefix.emit_node_name", randomBoolean()) .build(); transport = mock(Transport.class); - when(transport.boundAddress()).thenReturn(new BoundTransportAddress(DummyTransportAddress.INSTANCE, DummyTransportAddress.INSTANCE)); + when(transport.boundAddress()).thenReturn(new BoundTransportAddress(new TransportAddress[] { DummyTransportAddress.INSTANCE }, DummyTransportAddress.INSTANCE)); prefix = LoggingAuditTrail.resolvePrefix(settings, transport); } diff --git a/shield/src/test/java/org/elasticsearch/shield/authc/AnonymousUserTests.java b/shield/src/test/java/org/elasticsearch/shield/authc/AnonymousUserTests.java index 3bafddac63c..9d5aca55aff 100644 --- a/shield/src/test/java/org/elasticsearch/shield/authc/AnonymousUserTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/authc/AnonymousUserTests.java @@ -71,7 +71,7 @@ public class AnonymousUserTests extends ShieldIntegTestCase { } private String getNodeUrl() { - TransportAddress transportAddress = internalCluster().getInstance(HttpServerTransport.class).boundAddress().boundAddress(); + TransportAddress transportAddress = randomFrom(internalCluster().getInstance(HttpServerTransport.class).boundAddress().boundAddresses()); assertThat(transportAddress, is(instanceOf(InetSocketTransportAddress.class))); InetSocketTransportAddress inetSocketTransportAddress = (InetSocketTransportAddress) transportAddress; return String.format(Locale.ROOT, "http://%s:%s/", "localhost", inetSocketTransportAddress.address().getPort()); diff --git a/shield/src/test/java/org/elasticsearch/shield/authc/pki/PkiAuthenticationTests.java b/shield/src/test/java/org/elasticsearch/shield/authc/pki/PkiAuthenticationTests.java index 5fa1bd850cc..bcafc2b12e0 100644 --- a/shield/src/test/java/org/elasticsearch/shield/authc/pki/PkiAuthenticationTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/authc/pki/PkiAuthenticationTests.java @@ -71,7 +71,7 @@ public class PkiAuthenticationTests extends ShieldIntegTestCase { public void testTransportClientCanAuthenticateViaPki() { Settings settings = ShieldSettingsSource.getSSLSettingsForStore("/org/elasticsearch/shield/transport/ssl/certs/simple/testnode.jks", "testnode"); try (TransportClient client = createTransportClient(settings)) { - client.addTransportAddress(internalCluster().getInstance(Transport.class).boundAddress().boundAddress()); + client.addTransportAddress(randomFrom(internalCluster().getInstance(Transport.class).boundAddress().boundAddresses())); IndexResponse response = client.prepareIndex("foo", "bar").setSource("pki", "auth").get(); assertThat(response.isCreated(), is(true)); } @@ -84,7 +84,7 @@ public class PkiAuthenticationTests extends ShieldIntegTestCase { @Test(expected = NoNodeAvailableException.class) public void testTransportClientAuthenticationFailure() { try (TransportClient client = createTransportClient(Settings.EMPTY)) { - client.addTransportAddress(internalCluster().getInstance(Transport.class).boundAddress().boundAddress()); + client.addTransportAddress(randomFrom(internalCluster().getInstance(Transport.class).boundAddress().boundAddresses())); client.prepareIndex("foo", "bar").setSource("pki", "auth").get(); fail("transport client should not have been able to authenticate"); } @@ -144,7 +144,7 @@ public class PkiAuthenticationTests extends ShieldIntegTestCase { } private String getNodeUrl() { - TransportAddress transportAddress = internalCluster().getInstance(HttpServerTransport.class).boundAddress().boundAddress(); + TransportAddress transportAddress = randomFrom(internalCluster().getInstance(HttpServerTransport.class).boundAddress().boundAddresses()); assertThat(transportAddress, is(instanceOf(InetSocketTransportAddress.class))); InetSocketTransportAddress inetSocketTransportAddress = (InetSocketTransportAddress) transportAddress; return String.format(Locale.ROOT, "https://localhost:%s/", inetSocketTransportAddress.address().getPort()); diff --git a/shield/src/test/java/org/elasticsearch/shield/authc/pki/PkiOptionalClientAuthTests.java b/shield/src/test/java/org/elasticsearch/shield/authc/pki/PkiOptionalClientAuthTests.java index e6deeaadeb8..e884216fa63 100644 --- a/shield/src/test/java/org/elasticsearch/shield/authc/pki/PkiOptionalClientAuthTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/authc/pki/PkiOptionalClientAuthTests.java @@ -97,7 +97,7 @@ public class PkiOptionalClientAuthTests extends ShieldIntegTestCase { @Test public void testTransportClientWithoutClientCertificate() { Transport transport = internalCluster().getDataNodeInstance(Transport.class); - int port = ((InetSocketTransportAddress)transport.profileBoundAddresses().get("want_client_auth").boundAddress()).address().getPort(); + int port = ((InetSocketTransportAddress) randomFrom(transport.profileBoundAddresses().get("want_client_auth").boundAddresses())).address().getPort(); Settings settings = Settings.builder() .put(ShieldSettingsSource.getSSLSettingsForStore("/org/elasticsearch/shield/transport/ssl/certs/simple/truststore-testnode-only.jks", "truststore-testnode-only")) diff --git a/shield/src/test/java/org/elasticsearch/shield/transport/filter/IPFilterTests.java b/shield/src/test/java/org/elasticsearch/shield/transport/filter/IPFilterTests.java index 068fb8fc574..41c88b13114 100644 --- a/shield/src/test/java/org/elasticsearch/shield/transport/filter/IPFilterTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/transport/filter/IPFilterTests.java @@ -7,9 +7,11 @@ package org.elasticsearch.shield.transport.filter; import com.google.common.net.InetAddresses; import org.elasticsearch.common.component.Lifecycle; +import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.InetSocketTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.node.settings.NodeSettingsService; import org.elasticsearch.shield.audit.AuditTrail; @@ -21,7 +23,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import java.net.InetAddress; -import java.util.Locale; +import java.util.*; import static org.elasticsearch.common.settings.Settings.settingsBuilder; import static org.hamcrest.Matchers.is; @@ -45,13 +47,17 @@ public class IPFilterTests extends ESTestCase { httpTransport = mock(HttpServerTransport.class); InetSocketTransportAddress httpAddress = new InetSocketTransportAddress(InetAddress.getLoopbackAddress(), 9200); - when(httpTransport.boundAddress()).thenReturn(new BoundTransportAddress(httpAddress, httpAddress)); + when(httpTransport.boundAddress()).thenReturn(new BoundTransportAddress(new TransportAddress[] { httpAddress }, httpAddress)); when(httpTransport.lifecycleState()).thenReturn(Lifecycle.State.STARTED); transport = mock(Transport.class); InetSocketTransportAddress address = new InetSocketTransportAddress(InetAddress.getLoopbackAddress(), 9300); - when(transport.boundAddress()).thenReturn(new BoundTransportAddress(address, address)); + when(transport.boundAddress()).thenReturn(new BoundTransportAddress(new TransportAddress[]{ address }, address)); when(transport.lifecycleState()).thenReturn(Lifecycle.State.STARTED); + + Map profileBoundAddresses = Collections.singletonMap("client", + new BoundTransportAddress(new TransportAddress[]{ new InetSocketTransportAddress(InetAddress.getLoopbackAddress(), 9500) }, address)); + when(transport.profileBoundAddresses()).thenReturn(profileBoundAddresses); } @Test @@ -171,15 +177,25 @@ public class IPFilterTests extends ESTestCase { } @Test - @AwaitsFix(bugUrl = "https://github.com/elastic/x-plugins/issues/487") - public void testThatLocalhostIsNeverRejected() throws Exception { - Settings settings = settingsBuilder() - .put("shield.transport.filter.deny", "127.0.0.1") - .build(); + public void testThatBoundAddressIsNeverRejected() throws Exception { + List addressStrings = new ArrayList<>(); + for (TransportAddress address : transport.boundAddress().boundAddresses()) { + addressStrings.add(NetworkAddress.formatAddress(((InetSocketTransportAddress) address).address().getAddress())); + } + + Settings settings; + if (randomBoolean()) { + settings = settingsBuilder().putArray("shield.transport.filter.deny", addressStrings.toArray(new String[addressStrings.size()])).build(); + } else { + settings = settingsBuilder().put("shield.transport.filter.deny", "_all").build(); + } ipFilter = new IPFilter(settings, auditTrail, nodeSettingsService, transport).start(); ipFilter.setHttpServerTransport(httpTransport); - assertAddressIsAllowedForProfile(IPFilter.HTTP_PROFILE_NAME, "127.0.0.1"); + for (String addressString : addressStrings) { + assertAddressIsAllowedForProfile(IPFilter.HTTP_PROFILE_NAME, addressString); + assertAddressIsAllowedForProfile("default", addressString); + } } private void assertAddressIsAllowedForProfile(String profile, String ... inetAddresses) { diff --git a/shield/src/test/java/org/elasticsearch/shield/transport/filter/IpFilteringIntegrationTests.java b/shield/src/test/java/org/elasticsearch/shield/transport/filter/IpFilteringIntegrationTests.java index e396aa77a1a..65a78120596 100644 --- a/shield/src/test/java/org/elasticsearch/shield/transport/filter/IpFilteringIntegrationTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/transport/filter/IpFilteringIntegrationTests.java @@ -52,7 +52,7 @@ public class IpFilteringIntegrationTests extends ShieldIntegTestCase { @Test public void testThatIpFilteringIsIntegratedIntoNettyPipelineViaHttp() throws Exception { - TransportAddress transportAddress = internalCluster().getDataNodeInstance(HttpServerTransport.class).boundAddress().boundAddress(); + TransportAddress transportAddress = randomFrom(internalCluster().getDataNodeInstance(HttpServerTransport.class).boundAddress().boundAddresses()); assertThat(transportAddress, is(instanceOf(InetSocketTransportAddress.class))); InetSocketTransportAddress inetSocketTransportAddress = (InetSocketTransportAddress) transportAddress; @@ -88,7 +88,7 @@ public class IpFilteringIntegrationTests extends ShieldIntegTestCase { } private static int getProfilePort(String profile) { - TransportAddress transportAddress = internalCluster().getInstance(Transport.class).profileBoundAddresses().get(profile).boundAddress(); + TransportAddress transportAddress = randomFrom(internalCluster().getInstance(Transport.class).profileBoundAddresses().get(profile).boundAddresses()); assert transportAddress instanceof InetSocketTransportAddress; return ((InetSocketTransportAddress)transportAddress).address().getPort(); } diff --git a/shield/src/test/java/org/elasticsearch/shield/transport/filter/IpFilteringUpdateTests.java b/shield/src/test/java/org/elasticsearch/shield/transport/filter/IpFilteringUpdateTests.java index 3396660d53c..ccf8d251e78 100644 --- a/shield/src/test/java/org/elasticsearch/shield/transport/filter/IpFilteringUpdateTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/transport/filter/IpFilteringUpdateTests.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.node.Node; import org.elasticsearch.test.ESIntegTestCase.ClusterScope; import org.elasticsearch.test.ShieldIntegTestCase; +import org.junit.BeforeClass; import org.junit.Test; import java.net.InetAddress; @@ -24,14 +25,23 @@ import static org.hamcrest.Matchers.is; @ClusterScope(scope = TEST, numDataNodes = 1) public class IpFilteringUpdateTests extends ShieldIntegTestCase { + private static int randomClientPort; + private final boolean httpEnabled = randomBoolean(); + @BeforeClass + public static void getRandomPort() { + randomClientPort = randomIntBetween(49000, 65500); + } + @Override protected Settings nodeSettings(int nodeOrdinal) { + String randomClientPortRange = randomClientPort + "-" + (randomClientPort+100); return settingsBuilder() .put(super.nodeSettings(nodeOrdinal)) .put(Node.HTTP_ENABLED, httpEnabled) .put("shield.transport.filter.deny", "127.0.0.200") + .put("transport.profiles.client.port", randomClientPortRange) .build(); } @@ -127,16 +137,16 @@ public class IpFilteringUpdateTests extends ShieldIntegTestCase { @Test public void testThatDisablingIpFilterForProfilesWorksAsExpected() throws Exception { Settings settings = settingsBuilder() - .put("transport.profiles.myprofile.shield.filter.deny", "127.0.0.8") + .put("transport.profiles.client.shield.filter.deny", "127.0.0.8") .build(); updateSettings(settings); - assertConnectionRejected("myprofile", "127.0.0.8"); + assertConnectionRejected("client", "127.0.0.8"); settings = settingsBuilder() .put("shield.transport.filter.enabled", false) .build(); updateSettings(settings); - assertConnectionAccepted("myprofile", "127.0.0.8"); + assertConnectionAccepted("client", "127.0.0.8"); } diff --git a/shield/src/test/java/org/elasticsearch/shield/transport/filter/ShieldIpFilterRuleTests.java b/shield/src/test/java/org/elasticsearch/shield/transport/filter/ShieldIpFilterRuleTests.java new file mode 100644 index 00000000000..6469ff74f8d --- /dev/null +++ b/shield/src/test/java/org/elasticsearch/shield/transport/filter/ShieldIpFilterRuleTests.java @@ -0,0 +1,72 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.shield.transport.filter; + +import org.elasticsearch.test.ESTestCase; +import org.jboss.netty.handler.ipfilter.IpFilterRule; +import org.jboss.netty.handler.ipfilter.IpSubnetFilterRule; +import org.jboss.netty.handler.ipfilter.PatternRule; +import org.junit.Test; + +import static org.elasticsearch.shield.transport.filter.ShieldIpFilterRule.*; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.sameInstance; + +/** + * Unit tests for the {@link ShieldIpFilterRule} + */ +public class ShieldIpFilterRuleTests extends ESTestCase { + + @Test + public void testParseAllRules() { + IpFilterRule rule = getRule(true, "_all"); + assertThat(rule, sameInstance(ACCEPT_ALL)); + + rule = getRule(false, "_all"); + assertThat(rule, sameInstance(DENY_ALL)); + } + + @Test + public void testParseAllRuleWithOtherValues() { + String ruleValue = "_all," + randomFrom("name", "127.0.0.1", "127.0.0.0/24"); + try { + getRule(randomBoolean(), ruleValue); + fail("an illegal argument exception should have been thrown!"); + } catch (IllegalArgumentException e) { + // expected + } + } + + @Test + public void testParseIpSubnetFilterRule() throws Exception { + final boolean allow = randomBoolean(); + IpFilterRule rule = getRule(allow, "127.0.0.0/24"); + assertThat(rule, instanceOf(IpSubnetFilterRule.class)); + assertThat(rule.isAllowRule(), equalTo(allow)); + IpSubnetFilterRule ipSubnetFilterRule = (IpSubnetFilterRule) rule; + assertThat(ipSubnetFilterRule.contains("127.0.0.1"), equalTo(true)); + } + + @Test + public void testParseIpSubnetFilterRuleWithOtherValues() throws Exception { + try { + getRule(randomBoolean(), "127.0.0.0/24," + randomFrom("name", "127.0.0.1", "192.0.0.0/24")); + fail("expected an exception to be thrown because only one subnet can be specified at a time"); + } catch (IllegalArgumentException e) { + //expected + } + } + + @Test + public void testParsePatternRules() { + final boolean allow = randomBoolean(); + String ruleSpec = "127.0.0.1,::1,192.168.0.*,name*,specific_name"; + IpFilterRule rule = getRule(allow, ruleSpec); + assertThat(rule, instanceOf(PatternRule.class)); + assertThat(rule.isAllowRule(), equalTo(allow)); + } +} diff --git a/shield/src/test/java/org/elasticsearch/shield/transport/netty/IPFilterNettyUpstreamHandlerTests.java b/shield/src/test/java/org/elasticsearch/shield/transport/netty/IPFilterNettyUpstreamHandlerTests.java index 17e561b91ac..bc4711d45aa 100644 --- a/shield/src/test/java/org/elasticsearch/shield/transport/netty/IPFilterNettyUpstreamHandlerTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/transport/netty/IPFilterNettyUpstreamHandlerTests.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.component.Lifecycle; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.InetSocketTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.node.settings.NodeSettingsService; import org.elasticsearch.shield.audit.AuditTrail; @@ -47,7 +48,7 @@ public class IPFilterNettyUpstreamHandlerTests extends ESTestCase { Transport transport = mock(Transport.class); InetSocketTransportAddress address = new InetSocketTransportAddress(InetAddress.getLoopbackAddress(), 9300); - when(transport.boundAddress()).thenReturn(new BoundTransportAddress(address, address)); + when(transport.boundAddress()).thenReturn(new BoundTransportAddress(new TransportAddress[] { address }, address)); when(transport.lifecycleState()).thenReturn(Lifecycle.State.STARTED); NodeSettingsService nodeSettingsService = mock(NodeSettingsService.class); @@ -56,7 +57,7 @@ public class IPFilterNettyUpstreamHandlerTests extends ESTestCase { if (isHttpEnabled) { HttpServerTransport httpTransport = mock(HttpServerTransport.class); InetSocketTransportAddress httpAddress = new InetSocketTransportAddress(InetAddress.getLoopbackAddress(), 9200); - when(httpTransport.boundAddress()).thenReturn(new BoundTransportAddress(httpAddress, httpAddress)); + when(httpTransport.boundAddress()).thenReturn(new BoundTransportAddress(new TransportAddress[] { httpAddress }, httpAddress)); when(httpTransport.lifecycleState()).thenReturn(Lifecycle.State.STARTED); ipFilter.setHttpServerTransport(httpTransport); } diff --git a/shield/src/test/java/org/elasticsearch/shield/transport/ssl/SslIntegrationTests.java b/shield/src/test/java/org/elasticsearch/shield/transport/ssl/SslIntegrationTests.java index 1cd2f058edb..82eebbd28a0 100644 --- a/shield/src/test/java/org/elasticsearch/shield/transport/ssl/SslIntegrationTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/transport/ssl/SslIntegrationTests.java @@ -65,7 +65,7 @@ public class SslIntegrationTests extends ShieldIntegTestCase { .putArray("shield.ssl.ciphers", new String[]{"TLS_ECDH_anon_WITH_RC4_128_SHA", "SSL_RSA_WITH_3DES_EDE_CBC_SHA"}) .build()).build()) { - TransportAddress transportAddress = internalCluster().getInstance(Transport.class).boundAddress().boundAddress(); + TransportAddress transportAddress = randomFrom(internalCluster().getInstance(Transport.class).boundAddress().boundAddresses()); transportClient.addTransportAddress(transportAddress); transportClient.admin().cluster().prepareHealth().get(); @@ -82,7 +82,7 @@ public class SslIntegrationTests extends ShieldIntegTestCase { .putArray("shield.ssl.supported_protocols", new String[]{"SSLv3"}) .build()).build()) { - TransportAddress transportAddress = internalCluster().getInstance(Transport.class).boundAddress().boundAddress(); + TransportAddress transportAddress = randomFrom(internalCluster().getInstance(Transport.class).boundAddress().boundAddresses()); transportClient.addTransportAddress(transportAddress); transportClient.admin().cluster().prepareHealth().get(); @@ -121,7 +121,7 @@ public class SslIntegrationTests extends ShieldIntegTestCase { } private String getNodeUrl() { - TransportAddress transportAddress = internalCluster().getInstance(HttpServerTransport.class).boundAddress().boundAddress(); + TransportAddress transportAddress = randomFrom(internalCluster().getInstance(HttpServerTransport.class).boundAddress().boundAddresses()); assertThat(transportAddress, is(instanceOf(InetSocketTransportAddress.class))); InetSocketTransportAddress inetSocketTransportAddress = (InetSocketTransportAddress) transportAddress; return String.format(Locale.ROOT, "https://%s:%s/", "localhost", inetSocketTransportAddress.address().getPort()); diff --git a/shield/src/test/java/org/elasticsearch/shield/transport/ssl/SslMultiPortTests.java b/shield/src/test/java/org/elasticsearch/shield/transport/ssl/SslMultiPortTests.java index e1d95dc655d..ed08297c307 100644 --- a/shield/src/test/java/org/elasticsearch/shield/transport/ssl/SslMultiPortTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/transport/ssl/SslMultiPortTests.java @@ -186,7 +186,7 @@ public class SslMultiPortTests extends ShieldIntegTestCase { public void testThatProfileTransportClientCannotConnectToDefaultProfile() throws Exception { Settings settings = ShieldSettingsSource.getSSLSettingsForStore("/org/elasticsearch/shield/transport/ssl/certs/simple/testclient-client-profile.jks", "testclient-client-profile"); try (TransportClient transportClient = createTransportClient(settings)) { - TransportAddress transportAddress = internalCluster().getInstance(Transport.class).boundAddress().boundAddress(); + TransportAddress transportAddress = randomFrom(internalCluster().getInstance(Transport.class).boundAddress().boundAddresses()); transportClient.addTransportAddress(transportAddress); transportClient.admin().cluster().prepareHealth().get(); } @@ -232,7 +232,7 @@ public class SslMultiPortTests extends ShieldIntegTestCase { .put("cluster.name", internalCluster().getClusterName()) .build(); try (TransportClient transportClient = TransportClient.builder().settings(settings).build()) { - transportClient.addTransportAddress(internalCluster().getInstance(Transport.class).boundAddress().boundAddress()); + transportClient.addTransportAddress(randomFrom(internalCluster().getInstance(Transport.class).boundAddress().boundAddresses())); assertGreenClusterState(transportClient); } } @@ -326,7 +326,7 @@ public class SslMultiPortTests extends ShieldIntegTestCase { .put("shield.ssl.truststore.password", "truststore-testnode-only") .build(); try (TransportClient transportClient = TransportClient.builder().settings(settings).build()) { - transportClient.addTransportAddress(internalCluster().getInstance(Transport.class).boundAddress().boundAddress()); + transportClient.addTransportAddress(randomFrom(internalCluster().getInstance(Transport.class).boundAddress().boundAddresses())); assertGreenClusterState(transportClient); } } @@ -364,7 +364,7 @@ public class SslMultiPortTests extends ShieldIntegTestCase { .put("shield.transport.ssl", true) .build(); try (TransportClient transportClient = TransportClient.builder().settings(settings).build()) { - transportClient.addTransportAddress(internalCluster().getInstance(Transport.class).boundAddress().boundAddress()); + transportClient.addTransportAddress(randomFrom(internalCluster().getInstance(Transport.class).boundAddress().boundAddresses())); assertGreenClusterState(transportClient); } } @@ -424,7 +424,7 @@ public class SslMultiPortTests extends ShieldIntegTestCase { } private static int getProfilePort(String profile) { - TransportAddress transportAddress = internalCluster().getInstance(Transport.class).profileBoundAddresses().get(profile).boundAddress(); + TransportAddress transportAddress = randomFrom(internalCluster().getInstance(Transport.class).profileBoundAddresses().get(profile).boundAddresses()); assert transportAddress instanceof InetSocketTransportAddress; return ((InetSocketTransportAddress)transportAddress).address().getPort(); }