honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix NPE in Logfile Audit Filter (#38120) 

The culprit in #38097 is an `IndicesRequest` that has no indices,
but instead of `request.indices()` returning `null` or `String[0]`
it returned `String[] {null}` . This tripped the audit filter.

I have addressed this in two ways:
1. `request.indices()` returning `String[] {null}` is treated as `null`
    or `String[0]`, i.e. no indices
2. `null` values among the roles and indices lists, which are
    unexpected, will never again stumble the audit filter; `null` values
    are treated as special values that will not match any policy,
    i.e. their events will always be printed.

Closes #38097
This commit is contained in:
Albert Zaharovits 2019-02-03 10:34:17 +02:00 committed by GitHub
parent 89d7c57bd9
commit 3c1544d259
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 91 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java
View File

@ -50,6 +50,7 @@ import java.util.EnumSet;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects;
import java.util.Optional; import java.util.Optional;
import java.util.TreeMap; import java.util.TreeMap;
import java.util.function.Function; import java.util.function.Function;
@ -845,6 +846,7 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
EventFilterPolicy(String name, Predicate<String> ignorePrincipalsPredicate, Predicate<String> ignoreRealmsPredicate, EventFilterPolicy(String name, Predicate<String> ignorePrincipalsPredicate, Predicate<String> ignoreRealmsPredicate,
Predicate<String> ignoreRolesPredicate, Predicate<String> ignoreIndicesPredicate) { Predicate<String> ignoreRolesPredicate, Predicate<String> ignoreIndicesPredicate) {
this.name = name; this.name = name;
// "null" values are "unexpected" and should not match any ignore policy
this.ignorePrincipalsPredicate = ignorePrincipalsPredicate; this.ignorePrincipalsPredicate = ignorePrincipalsPredicate;
this.ignoreRealmsPredicate = ignoreRealmsPredicate; this.ignoreRealmsPredicate = ignoreRealmsPredicate;
this.ignoreRolesPredicate = ignoreRolesPredicate; this.ignoreRolesPredicate = ignoreRolesPredicate;
@ -894,8 +896,10 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
* predicate of the corresponding field. * predicate of the corresponding field.
*/ */
Predicate<AuditEventMetaInfo> ignorePredicate() { Predicate<AuditEventMetaInfo> ignorePredicate() {
return eventInfo -> ignorePrincipalsPredicate.test(eventInfo.principal) && ignoreRealmsPredicate.test(eventInfo.realm) return eventInfo -> eventInfo.principal != null && ignorePrincipalsPredicate.test(eventInfo.principal)
&& eventInfo.roles.get().allMatch(ignoreRolesPredicate) && eventInfo.indices.get().allMatch(ignoreIndicesPredicate); && eventInfo.realm != null && ignoreRealmsPredicate.test(eventInfo.realm)
&& eventInfo.roles.get().allMatch(role -> role != null && ignoreRolesPredicate.test(role))
&& eventInfo.indices.get().allMatch(index -> index != null && ignoreIndicesPredicate.test(index));
} }
@Override @Override
@ -983,8 +987,10 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
// conditions on the `principal` and `realm` fields // conditions on the `principal` and `realm` fields
// 2. reusability of the AuditEventMetaInfo instance: in this case Streams have // 2. reusability of the AuditEventMetaInfo instance: in this case Streams have
// to be regenerated as they cannot be operated upon twice // to be regenerated as they cannot be operated upon twice
this.roles = () -> roles.filter(r -> r.length != 0).map(Arrays::stream).orElse(Stream.of("")); this.roles = () -> roles.filter(r -> r.length > 0).filter(a -> Arrays.stream(a).anyMatch(Objects::nonNull))
this.indices = () -> indices.filter(i -> i.length != 0).map(Arrays::stream).orElse(Stream.of("")); .map(Arrays::stream).orElse(Stream.of(""));
this.indices = () -> indices.filter(i -> i.length > 0).filter(a -> Arrays.stream(a).anyMatch(Objects::nonNull))
.map(Arrays::stream).orElse(Stream.of(""));
} }
AuditEventMetaInfo(Optional<AuthenticationToken> authenticationToken, Optional<String> realm, Optional<String[]> indices) { AuditEventMetaInfo(Optional<AuthenticationToken> authenticationToken, Optional<String> realm, Optional<String[]> indices) {

83
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailFilterTests.java
View File

@ -82,6 +82,78 @@ public class LoggingAuditTrailFilterTests extends ESTestCase {
}).when(clusterService).addListener(Mockito.isA(LoggingAuditTrail.class)); }).when(clusterService).addListener(Mockito.isA(LoggingAuditTrail.class));
} }
public void testPolicyDoesNotMatchNullValuesInEvent() throws Exception {
final Logger logger = CapturingLogger.newCapturingLogger(Level.INFO, null);
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
final Settings.Builder settingsBuilder = Settings.builder().put(settings);
// filter by username
final List<String> filteredUsernames = randomNonEmptyListOfFilteredNames();
final List<User> filteredUsers = filteredUsernames.stream().map(u -> {
if (randomBoolean()) {
return new User(u);
} else {
return new User(new User(u), new User(UNFILTER_MARKER + randomAlphaOfLengthBetween(1, 4)));
}
}).collect(Collectors.toList());
settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters.userPolicy.users", filteredUsernames);
// filter by realms
final List<String> filteredRealms = randomNonEmptyListOfFilteredNames();
settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters.realmsPolicy.realms", filteredRealms);
// filter by roles
final List<String> filteredRoles = randomNonEmptyListOfFilteredNames();
settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters.rolesPolicy.roles", filteredRoles);
// filter by indices
final List<String> filteredIndices = randomNonEmptyListOfFilteredNames();
settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters.indicesPolicy.indices", filteredIndices);
final LoggingAuditTrail auditTrail = new LoggingAuditTrail(settingsBuilder.build(), clusterService, logger, threadContext);
// user field matches
assertTrue("Matches the user filter predicate.", auditTrail.eventFilterPolicyRegistry.ignorePredicate().test(
new AuditEventMetaInfo(Optional.of(randomFrom(filteredUsers)), Optional.empty(), Optional.empty(), Optional.empty())));
final User unfilteredUser;
if (randomBoolean()) {
unfilteredUser = new User(null);
} else {
unfilteredUser = new User(new User(null), new User(randomFrom(filteredUsers).principal()));
}
// null user field does NOT match
assertFalse("Does not match the user filter predicate because of null username.",
auditTrail.eventFilterPolicyRegistry.ignorePredicate()
.test(new AuditEventMetaInfo(Optional.of(unfilteredUser), Optional.empty(), Optional.empty(), Optional.empty())));
// realm field matches
assertTrue("Matches the realm filter predicate.", auditTrail.eventFilterPolicyRegistry.ignorePredicate().test(
new AuditEventMetaInfo(Optional.empty(), Optional.of(randomFrom(filteredRealms)), Optional.empty(), Optional.empty())));
// null realm field does NOT match
assertFalse("Does not match the realm filter predicate because of null realm.",
auditTrail.eventFilterPolicyRegistry.ignorePredicate()
.test(new AuditEventMetaInfo(Optional.empty(), Optional.ofNullable(null), Optional.empty(), Optional.empty())));
// role field matches
assertTrue("Matches the role filter predicate.", auditTrail.eventFilterPolicyRegistry.ignorePredicate()
.test(new AuditEventMetaInfo(Optional.empty(), Optional.empty(),
Optional.of(randomSubsetOf(randomIntBetween(1, filteredRoles.size()), filteredRoles).toArray(new String[0])),
Optional.empty())));
final List<String> unfilteredRoles = new ArrayList<>();
unfilteredRoles.add(null);
unfilteredRoles.addAll(randomSubsetOf(randomIntBetween(1, filteredRoles.size()), filteredRoles));
// null role among roles field does NOT match
assertFalse("Does not match the role filter predicate because of null role.",
auditTrail.eventFilterPolicyRegistry.ignorePredicate().test(new AuditEventMetaInfo(Optional.empty(), Optional.empty(),
Optional.of(unfilteredRoles.toArray(new String[0])), Optional.empty())));
// indices field matches
assertTrue("Matches the index filter predicate.",
auditTrail.eventFilterPolicyRegistry.ignorePredicate().test(new AuditEventMetaInfo(Optional.empty(), Optional.empty(),
Optional.empty(),
Optional.of(randomSubsetOf(randomIntBetween(1, filteredIndices.size()), filteredIndices).toArray(new String[0])))));
final List<String> unfilteredIndices = new ArrayList<>();
unfilteredIndices.add(null);
unfilteredIndices.addAll(randomSubsetOf(randomIntBetween(1, filteredIndices.size()), filteredIndices));
// null index among indices field does NOT match
assertFalse("Does not match the indices filter predicate because of null index.",
auditTrail.eventFilterPolicyRegistry.ignorePredicate().test(new AuditEventMetaInfo(Optional.empty(), Optional.empty(),
Optional.empty(), Optional.of(unfilteredIndices.toArray(new String[0])))));
}
public void testSingleCompletePolicyPredicate() throws Exception { public void testSingleCompletePolicyPredicate() throws Exception {
final Logger logger = CapturingLogger.newCapturingLogger(Level.INFO, null); final Logger logger = CapturingLogger.newCapturingLogger(Level.INFO, null);
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
@ -265,11 +337,18 @@ public class LoggingAuditTrailFilterTests extends ESTestCase {
.test(new AuditEventMetaInfo(Optional.of(randomFrom(filteredUsers)), Optional.of(randomFrom(filteredRealms)), .test(new AuditEventMetaInfo(Optional.of(randomFrom(filteredUsers)), Optional.of(randomFrom(filteredRealms)),
Optional.of(randomSubsetOf(randomIntBetween(1, filteredRoles.size()), filteredRoles).toArray(new String[0])), Optional.of(randomSubsetOf(randomIntBetween(1, filteredRoles.size()), filteredRoles).toArray(new String[0])),
Optional.of(someIndicesDoNotMatch.toArray(new String[0]))))); Optional.of(someIndicesDoNotMatch.toArray(new String[0])))));
final Optional<String[]> emptyIndices = randomBoolean() ? Optional.empty() : Optional.of(new String[0]);
assertTrue("Matches the filter predicate because of the empty indices.", auditTrail.eventFilterPolicyRegistry.ignorePredicate() assertTrue("Matches the filter predicate because of the empty indices.", auditTrail.eventFilterPolicyRegistry.ignorePredicate()
.test(new AuditEventMetaInfo(Optional.of(randomFrom(filteredUsers)), Optional.of(randomFrom(filteredRealms)), .test(new AuditEventMetaInfo(Optional.of(randomFrom(filteredUsers)), Optional.of(randomFrom(filteredRealms)),
Optional.of(randomSubsetOf(randomIntBetween(1, filteredRoles.size()), filteredRoles).toArray(new String[0])), Optional.of(randomSubsetOf(randomIntBetween(1, filteredRoles.size()), filteredRoles).toArray(new String[0])),
emptyIndices))); Optional.empty())));
assertTrue("Matches the filter predicate because of the empty indices.", auditTrail.eventFilterPolicyRegistry.ignorePredicate()
.test(new AuditEventMetaInfo(Optional.of(randomFrom(filteredUsers)), Optional.of(randomFrom(filteredRealms)),
Optional.of(randomSubsetOf(randomIntBetween(1, filteredRoles.size()), filteredRoles).toArray(new String[0])),
Optional.of(new String[0]))));
assertTrue("Matches the filter predicate because of the empty indices.", auditTrail.eventFilterPolicyRegistry.ignorePredicate()
.test(new AuditEventMetaInfo(Optional.of(randomFrom(filteredUsers)), Optional.of(randomFrom(filteredRealms)),
Optional.of(randomSubsetOf(randomIntBetween(1, filteredRoles.size()), filteredRoles).toArray(new String[0])),
Optional.of(new String[] { null }))));
} }
public void testTwoPolicyPredicatesWithMissingFields() throws Exception { public void testTwoPolicyPredicatesWithMissingFields() throws Exception {