Skip to content

Commit

Permalink
Validate IP filter settings (#72195)
Browse files Browse the repository at this point in the history 
Today it is possible to set up an IP filter which throws an exception on
creation, which is fatal to the cluster. This commit adds validation to
the IP filtering settings to prevent this.

Closes #72192
  • Loading branch information
@DaveCTurner
DaveCTurner committed Apr 26, 2021
1 parent aacfaa2 commit 115b385
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 24 deletions.
37 changes: 37 additions & 0 deletions ...erTest/java/org/elasticsearch/xpack/security/transport/filter/IpFilteringUpdateTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

import static org.elasticsearch.test.ESIntegTestCase.Scope.TEST;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;

@ClusterScope(scope = TEST, supportsDedicatedMasters = false, numDataNodes = 1)
Expand Down Expand Up @@ -122,6 +124,41 @@ public void testThatIpFilterConfigurationCanBeChangedDynamically() throws Except
}
}

public void testThatInvalidDynamicIpFilterConfigurationIsRejected() {
final Settings.Builder initialSettingsBuilder = Settings.builder();
if (randomBoolean()) {
initialSettingsBuilder.put(IPFilter.IP_FILTER_ENABLED_SETTING.getKey(), randomBoolean());
}
if (randomBoolean()) {
initialSettingsBuilder.put(IPFilter.IP_FILTER_ENABLED_HTTP_SETTING.getKey(), randomBoolean());
}
final Settings initialSettings = initialSettingsBuilder.build();
if (initialSettings.isEmpty() == false) {
updateSettings(initialSettings);
}

final String invalidValue = "http://";

for (final String settingPrefix : new String[] {
"xpack.security.transport.filter",
"xpack.security.http.filter",
"transport.profiles.default.xpack.security.filter",
"transport.profiles.anotherprofile.xpack.security.filter"
}) {
for (final String settingSuffix : new String[]{"allow", "deny"}) {
final String settingName = settingPrefix + "." + settingSuffix;
final Settings settings = Settings.builder().put(settingName, invalidValue).build();
assertThat(
settingName,
expectThrows(
IllegalArgumentException.class,
settingName,
() -> updateSettings(settings)).getMessage(),
allOf(containsString("invalid IP filter"), containsString(invalidValue)));
}
}
}

// issue #762, occurred because in the above test we use HTTP and transport
public void testThatDisablingIpFilterWorksAsExpected() throws Exception {
Settings settings = Settings.builder()
Expand Down
116 changes: 92 additions & 24 deletions ...in/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,30 +56,76 @@ public class IPFilter {
public static final Setting<Boolean> IP_FILTER_ENABLED_SETTING = Setting.boolSetting(setting("transport.filter.enabled"),
true, Property.OperatorDynamic, Property.NodeScope);

public static final Setting<List<String>> TRANSPORT_FILTER_ALLOW_SETTING = Setting.listSetting(setting("transport.filter.allow"),
Collections.emptyList(), Function.identity(), Property.OperatorDynamic, Property.NodeScope);

public static final Setting<List<String>> TRANSPORT_FILTER_DENY_SETTING = Setting.listSetting(setting("transport.filter.deny"),
Collections.emptyList(), Function.identity(), Property.OperatorDynamic, Property.NodeScope);

public static final Setting.AffixSetting<List<String>> PROFILE_FILTER_DENY_SETTING = Setting.affixKeySetting("transport.profiles.",
"xpack.security.filter.deny", key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(),
Property.OperatorDynamic, Property.NodeScope));
public static final Setting.AffixSetting<List<String>> PROFILE_FILTER_ALLOW_SETTING = Setting.affixKeySetting("transport.profiles.",
"xpack.security.filter.allow", key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(),
Property.OperatorDynamic, Property.NodeScope));

private static final Setting<List<String>> HTTP_FILTER_ALLOW_FALLBACK =
Setting.listSetting("transport.profiles.default.xpack.security.filter.allow", TRANSPORT_FILTER_ALLOW_SETTING, s -> s,
Property.NodeScope);
public static final Setting<List<String>> HTTP_FILTER_ALLOW_SETTING = Setting.listSetting(setting("http.filter.allow"),
HTTP_FILTER_ALLOW_FALLBACK, Function.identity(), Property.OperatorDynamic, Property.NodeScope);

private static final Setting<List<String>> HTTP_FILTER_DENY_FALLBACK =
Setting.listSetting("transport.profiles.default.xpack.security.filter.deny", TRANSPORT_FILTER_DENY_SETTING, s -> s,
Property.NodeScope);
public static final Setting<List<String>> HTTP_FILTER_DENY_SETTING = Setting.listSetting(setting("http.filter.deny"),
HTTP_FILTER_DENY_FALLBACK, Function.identity(), Property.OperatorDynamic, Property.NodeScope);
private static final IPFilterValidator ALLOW_VALIDATOR = new IPFilterValidator(true);
private static final IPFilterValidator DENY_VALIDATOR = new IPFilterValidator(false);

public static final Setting<List<String>> TRANSPORT_FILTER_ALLOW_SETTING = Setting.listSetting(
setting("transport.filter.allow"),
Collections.emptyList(),
Function.identity(),
ALLOW_VALIDATOR,
Property.OperatorDynamic,
Property.NodeScope);
public static final Setting<List<String>> TRANSPORT_FILTER_DENY_SETTING = Setting.listSetting(
setting("transport.filter.deny"),
Collections.emptyList(),
Function.identity(),
DENY_VALIDATOR,
Property.OperatorDynamic,
Property.NodeScope);

public static final Setting.AffixSetting<List<String>> PROFILE_FILTER_DENY_SETTING = Setting.affixKeySetting(
"transport.profiles.",
"xpack.security.filter.deny",
key -> Setting.listSetting(
key,
Collections.emptyList(),
Function.identity(),
DENY_VALIDATOR,
Property.OperatorDynamic,
Property.NodeScope));
public static final Setting.AffixSetting<List<String>> PROFILE_FILTER_ALLOW_SETTING = Setting.affixKeySetting(
"transport.profiles.",
"xpack.security.filter.allow",
key -> Setting.listSetting(
key,
Collections.emptyList(),
Function.identity(),
ALLOW_VALIDATOR,
Property.OperatorDynamic,
Property.NodeScope));

private static final Setting<List<String>> HTTP_FILTER_ALLOW_FALLBACK = Setting.listSetting(
"transport.profiles.default.xpack.security.filter.allow",
TRANSPORT_FILTER_ALLOW_SETTING,
Function.identity(),
TRANSPORT_FILTER_ALLOW_SETTING::get,
ALLOW_VALIDATOR,
Property.NodeScope);
public static final Setting<List<String>> HTTP_FILTER_ALLOW_SETTING = Setting.listSetting(
setting("http.filter.allow"),
HTTP_FILTER_ALLOW_FALLBACK,
Function.identity(),
HTTP_FILTER_ALLOW_FALLBACK::get,
ALLOW_VALIDATOR,
Property.OperatorDynamic,
Property.NodeScope);

private static final Setting<List<String>> HTTP_FILTER_DENY_FALLBACK = Setting.listSetting(
"transport.profiles.default.xpack.security.filter.deny",
TRANSPORT_FILTER_DENY_SETTING,
Function.identity(),
TRANSPORT_FILTER_DENY_SETTING::get,
DENY_VALIDATOR,
Property.NodeScope);
public static final Setting<List<String>> HTTP_FILTER_DENY_SETTING = Setting.listSetting(
setting("http.filter.deny"),
HTTP_FILTER_DENY_FALLBACK,
Function.identity(),
HTTP_FILTER_DENY_FALLBACK::get,
DENY_VALIDATOR,
Property.OperatorDynamic,
Property.NodeScope);

public static final Map<String, Object> DISABLED_USAGE_STATS = new MapBuilder<String, Object>()
.put("http", false)
Expand Down Expand Up @@ -306,4 +352,26 @@ public static void addSettings(List<Setting<?>> settings) {
settings.add(PROFILE_FILTER_ALLOW_SETTING);
settings.add(PROFILE_FILTER_DENY_SETTING);
}

private static class IPFilterValidator implements Setting.Validator<List<String>> {

private final boolean isAllowRule;

IPFilterValidator(boolean isAllowRule) {
this.isAllowRule = isAllowRule;
}

@Override
public void validate(List<String> filterSpecs) {
for (String filterSpec : filterSpecs) {
try {
// It's enough just to create the specified rule:
new SecurityIpFilterRule(isAllowRule, filterSpec);
} catch (Exception e) {
throw new IllegalArgumentException("invalid IP filter [" + filterSpec + "]", e);
}
}
}
}

}

0 comments on commit 115b385

Please sign in to comment.