elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
69.97k stars 24.75k forks source link

Dynamic audit log filter settings via cluster update API #30060

Open elasticmachine opened 6 years ago

elasticmachine commented 6 years ago

Original comment by @albertzaharovits:

It is expected that getting the audit filtering policies suitable for each environment will require manual tunning. In this case, it would be beneficial to have these settings dynamic, via cluster settings update API.

@joshbressers I have misinformed one of the SAs (I am sorry I don't recall his name) when he asked me about this in the meeting you demoed this functionality. I assumed this magically works....

elasticmachine commented 6 years ago

Original comment by @joshbressers:

We can consider this a new feature request then :)

elasticmachine commented 6 years ago

Original comment by @albertzaharovits:

The request to have an API for audit policies was to facilitate faster iterations over them, so that changing the policies will not require a node restart.

Yet, I am concerned about adding an API for changing the audit log settings. This is a second thought after creating LINK REDACTED, and also covers LINK REDACTED . The problem is that an attacker can use the API to hide his actions. In LINK REDACTED any user with privileges to change cluster settings can mask his actions from being audited by creating a policy ignoring it's future actions. Instead, if the audit settings (incl policies) are inside the elasticsearch.yml file the attacker requires local host privileges, which is sound.

I think we should keep high privileges for changing the audit settings. I could not think of any clear solution to this. Three of the less phantasmagorical solutions I could come up with are: 1) these settings can only be changed 6h after node startup. 2) the elasticsearch.yml file can be monitored for changes, and only keep these settings there. 3) the authorization logic has a wild branch allowing changes only from a localhost remote endpoint.

Ideally there might be other settings requiring this kind of special attention, to have a stronger point when deciding.

/CC @elastic/es-security

elasticmachine commented 6 years ago

Original comment by @jaymode:

I think we should keep high privileges for changing the audit settings.

I agree but I think if you are allowed to update the cluster settings, then you already have high privileges?

Another option could include the ability to ignore dynamic settings in the audit trail via a setting that has to be in the elasticsearch.yml file.

elasticmachine commented 6 years ago

Original comment by @albertzaharovits:

What I mean is that the update API effectively limits the capability of auditing superusers . File system permissions are privileges that an ES application user does not have, hence the 'higher privileges' .

Another option could include the ability to ignore dynamic settings in the audit trail via a setting that has to be in the elasticsearch.yml file.

I like this a lot! I will open a PR 🙂

elasticmachine commented 6 years ago

Original comment by @albertzaharovits:

Another option could include the ability to ignore dynamic settings in the audit trail via a setting that has to be in the elasticsearch.yml file.

We have concluded over Slack that the proposed solutions to ignore dynamic settings are more confusing than useful. Any such solution would effectively reverse the standard priorities (transient > persistent > file) for some settings, for some nodes, if they have been restarted. This would definitely lead to confusion about which settings are actually in effect.

Audit settings remain dynamic as any other setting.

elasticmachine commented 6 years ago

Original comment by @LucaWintergerst:

@albertzaharovits can we think about adding an option to disable changing audit settings dynamically?

I really like having them dynamic for testing purposes but once I have something that works for me I would like to lock them down by setting something like this: xpack.security.audit.logfile.events.ignore_filters.dynamic: false

elasticmachine commented 6 years ago

Original comment by @albertzaharovits:

@LucaWintergerst that's exactly what we thought sounds like a good idea but turned out it is not, and we have filed it. Let me elaborate.

Let's suppose you have iterated over your settings and now you want to pin them down. xpack.security.audit.logfile.events.ignore_filters.dynamic: false should be a static node setting, ie. inside elasticsearch.yml. The settings you want fixed from now on also have to be copied manually from the cluster state to the elasticsearch.yml file. Just to make sure this is clear, you have to put the final settings inside elasticsearch.yml together with ignore_filters.dynamic: false and then restart the nodes (one by one or cluster restart). And there's the rub! You have settings inside the elasticsearch.yml file that override settings from the cluster state, contrary to the canonical priority, but only on nodes that have been restarted. This is messy.

It is not yet certain if this can be done more elegantly. Ideas are welcome!

elasticmachine commented 6 years ago

Original comment by @LucaWintergerst:

@albertzaharovits The only other messy thing I can think of would be that we don't copy the settings to the elasticsearch.yml and instead just pin them in the cluster settings. This would likely also be the first of its kind and it may be too ugly/hard to get working with the way cluster settings work.

  1. dynamically change filters using _cluster/settings
  2. add dynamic: false to elasticsearch.yml
  3. restart node(s)
  4. restrict dynamic changes if at least one node has false set

Not saying that this is in any way better, but that's all I can think of right now

elasticmachine commented 6 years ago

Original comment by @jaymode:

restrict dynamic changes if at least one node has false set

There is a consensus problem since we may not be able to talk to a node or the current node's cluster state lags behind. IMO we'd remove the dynamic configuration ability altogether before implementing anything that ignores the cluster settings.

elasticmachine commented 6 years ago

Original comment by @albertzaharovits:

  1. restrict dynamic changes if at least one node has false set

As @jaymode said, unfortunately there is currently no easy way to check for consensus over the static settings. Cluster settings are validated on master and then broadcasted to nodes.

Maybe someone @elastic/es-distributed has a wiser take on this ?

elasticmachine commented 6 years ago

Original comment by @DaveCTurner:

@jaymode hit the nail on the head: checking to make sure that none of the nodes have dynamic: false requires talking to all the nodes, and any plan that relies on talking to all the nodes is doomed since at any time this might include nodes that are disconnected/shut down/not yet even in existence.

Is it not enough to include all dynamic changes to the audit log filter in the audit log in a manner that cannot be disabled? Yes, an attacker can hide their subsequent steps, but they can't hide that they're hiding their steps.

elasticmachine commented 6 years ago

Original comment by @albertzaharovits:

@DaveCTurner thank you for the clear answer!

Is it not enough to include all dynamic changes to the audit log filter in the audit log in a manner that cannot be disabled? Yes, an attacker can hide their subsequent steps, but they can't hide that they're hiding their steps.

I would say normally it is. For what's worth LINK REDACTED allows live tailoring of the audit trail by any user which has the AUDIT SYSTEM privilege. Trouble is we don't have such fine grained privileges. The same privilege to change any cluster setting is required to change the audit policies. Formally, the cluster state is the resource for which privileges are required. There is an issue LINK REDACTED to have privileges for groups of settings, but this might prove rather complex to get done (although haven't though it through).