Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configurations to enable/disable individual built-in action types #52326

Closed
peterschretlen opened this issue Dec 5, 2019 · 8 comments · Fixed by #52967
Closed

Add configurations to enable/disable individual built-in action types #52326

peterschretlen opened this issue Dec 5, 2019 · 8 comments · Fixed by #52967
Assignees
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.6.0

Comments

@peterschretlen
Copy link
Contributor

As a security control for server administrators, provide the ability to enable/disable built-in action types in kibana.yml. For example:

xpack.actions.webhook.enabled
xpack.actions.email.enabled
xpack.actions.slack.enabled
xpack.actions.pagerduty.enabled
xpack.actions.index.enabled
xpack.actions.serverlog.enabled

Disabling an action would prevent it from being registered in x-pack/legacy/plugins/actions/server/builtin_action_types/index.ts

We should consider if we want to expose settings for all action types. Some apps may expect built-in types to be available, and we create the edge case where all of them may be disabled, effectively disabling alerting altogether.

At a minimum I think we should expose xpack.actions.webhook.enabled because of the potential for webhooks to be used for SSRF, it is the mostly likely one people may want to disable.

This may impact the configurations we need to whitelist in cloud #51205

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-services (Team:Stack Services)

@pmuellr
Copy link
Member

pmuellr commented Dec 6, 2019

The function sounds good, but we should base the action names on the names provided by the action type specifically, that way we can easily handle other action types other than the built-in ones provided by the action plugin. I think. If this stays in kibana.yml, we may have constraints on the way we name these (in terms of how we'd enumerate/get values of the values).

One interesting twist with this is that our current action names all start with ., eg .email. Thinking was at the time, actions starting with . are "special", but that probably doesn't make sense at this point. So we may want to "re-name" the built-in actions to remove the . prefix. Otherwise if the scheme was based on action type names, it would be the super-clumsy xpack.actions..webhook.enabled (two dots before webhook).

I think we should also consider whether we want this in kibana.yml, or perhaps the new thought that all this whitelisting-esque stuff should be done via Kibana management/advanced settings. The more settings we add to kibana.yml means more aggravation for customers (edit settings and restart Kibana), as well as legacy issues if we finally do end up with a more "dynamic" story like Kibana advanced settings.

@pmuellr
Copy link
Member

pmuellr commented Dec 9, 2019

As an alternative, we could allow the config to be an array of action types that are disabled (default is they're enabled).

@mikecote
Copy link
Contributor

mikecote commented Dec 9, 2019

As an alternative, we could allow the config to be an array of action types that are disabled (default is they're enabled).

There is one drawback that came to mind. If ever we provide a default list of disabled action types and the user overwrites with ['foo'], they could forget to include the defaults and automatically enable them. I think we're ok for that scenario since we don't have a default set but worth noting if ever we do.

@pmuellr
Copy link
Member

pmuellr commented Dec 10, 2019

Ya, so thinking if we do a list, it should be a list of action types that enabled, allowing for '*' (like whitelistedHosts) that means "enabled all action types", and that would be the default.

Then folks who want to lock down the action types would change that value to the list of action types to enable, other action types would be disabled.

The upside here is that there is no surprise at upgrade time - they had a list of action types enabled, and that list will STILL be enabled, the ones they didn't enable STILL aren't enabled, and new action types will NOT be enabled, which I think is what the expectation would be. I think we'd want to validate the action types entered and warn if they list an action type we don't know of.

@pmuellr
Copy link
Member

pmuellr commented Dec 10, 2019

from #52326 (comment)

Disabling an action would prevent it from being registered in x-pack/legacy/plugins/actions/server/builtin_action_types/index.ts

I'm guessing that may be hard - if they previously had created actions with an enabled action type, and then disabled that action type and restarted, we may have issues when those actions get touched by various things.

Feels safer to do the following for "disabled" action types:

  • new actions cannot be created
  • actions cannot be executed

That would still allow:

  • read
  • update
  • delete

So, cRUDx.

The reason I left update in the allowed list, is you could imagine someone wanting to rename the action, and it seems silly to not allow that.

It also feels like we'll want to provide some kind of live "enabled" state on actions themselves, where if the action type is disabled, this "enabled" state would return "false". But we'd also like a "reason". An action could be disabled because it no longer validates (say after an export/import and it lost it's credentials). Will need to noodle on that a bit more, but feels like:

{
... existing action "state";
enabled: boolean;
disabledReason: string;
}

Feels pretty gross, and of course it could be disabled for MULTIPLE reasons ...

I think we'll also need to augment the service/http API that returns the list of action types to return the disabled state.

@peterschretlen
Copy link
Contributor Author

I'm guessing that may be hard - if they previously had created actions with an enabled action type, and then disabled that action type and restarted, we may have issues when those actions get touched by various things.

I think failing actions are OK. Preventing action types from executing is the purpose of the config. If connectors and actions of that type have been created already, then the type is disabled, the alerts and connectors should error.

It's definitely a blunt instrument, but I think we should keep it simple and support the use case of the server administrator who wants to lock down the types of actions that can be run.

As for whitelist vs blacklist approach, I agree with @pmuellr using whitelist with a default of *. That supports getting new types on upgrade by default, while avoiding upgrade surprises when you have an explicit list and new types are added.

@mikecote
Copy link
Contributor

As for whitelist vs blacklist approach, I agree with @pmuellr using whitelist with a default of *. That supports getting new types on upgrade by default, while avoiding upgrade surprises when you have an explicit list and new types are added.

++ makes sense to me as well.

@bmcconaghy bmcconaghy added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) and removed Team:Stack Services labels Dec 12, 2019
pmuellr added a commit to pmuellr/kibana that referenced this issue Dec 12, 2019
pmuellr added a commit that referenced this issue Dec 17, 2019
…Types (#52967)

* adds per-actionType enablement via config xpack.actions.enabledTypes

resolves: #52326
pmuellr added a commit to pmuellr/kibana that referenced this issue Dec 17, 2019
…Types (elastic#52967)

* adds per-actionType enablement via config xpack.actions.enabledTypes

resolves: elastic#52326
pmuellr added a commit that referenced this issue Dec 17, 2019
…Types (#52967) (#53300)

* adds per-actionType enablement via config xpack.actions.enabledTypes

resolves: #52326
@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.6.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants