Audit trail filtering rules cannot be deleted in the usual way other cluster settings are removed.

[markharwood]

A user reported an issue where they wanted to delete an existing audit filter rule by changing the cluster setting to null (the usual way cluster settings are effectively removed).
Unfortunately the net effect was to create a rule that filtered all audit log entries.

I reproduced this by adding this test to LoggingAuditTrailFilterTests

public void testNullPolicyDoesNotMatchEvent() 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);
    settingsBuilder.putNull("xpack.security.audit.logfile.events.ignore_filters.userPolicy.users");
    final LoggingAuditTrail auditTrail = new LoggingAuditTrail(settingsBuilder.build(), clusterService, logger, threadContext);
    final User unfilteredUser = new User("Fred");
    // Null setting should not match
    assertFalse("Shouldn't match users wiih a null rule",
            auditTrail.eventFilterPolicyRegistry.ignorePredicate()
                    .test(new AuditEventMetaInfo(Optional.of(unfilteredUser), Optional.empty(), Optional.empty(), Optional.empty())));
}

[elasticmachine]

Pinging @elastic/es-security (Team:Security)

[ywangd]

Thanks for reporting this issue. It could be hard to explain, but this behaviour is somewhat intended/expected, but also flawed.

A ignore policy is comprised of 4 rules, users, realms, roles and indices. And the documentation says:

A policy matches an event if all the rules comprising it match the event.

Because one can configure only one rule of a policy and expect it to work, e.g.:

xpack.security.audit.logfile.events.ignore_filters.userPolicy.users: ['foo']
xpack.security.audit.logfile.events.ignore_filters.userPolicy.roles: ['foo-role']

The above configuration means that as long as the user is foo and role is foo-role, the event will be ignored regardless of the values of other rules. That is, the other rules (realms, indices), while being missing, essentially mean they are "match-all" and this is the default behaviour. In another word, removing a rule by giving it a null value resets it to the default behaviour, which is match everything (instead of match nothing). For example, if we set the roles to null, i.e:

PUT _cluster/settings
{
  "xpack.security.audit.logfile.events.ignore_filters.userPolicy.roles": null
}

This makes the ignore policy to only look at the username, as long as it is foo, the event will be ignored. The roles rule, by having null value, becomes "match-all".

With above being said, there is an issue when a policy only has a single rule, e.g. user:

xpack.security.audit.logfile.events.ignore_filters.userPolicy.users: ['foo']

When it is set to the null value by:

PUT _cluster/settings
{
  "xpack.security.audit.logfile.events.ignore_filters.userPolicy.users": null
}

It makes the whole policy match everything. This is technically correct. However, it no longer shows up in the response of GET _cluster/settings and that is very confusing, because there is an invisible ignore policy that match everything but you cannot see it in the settings. Worse, you cannot easily remove it, but have to set to match-none with something like:

PUT _cluster/settings
{
  "transient": {
    "xpack.security.audit.logfile.events.ignore_filters.userPolicy.users": []
  },
}

This is something we can improve, by either automatically remove the policy once all rules are null or display the policy in the get settings response or something else.

[tvernum]

This is technically correct.

I think the behaviour is explainable, but I would argue quite strongly that it is not correct (technically or otherwise).
We really should fix this.

Once there are no more settings for a filter policy the policy does not exist anymore (in the settings) and should not be applied.

[bytebilly]

I agree we should fix the behavior, since I don't think it is intentional and it is very counterintuitive and hard to debug from a user perspective.

[ppf2]

We really should fix this.

+1 Given that the recommended and commonly used way to reset Elasticsearch settings is to set them to null, it's easy for users to inadvertently break audit logging completely (and silently) simply by settings an ignore policy to null. Restarting all nodes in the cluster is a heavy workaround esp. for clusters with large amounts of nodes/data that can take many hours to recover.

For those encountering this issue, you will see the following log entry on the nodes indicating that you have set the ignore filter policy to match "*" when you didn't actually set it to wildcard yourself :)

[instance-0000000035] updating [xpack.security.audit.logfile.events.ignore_filters.realms_policy.realms] from [["my_api_key"]] to [["*"]]