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

[Discuss] provide action whitelist information to UI clients #51784

Closed
pmuellr opened this issue Nov 26, 2019 · 24 comments
Closed

[Discuss] provide action whitelist information to UI clients #51784

pmuellr opened this issue Nov 26, 2019 · 24 comments

Comments

@pmuellr
Copy link
Member

pmuellr commented Nov 26, 2019

Currently actions have a plugin configuration that allows customers to provide a list of white-listed hostnames that can be used to access 3rd party services. Currently only webhooks makes use of this, but there's a PR to add whitelisting support for email as well. Slack and Pagerduty are considered "whitelisted by default" for now, for their respective actions.

Unfortunately, we don't have a way of exposing this to the UI at present, though it's possible that it could be passed through via "injected vars" (or the NP equivalent, when we move actions to NP).

But there's another wrinkle with emails. When configuring an email action, you can provide EITHER a host/port, or a "well known service name" (as defined by the nodemailer plugin). The UI won't have access to the nodemailer well known service name list either, nor what hosts those service names end up being dereferenced to. And ... you could imagine a UI that would show all the email services possible, with an indicator of which are whitelisted and which aren't.

Again, it's possible we could somehow get this info into the client via an injected vars type scheme, but I'm wondering if an explicit API to handle this might be the best route. This could then be used by other clients that didn't have special access to the Kibana inner bits that the alerting UI will have.

This would presumably be a GET /api/action/whitelist-info sort of API (just read, no update), that would provide the whitelisted hosts, and perhaps a list of whitelisted email services, and perhaps a list of all the potential email services (including those not whitelisted). Probably design it so that we could add a peer of email (say, issue tracking services) in the future.

We're currently thinking some kind of link to some help describing the service names, and how to add entries to the whitelist, near any UI that would let a customer enter a email host/service may be good enough for now, not certainly not ideal.

@elasticmachine
Copy link
Contributor

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

@pmuellr
Copy link
Member Author

pmuellr commented Nov 26, 2019

I suppose there are some security considerations as well. Should we really even be exposing the whitelist as an API as this issue suggests?

My gut says this isn't a security issue with this, but I would't bet any money that my gut is right ...

@peterschretlen
Copy link
Contributor

There are a few issues this addresses, summarizing some of the discussions to date:

  1. Knowing there is a whitelist: Documentation and UI cues are needed to know that a whitelist exists and that it is being applied to an action. This could be something like a disclaimer in the connector creation with a link to the docs.

  2. Minimize the setup steps : The whitelist is problematic in that it requires two steps to setup an email or webhook action (whitelist & restart kibana, then setup your action)

  3. Validating and handling errors: If an action is setup and it does not match the whitelist entries, it's an error. We could do a validation check using an API like the one suggested, and/or indicate in the error messaging that the error is whitelist related.

Overall we want to balance easy-setup with ability-to-secure.
I'll propose the following:

  1. Make the default whitelist = '*'. I think the whitelist should exist, but we have other levels of control like feature controls to restrict access to actions. Having an empty whitelist is the safest ( it avoids the 'surprise on upgrade' problem when in a minor version upgrade anyone with base 'all' privileges get the ability to create actions automatically ), but it is also the least-friendly.
  2. The whitelist should be applied to all builtin actions that connect externally, including slack and pagerduty. If we have a whitelist we should be consistent and enforce it.
  3. If an action can't be created because it does not satisfy the whitelist, that should be in the create API error message - that error is an expected case - handling/displaying that error is an expectation of action UIs. No validation API or list API is needed.

@pmuellr
Copy link
Member Author

pmuellr commented Dec 2, 2019

Peter's suggestion in the comment above makes sense to me. Only worried about the default being '*', think it we should note this behavior in general setup and/or security docs. And we'd need to document the "known" hosts for PagerDuty, Slack, etc.

@peterschretlen
Copy link
Contributor

Would be curious to get @elastic/kibana-security thoughts on the proposal to whitelist all hosts for actions, and rely on feature controls to restrict access to actions created.

@kobelb
Copy link
Contributor

kobelb commented Dec 2, 2019

Would be curious to get @elastic/kibana-security thoughts on the proposal to whitelist all hosts for actions, and rely on feature controls to restrict access to actions created.

I'm hesitant for us to whitelist all hosts for actions, and release functionality that takes advantage of it in a minor version upgrade. Actions can be abused to perform a SSRF, which depends on the specifics of the actions for how unconstrained this is and what an attacker can do. A considerably large number of users will be authorized to create actions because it's quite common for roles to use the base privileges which will grant access to all current and future features, therefore allowing them to create actions.

@peterschretlen
Copy link
Contributor

The equivalent watcher setting xpack.http.whitelist defaults to * and so I was hoping we could mirror that. Though I suppose the risk is lower with Watcher because watcher requires an explicit cluster privilege.

https://www.elastic.co/guide/en/elasticsearch/reference/7.4/notification-settings.html

xpack.http.whitelist
A list of URLs, that the internal HTTP client is allowed to connect to. This client is used in the HTTP input, the webhook, the slack, pagerduty, and jira actions. This setting can be updated dynamically. It defaults to * allowing everything. Note: If you configure this setting and you are using one of the slack/pagerduty actions, you have to ensure that the corresponding endpoints are whitelisted as well.

@peterschretlen
Copy link
Contributor

I think webhook is probably the only one prone to SSRF? email, slack, pagerduty, log, indexing are not zero-risk, but at least they are not open ended.

If we're primarily worried about SSRF and webhook, what if we turn off webhook by default, and document how to properly secure it before enabling? I would rather do that than force all users to configure the whitelist in order to use actions. I think there are other options like giving base-privielge users read-only access to actions until 8.0, but that also involves an additional setup step and does nothing to mitigate risk after 8.0.

@kobelb
Copy link
Contributor

kobelb commented Dec 2, 2019

I think webhook is probably the only one prone to SSRF? email, slack, pagerduty, log, indexing are not zero-risk, but at least they are not open ended.

I think this really depends on the environment in which Kibana is running, and the particulars of the way the actions are implemented. Kibana is used in countless network topologies with any possibility of internal services being available.

With regard to e-mail, Kibana could access an internal SMTP server which allows spoofing the from address which could be used in a phishing attack. If the SMTP server includes the originating X-Originating-IP header, it could reveal the private IP address of the Kibana server. I'm sure there are other concerns here...

@pmuellr
Copy link
Member Author

pmuellr commented Dec 2, 2019

what if we turn off webhook by default, and document how to properly secure it before enabling?

That is a great start. Webhook is a more dangerous action than others since it allows arbitrary urls and bodies, whereas the other actions typically have "fixed" sets of urls and bodies. Email is likely the next most dangerous, but for that I think that auto-whitelisting the existing nodemailer "well-defined" service hosts would be safe-ish.

So what about then a config enable option for webhooks, false by default. For email, a similar sort of config option allowing auto-whitelisting of nodemailer's "well-known" hosts, true by default. And a general whitelist setting of ["*"] to cover the rest, which we already have as a config property (but currently the default is []).

xpack.actions.whitelistedHosts: ['*']
xpack.actions.webhook.enabled: false
xpack.actions.email.whitelistWellknown: true

The "aggravation" factor here comes from having to restart Kibana to pick up new settings. If I understand correctly, NP plugins can be sensitive to config changes "live" as opposed to just via restarting Kibana. In that world, you probably want a single xpack.actions.whitelistedHosts property be configuring. Not sure when we'll get there (actions as NP plugins), but we could consider these other more elaborate settings to be temporary, until dynamic re-config becomes a possibility. When we get there, we deprecate the webhook- and email- specific configs, and default whitelistedHosts back to [].

@pmuellr
Copy link
Member Author

pmuellr commented Dec 3, 2019

That "richer" set of config in the comment ^^^ is smelling a bit rich to me this morning, and doesn't capture all the secure + least-aggravating cases anyway.

Thinking we just go with the original single xpack.actions.whitelistedHosts: [] for now, let's see how aggravating it gets - hopefully short-term until actions can pick up live config changes.

@kobelb
Copy link
Contributor

kobelb commented Dec 3, 2019

Thinking we just go with the original single xpack.actions.whitelistedHosts: [] for now, let's see how aggravating it gets - hopefully short-term until actions can pick up live config changes.

Will the single xpack.actions.whitelistedHosts setting limit e-mail to the well-known hosts?

@pmuellr
Copy link
Member Author

pmuellr commented Dec 3, 2019

Currently xpack.actions.whitelistedHosts can/should be general across all actions. Prior to this, it was only used with webhook - slack and pagerduty likewise have do no whitelisting at all.

If we want to allow whitelisting the well-known email hosts, for the email action, I think we'd want a separate setting, kinda independent of xpack.actions.whitelistedHosts - eg, xpack.actions.email.whitelistWellKnownServers. If that's set to true, consider the well-known services and their associated hosts to be whitelisted independent of xpack.actions.whitelistedHosts. If set to false, don't consider the well-known services.

service/server x.a.whitelistedHosts x.a.e.whitelistWellKnownServers allowed
* [*] * yes
* [] false no
gmail [] false no
gmail [] true yes
gmail [smtp.gmail.com] * yes
myPrivateServer [] * no
myPrivateServer [myPrivateServer] * yes

@peterschretlen
Copy link
Contributor

OK, it seems like xpack.actions.whitelistedHosts: ['*'] as a default isn't an option without compromising security, at least for a minor version release.

I agree with @pmuellr then, lets go forward with xpack.actions.whitelistedHosts: [] as the default configuration.

I think the xpack.actions.email.whitelistWellKnownServers is an option but I'd rather keep it simple for now and just run everything through a single whitelist.

That brings things back to @pmuellr original concern: given we have a blank whitelist - which means no actions other than logging and indexing are allowed out-of-the box - what APIs can we provide that can show an action's whitelist status to users?

Perhaps a validation API? At least then the affected actions (webhook, email, slack, pagerduty ...) can check against the whitelist proactively when the action is being set up display a warning/messaging before someone clicks the 'save' button and gets an error.

@pmuellr
Copy link
Member Author

pmuellr commented Dec 3, 2019

... What APIs can we provide that can show an action's whitelist status to users?
Perhaps a validation API?

With our server-side CRUD apis, you'll get whitelisting validation on C and U, but not R nor D, which makes sense. A message will get returned from the create/update request, indicating that the relevant server is not in the whitelist (message could be improved from what they are today though).

And then also when an action that was previously created, and passed whitelist validation, but since then, the whitelist has changed to not allow that server, and then the action is executed, it will also fail the validation. An error message returned with appropriate messaging (need to check that we do this, but pretty sure we do).

That's all server-side. If we want the client to be able to make intelligent decisions re: whitelisting without waiting for the create/update http requests to fail, it will need to somehow get the whitelist information, and interpret it. I think we could eventually make this happen, don't think it's required immediately tho.

@pmuellr
Copy link
Member Author

pmuellr commented Dec 3, 2019

For an example of what a whitelisting error might look like in a UI, see the following (which isn't a whitelisting error, but an execution time error with bad user/password, same kind of situation though) - #49219 (comment)

Note, the conversation there is leading to extending the error information returned from action execution, not sure if extending error info for CRUD operations will be required.

@peterschretlen
Copy link
Contributor

It's mostly the client experience I'm worried about.

My concern is UI gives people the impression they can create actions, all the way up through configuring a connector, only to have them click 'save' and get an error message. The error message tells them to edit kibana.yml, whitelist, restart the server and try again. And that's in the best case assuming they have access to kibana.yml and the opportunity to restart. As a user you have been "set up to fail".

If we allow no hosts by default, I think the 'least-worst' experience would be:

  1. I create a new alert in the UI, and configure the trigger settings
  2. The actions I can choose from are 'index' and 'log', Maybe slack, email, webhook and pagerduty are visible options, but are grayed-out and have an information tooltip says I need to setup my whitelist in order to enable these actions.
  3. I either stick with the log or index action and create the alert OR I cancel out and go reconfigure the server / ask your admin, etc. But there is no way to fail.

Say I now have a whitelist setup, and slack, pagerduty, webhook and email are now options.

  1. I can create and configure my alert as before
  2. I select an email action
  3. When I enter the host, Kibana checks the value against the whitelist and lets me know right away if the value is whitelisted. If its not whitelisted, there is a message telling me about the whitelist setting. ( I think this could be done through an _is_whitelisted API rather than the client having to somehow obtain or expose this global list )

@peterschretlen
Copy link
Contributor

peterschretlen commented Dec 3, 2019

cc @mdefazio , perhaps there are better ways to mitigate the impact on the user experience.

@kobelb
Copy link
Contributor

kobelb commented Dec 3, 2019

Elasticsearch has the concept of "production mode" vs "dev mode" that they use for their bootstrap checks. If we could figure out a way to differentiate between "production mode" and "dev mode" for Kibana, perhaps we could provide a looser default whitelist in "dev mode"? I haven't been able to think of a great way to differentiate between "production mode" and "dev mode" for Kibana though...

This is what Elasticsearch does:

Thus, we consider an Elasticsearch node to be in development mode if it can not form a cluster with another machine via a non-loopback address, and is otherwise in production mode if it can join a cluster via non-loopback addresses.

Note that HTTP and transport can be configured independently via http.host and transport.host; this can be useful for configuring a single node to be reachable via HTTP for testing purposes without triggering production mode

One idea I had was to treat Kibana as being in production mode when elasticsearch.hosts includes a non-loopback host. However, this wouldn't work when using a coordinating node on the same server as Kibana, which we've recommended rather extensively before.

@chrisronline
Copy link
Contributor

I think we need to provide something more out-of-the-box than an empty whitelist array, specifically thinking about the email action. In the stack monitoring PR for alerting, we have a dropdown for selecting a known nodemailer email service - it feels like an extremely awkward situation to allow the user to select one of these specific services and then tell them later they can't create the email action because they need to whitelist one of these known email services hosts (which requires access and edits to the kibana config file).

I don't know the right answer on the security side of things, but this empty whitelist (especially in regards to email actions) feels like a step backwards for the user.

@snide
Copy link
Contributor

snide commented Dec 3, 2019

Design request would be to start with * as well. Put bluntly, the experience will be pretty poor if they have to go through this much setup.

I'd go so far as to recommend that we set it as * and annoy them with a toast each session to remind them to change it if we're really that worried (similar to our IE11 warning), rather than have it empty and have portions of Kibana unusable by default.

Main preference beyond that would be to make whitelists configurable through the UI so we could handle all this during an initial Kibana setup screen.

Barring those options, the only way to solve this is by disabling portions of the UI and providing callouts in a bunch of places that link over to documentation, which we can do, but it won't be elegant.

@pmuellr
Copy link
Member Author

pmuellr commented Dec 4, 2019

I'd go so far as to recommend that we set it as * and annoy them with a toast each session to remind them to change

I dunno, that sounds equally aggravating :-)

Let me riff a little on changing the whitelist to [].

changes:

  • add whitelisting checks to pagerduty and slack (fixed URLs for the usual case, AFAICT)
  • ensure error messages relating to whitelisting are actionable (indicate a change to the whitelist config setting is required)
  • add an API which returns the whitelist setting, for use in the UI
    • eventually we can make the verification logic for action config work on server and client since it would have the whitelist setting (not required ATM)
    • UI can make use of this today tho, if the value comes back as [] could be a trigger in the UI to change behavior (disable all actions but index and server log) as well as provide some kind of "hint" to the user that the setting needs to be changed to enable the other actions.
  • we probably need to add a property to action types so they can indicate if they do whitelisting, which the UI could use to determine which actions to enable/disable; could just hard-code it for now though to index/server.log.

@peterschretlen
Copy link
Contributor

Updating the status here after internal discussions:

Let's move forward assuming the whitelist will be empty by default at least for 7.6. This is the most secure scenario and we should design and code for it, regardless of what the default ultimately is. I think @pmuellr suggested changes above all make sense regardless of the default.

In the meantime I'll review the options with the elastic security team for 7.x, as there needs to be a security discussion for other deployment scenarios like ESS. I still think there's room to satisfy both security and usability concerns, but security must be on board before we relax the whitelist.

@pmuellr
Copy link
Member Author

pmuellr commented Dec 10, 2019

The whitelist conversation here has pretty much died down, so I'm going to close it.

I've opened a new umbrella issue for actions whitelisting for Kibana 7.6 - #52595

We can use that for further discussion, or in the individual issues in it's task list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants