From a7da064369fb9714e2d4ad9343e5819e086d0837 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 24 Feb 2020 12:30:44 +0200 Subject: [PATCH 1/2] Logfile audit settings validation (#52537) Add validation for the following logfile audit settings: xpack.security.audit.logfile.events.include xpack.security.audit.logfile.events.exclude xpack.security.audit.logfile.events.ignore_filters.*.users xpack.security.audit.logfile.events.ignore_filters.*.realms xpack.security.audit.logfile.events.ignore_filters.*.roles xpack.security.audit.logfile.events.ignore_filters.*.indices Closes #52357 Relates #47711 #47038 Follows the example from #47246 --- .../audit/logfile/LoggingAuditTrail.java | 34 ++++++++----- .../AuditTrailSettingsUpdateTests.java | 2 +- .../audit/logfile/LoggingAuditTrailTests.java | 51 +++++++++++++++++++ 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java index 119064d0ab7f5..e8d7ac934aa33 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java @@ -142,27 +142,33 @@ public class LoggingAuditTrail extends AbstractComponent implements AuditTrail, ANONYMOUS_ACCESS_DENIED.toString(), AUTHENTICATION_FAILED.toString(), CONNECTION_DENIED.toString(), TAMPERED_REQUEST.toString(), RUN_AS_DENIED.toString(), RUN_AS_GRANTED.toString()); public static final Setting> INCLUDE_EVENT_SETTINGS = Setting.listSetting(setting("audit.logfile.events.include"), - DEFAULT_EVENT_INCLUDES, Function.identity(), Property.NodeScope, Property.Dynamic); + DEFAULT_EVENT_INCLUDES, Function.identity(), value -> AuditLevel.parse(value, List.of()), + Property.NodeScope, Property.Dynamic); public static final Setting> EXCLUDE_EVENT_SETTINGS = Setting.listSetting(setting("audit.logfile.events.exclude"), - Collections.emptyList(), Function.identity(), Property.NodeScope, Property.Dynamic); + Collections.emptyList(), Function.identity(), value -> AuditLevel.parse(List.of(), value), + Property.NodeScope, Property.Dynamic); public static final Setting INCLUDE_REQUEST_BODY = Setting.boolSetting(setting("audit.logfile.events.emit_request_body"), false, Property.NodeScope, Property.Dynamic); public static final String FILTER_POLICY_PREFIX = setting("audit.logfile.events.ignore_filters."); // because of the default wildcard value (*) for the field filter, a policy with // an unspecified filter field will match events that have any value for that // particular field, as well as events with that particular field missing - public static final Setting.AffixSetting> FILTER_POLICY_IGNORE_PRINCIPALS = Setting.affixKeySetting(FILTER_POLICY_PREFIX, - "users", - (key) -> Setting.listSetting(key, Collections.singletonList("*"), Function.identity(), Property.NodeScope, Property.Dynamic)); - public static final Setting.AffixSetting> FILTER_POLICY_IGNORE_REALMS = Setting.affixKeySetting(FILTER_POLICY_PREFIX, - "realms", - (key) -> Setting.listSetting(key, Collections.singletonList("*"), Function.identity(), Property.NodeScope, Property.Dynamic)); - public static final Setting.AffixSetting> FILTER_POLICY_IGNORE_ROLES = Setting.affixKeySetting(FILTER_POLICY_PREFIX, - "roles", - (key) -> Setting.listSetting(key, Collections.singletonList("*"), Function.identity(), Property.NodeScope, Property.Dynamic)); - public static final Setting.AffixSetting> FILTER_POLICY_IGNORE_INDICES = Setting.affixKeySetting(FILTER_POLICY_PREFIX, - "indices", - (key) -> Setting.listSetting(key, Collections.singletonList("*"), Function.identity(), Property.NodeScope, Property.Dynamic)); + protected static final Setting.AffixSetting> FILTER_POLICY_IGNORE_PRINCIPALS = + Setting.affixKeySetting(FILTER_POLICY_PREFIX, "users", + (key) -> Setting.listSetting(key, Collections.singletonList("*"), Function.identity(), + value -> EventFilterPolicy.parsePredicate(value), Property.NodeScope, Property.Dynamic)); + protected static final Setting.AffixSetting> FILTER_POLICY_IGNORE_REALMS = + Setting.affixKeySetting(FILTER_POLICY_PREFIX, "realms", + (key) -> Setting.listSetting(key, Collections.singletonList("*"), Function.identity(), + value -> EventFilterPolicy.parsePredicate(value), Property.NodeScope, Property.Dynamic)); + protected static final Setting.AffixSetting> FILTER_POLICY_IGNORE_ROLES = + Setting.affixKeySetting(FILTER_POLICY_PREFIX, "roles", + (key) -> Setting.listSetting(key, Collections.singletonList("*"), Function.identity(), + value -> EventFilterPolicy.parsePredicate(value), Property.NodeScope, Property.Dynamic)); + protected static final Setting.AffixSetting> FILTER_POLICY_IGNORE_INDICES = + Setting.affixKeySetting(FILTER_POLICY_PREFIX, "indices", + (key) -> Setting.listSetting(key, Collections.singletonList("*"), Function.identity(), + value -> EventFilterPolicy.parsePredicate(value), Property.NodeScope, Property.Dynamic)); private static final Marker AUDIT_MARKER = MarkerManager.getMarker("org.elasticsearch.xpack.security.audit"); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/AuditTrailSettingsUpdateTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/AuditTrailSettingsUpdateTests.java index 531749c8efbcd..484ce50c2e5ec 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/AuditTrailSettingsUpdateTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/AuditTrailSettingsUpdateTests.java @@ -104,7 +104,7 @@ public void testInvalidFilterSettings() throws Exception { settingsBuilder.put(randomFrom(allSettingsKeys), invalidLuceneRegex); final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings().setTransientSettings(settingsBuilder.build()).get()); - assertThat(e.getMessage(), containsString("illegal value can't update")); + assertThat(e.getMessage(), containsString("invalid pattern [/invalid]")); } public void testDynamicHostSettings() { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java index be6812a39f486..7fd3870b234ec 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java @@ -66,6 +66,9 @@ import java.util.regex.Pattern; import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail.PRINCIPAL_ROLES_FIELD_NAME; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; @@ -206,6 +209,54 @@ public void clearLog() throws Exception { CapturingLogger.output(logger.getName(), Level.INFO).clear(); } + public void testEventsSettingValidation() { + final String prefix = "xpack.security.audit.logfile.events."; + Settings settings = Settings.builder().putList(prefix + "include", Arrays.asList("access_granted", "bogus")).build(); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> LoggingAuditTrail.INCLUDE_EVENT_SETTINGS.get(settings)); + assertThat(e, hasToString(containsString("invalid event name specified [bogus]"))); + + Settings settings2 = Settings.builder().putList(prefix + "exclude", Arrays.asList("access_denied", "foo")).build(); + e = expectThrows(IllegalArgumentException.class, () -> LoggingAuditTrail.EXCLUDE_EVENT_SETTINGS.get(settings2)); + assertThat(e, hasToString(containsString("invalid event name specified [foo]"))); + } + + public void testAuditFilterSettingValidation() { + final String prefix = "xpack.security.audit.logfile.events."; + Settings settings = + Settings.builder().putList(prefix + "ignore_filters.filter1.users", Arrays.asList("mickey", "/bogus")).build(); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> LoggingAuditTrail.FILTER_POLICY_IGNORE_PRINCIPALS.getConcreteSettingForNamespace("filter1").get(settings)); + assertThat(e, hasToString(containsString("invalid pattern [/bogus]"))); + + Settings settings2 = Settings.builder() + .putList(prefix + "ignore_filters.filter2.users", Arrays.asList("tom", "cruise")) + .putList(prefix + "ignore_filters.filter2.realms", Arrays.asList("native", "/foo")).build(); + assertThat(LoggingAuditTrail.FILTER_POLICY_IGNORE_PRINCIPALS.getConcreteSettingForNamespace("filter2").get(settings2), + containsInAnyOrder("tom", "cruise")); + e = expectThrows(IllegalArgumentException.class, + () -> LoggingAuditTrail.FILTER_POLICY_IGNORE_REALMS.getConcreteSettingForNamespace("filter2").get(settings2)); + assertThat(e, hasToString(containsString("invalid pattern [/foo]"))); + + Settings settings3 = Settings.builder() + .putList(prefix + "ignore_filters.filter3.realms", Arrays.asList("native", "oidc1")) + .putList(prefix + "ignore_filters.filter3.roles", Arrays.asList("kibana", "/wrong")).build(); + assertThat(LoggingAuditTrail.FILTER_POLICY_IGNORE_REALMS.getConcreteSettingForNamespace("filter3").get(settings3), + containsInAnyOrder("native", "oidc1")); + e = expectThrows(IllegalArgumentException.class, + () -> LoggingAuditTrail.FILTER_POLICY_IGNORE_ROLES.getConcreteSettingForNamespace("filter3").get(settings3)); + assertThat(e, hasToString(containsString("invalid pattern [/wrong]"))); + + Settings settings4 = Settings.builder() + .putList(prefix + "ignore_filters.filter4.roles", Arrays.asList("kibana", "elastic")) + .putList(prefix + "ignore_filters.filter4.indices", Arrays.asList("index-1", "/no-inspiration")).build(); + assertThat(LoggingAuditTrail.FILTER_POLICY_IGNORE_ROLES.getConcreteSettingForNamespace("filter4").get(settings4), + containsInAnyOrder("kibana", "elastic")); + e = expectThrows(IllegalArgumentException.class, + () -> LoggingAuditTrail.FILTER_POLICY_IGNORE_INDICES.getConcreteSettingForNamespace("filter4").get(settings4)); + assertThat(e, hasToString(containsString("invalid pattern [/no-inspiration]"))); + } + public void testAnonymousAccessDeniedTransport() throws Exception { final TransportMessage message = randomBoolean() ? new MockMessage(threadContext) : new MockIndicesRequest(threadContext); From 8db7bd8db0586fa8d9c2da20190405a6e6268470 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 24 Feb 2020 13:47:03 +0200 Subject: [PATCH 2/2] Compile error --- .../xpack/security/audit/logfile/LoggingAuditTrail.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java index e8d7ac934aa33..e446c44f99de0 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java @@ -142,10 +142,10 @@ public class LoggingAuditTrail extends AbstractComponent implements AuditTrail, ANONYMOUS_ACCESS_DENIED.toString(), AUTHENTICATION_FAILED.toString(), CONNECTION_DENIED.toString(), TAMPERED_REQUEST.toString(), RUN_AS_DENIED.toString(), RUN_AS_GRANTED.toString()); public static final Setting> INCLUDE_EVENT_SETTINGS = Setting.listSetting(setting("audit.logfile.events.include"), - DEFAULT_EVENT_INCLUDES, Function.identity(), value -> AuditLevel.parse(value, List.of()), + DEFAULT_EVENT_INCLUDES, Function.identity(), value -> AuditLevel.parse(value, Collections.emptyList()), Property.NodeScope, Property.Dynamic); public static final Setting> EXCLUDE_EVENT_SETTINGS = Setting.listSetting(setting("audit.logfile.events.exclude"), - Collections.emptyList(), Function.identity(), value -> AuditLevel.parse(List.of(), value), + Collections.emptyList(), Function.identity(), value -> AuditLevel.parse(Collections.emptyList, value), Property.NodeScope, Property.Dynamic); public static final Setting INCLUDE_REQUEST_BODY = Setting.boolSetting(setting("audit.logfile.events.emit_request_body"), false, Property.NodeScope, Property.Dynamic);