Fix NPE in Logfile Audit Filter (#38120) (#38273)

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

Author: albertzaharovits

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

@@ -52,6 +52,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.TreeMap;
 import java.util.function.Function;
@@ -832,6 +833,7 @@ static final class EventFilterPolicy {
         EventFilterPolicy(String name, Predicate<String> ignorePrincipalsPredicate, Predicate<String> ignoreRealmsPredicate,
                 Predicate<String> ignoreRolesPredicate, Predicate<String> ignoreIndicesPredicate) {
             this.name = name;
+            // "null" values are "unexpected" and should not match any ignore policy
             this.ignorePrincipalsPredicate = ignorePrincipalsPredicate;
             this.ignoreRealmsPredicate = ignoreRealmsPredicate;
             this.ignoreRolesPredicate = ignoreRolesPredicate;
@@ -881,8 +883,10 @@ private static List<String> emptyStringBuildsEmptyAutomaton(List<String> l) {
          * predicate of the corresponding field.
          */
         Predicate<AuditEventMetaInfo> ignorePredicate() {
-            return eventInfo -> ignorePrincipalsPredicate.test(eventInfo.principal) && ignoreRealmsPredicate.test(eventInfo.realm)
-                    && eventInfo.roles.get().allMatch(ignoreRolesPredicate) && eventInfo.indices.get().allMatch(ignoreIndicesPredicate);
+            return eventInfo -> eventInfo.principal != null && ignorePrincipalsPredicate.test(eventInfo.principal)
+                    && 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
@@ -970,8 +974,10 @@ static final class AuditEventMetaInfo {
             // conditions on the `principal` and `realm` fields
             // 2. reusability of the AuditEventMetaInfo instance: in this case Streams have
             // 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.indices = () -> indices.filter(i -> i.length != 0).map(Arrays::stream).orElse(Stream.of(""));
+            this.roles = () -> roles.filter(r -> r.length > 0).filter(a -> Arrays.stream(a).anyMatch(Objects::nonNull))
+                    .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) {

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

@@ -82,6 +82,78 @@ public void init() throws Exception {
         }).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 {
         final Logger logger = CapturingLogger.newCapturingLogger(Level.INFO, null);
         final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
@@ -265,11 +337,18 @@ public void testSingleCompleteWithEmptyFieldPolicyPredicate() throws Exception {
                 .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(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()
                 .test(new AuditEventMetaInfo(Optional.of(randomFrom(filteredUsers)), Optional.of(randomFrom(filteredRealms)),
                         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 {