-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[actions] allow ssl/tls options to be customized for actions #80120
Comments
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
Looks like we can do this per connector; links from nodemailer -> node for where this would be set:
So we'd create via a secure context via Also wondering if there's a node env var that we could use for this, but like all node env var "hacks", it would apply to everything in Kibana. |
I'd like to expand the scope of this issue from just ciphers on email, to all reasonable ssl/tls options for all actions. It's not quite clear to me yet how we would set these sort of configuration options, but I'd guess we should provide all the sorts of things that you might need to configure for ssl/tls for all the actions - https-based actions would set appropriate axios options, and email would set it's nodemailer options. The full list of options is here: https://nodejs.org/dist/latest-v12.x/docs/api/tls.html#tls_tls_createsecurecontext_options Of those, It seems painful to set these on a per-connector basis - it will mean adding all these options to every action type. Lots of duplicated code. Also painful for customers, having to set these options whenever a new connector is being created. One thought to "centralizing" this a bit more, would be to allow configuration options for host/port which would indicate ssl/tls options to use when xpack.actions.tls.options:
- server: 'foo.bar.com:443'
ca: 'file-with-ca.pem'
ciphers: 'TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256' A structure like that may not be easy to set as environment variables though. |
Similar issue rolled into this issue: #80800. |
To continue the conversation, below I've outlined a few options based on @pmuellr's notes and some earlier conversations. I'm interested at hearing thoughts from @elastic/kibana-alerting-services (and of course @kobelb or @peterschretlen in case there's similarities elsewhere in Kibana or technical perspectives we should think about) before reaching out to Platform or Security team for further feedback (if needed). Option 1: Generic kibana.yml configuration shared between action typesThis option is based on @legrego's comment and @pmuellr's suggestion. Similar to As per @pmuellr's suggestion:
The downside with this approach is adding another step for the user to get started with connectors. They may realize when testing their connector that they need to add custom tls/ssl configuration and fix this by; stopping Kibana, change the configuration, start Kibana again, and test again. They may have already done the same cycle for adding a URL to Also, I'm not sure if a file upload is doable in cloud? The user may have to copy / paste the file content. Option 2: Every action type gets custom configurationAs per @pmuellr's comment
This option wouldn't require a restart of Kibana like in option 1 but adds a lot of steps if the same Option 3: Separate management sectionThis section would be for managing This option has the same benefits as option 2 and would remove repetition between action types. On the down side, this adds technical complexity and flows to creating an action type but would allow users to do everything they need right within Kibana. Option 4: Something else? |
My opinion is option 1 then wait and see what user feedback we get. With that, we can decide if it's worth the extra effort for option 2 or 3. Option 1 also seems to be a common recommendation so far. |
Ignoring Cloud, option 1 makes sense to me. However, as far as I'm aware, ESS will not allow users to upload files for Kibana to use. However, the YAML spec does support binary data, so it's theoretically possible that we could allow users to specify their certificates this way. It'd also be possible to allow end-users to upload the certificates that are stored in an Elasticsearch document, which Kibana would read and supply when initiating these HTTP requests. They could theoretically be stored as part of the action itself. |
This is definitely what I was thinking for option 2 and 3. We can encrypt them with the action. A few questions that can help the conversation:
I may be wrong but I think there was feedback from Watcher where users didn't like configuring actions in the .yml file because they had to restart Elasticsearch or talk to an administrator to apply changes to the .yml file. This may apply for certificates as well? Or maybe not because only administrators should be authorizing usage of these? |
Good question. I'd think they could in theory be shared by multiple connectors. Certainly, if you had multiple connectors pointing to the same service, in multiple spaces, adding the certs to the actions themselves will be painful, since you'd have to copy the data every time you created another connector to the same thing. Hence wanting to have a different place to "share" them.
First off, here's what a ca.crt file
Here's how Github allows you to add new SSH keys, which is a similar-but-different story: https://github.com/settings/ssh/new Since the files are 7-bit clean, I think this often the approach to adding keys in structured formats like this. Via a form. I think that would work for us as well. I believe there are other similar "keys" in the ssl/tls configuration space that we would end up needing to do the same thing for. Presumably these would go in some kind of saved object. Perhaps the saved object would store ALL of the ssl/tls settings, along with a "name" (like above) and a way to match these with a url. We'd then look these up when making an outgoing https request, see if any configs matched the outgoing urls, and if so, add those settings to the outgoing request. Alternatively, we could add a field to every action type, or maybe it's a "built-in" action property, that would be a reference to the SO with the ssl config. No matching required, but customers would need to remember to "set" it every time they created connectors requiring them. A way of magically "matching" these seems like it would probably work. |
Yeah that can make sense, do you think sharing connectors between spaces could help?
The more I read through this and think about it, the more my opinion is siding with option 2 instead of option 1 now. I agree with your thoughts and also agree that we would need some framework / library way to do this because almost all our connectors would need something like this. It should be easy to add-on to a connector flyout and consistent to configure for our users. Maybe a URL component that provides its connector config / secrets in nested JSON or something like that.. |
I don't think it helps, but we should also make sure it doesn't hurt :-). It would mean if we had a separate SO for "ssl/tls config", it would need to be space agnostic, but we'd want that anyway.
Not a fan of option 2, as it means we need to add ssl config bits to ALL the actions (except server log and es index?), complicating them. I guess I'll propose an option 4: kinda like option 3, but instead of direct association of actions -> config (or vice versa), we do it "loosely" based on the URL in the action. When creating an action, once the URL is entered, we could display the ssl/tls config "name" that would end up being used with it (what to do if there are multiple matches!?). When executing the alert, we do the same match, and apply that config. In the ssl/ts config management view, we could also poke through the connectors to see which ones match, and display those. Should probably poke around and see if anyone else does this sort of "magical mapping" of URLs to ssl/tls config, see if there are sharp edges there. And wondering about security concern - we'd probably want a special role to create these, to prevent rando's from adding rogue sites to the "URL matches" in hopes of catching passwords, kinda thing. In fact, we may need to do this kind of loose coupling of actions <-> config, so a rogue user can't explicitly associate an action with a config, but the action is pointed to a rogue site. Thinking ahead a bit more, we could actually have some config for this new ssl/tls config thing, which would allow you to "preload" some configs at startup, which would be an on-prem thing today, since you'd likely have to reference files. |
Option 4 looks reasonable as well. I should have said I'm less in favour of option 1. I think it will comes down to a product question (cc @arisonl) and hopefully we have a better idea what the ratio of users is for needing such feature. |
From my side a +1 with a mixture of option 1 and option 2. |
Ignoring technical complexity, Option 3 seems to give us a good balance of usability and flexibility. If I'm understanding correctly, it could add complexity to the "Create action type" flow, but only if that action type is connecting to a service that doesn't already use a well-trusted CA. The commodity services running on public clouds won't have a need for this flow -- the extra complexity would only be incurred when setting up an action that doesn't use a well-trusted CA.
We've historically used We could consider adding a sub-feature privilege so that administrators can choose which of their action creators are allowed to specify custom TLS options. I worry about Option 1 because of ESS. Option 1 is the "easy way out", but asking a deployment admin to paste a CA into their |
Option 1 doesn't seem like it will scale long-term, given this comment, which I assume means that we may want the same server reference to be able to handle multiple ssl/tls options.
Option 1 is also kinda nasty in that it will require a Kibana reboot, and likely multiple, while getting the non-trivial config sorted out and working. Option 2 doesn't scale real well, in that we're adding a bunch of new properties for most actions, which will involve both UI work and a lot of duplication within the actions themselves. And those UIs are going to get complicated, adding the ability to allow customers to upload / paste their keys/certs in. And they'll have to duplicate that for all relevant actions. Even if we modularize this, code-wise, it's seems like it's still going to be the dog's breakfast in terms of the UX for customers. I think the super-long-term solution will be option 3, where there's a separate management section and encrypted SO's to allow admins to create named SSL/TLS configs. Which would include either having them "upload via the browser" their certs/keys, or entry fields to paste the content in, which then gets stored in encrypted SO's with customer-specified names. We'd then allow one of these configs to be referenced from any action as an additional, optional action field. That basically gives us the per-action flexibility of option 2, without making the action ui / fields a lot more complicated - "pick a config by name" instead of "enter your entire config here (sorry, if you've already done that for another action, you need to do it again tho)". |
In case option 3 was meant like you are describing (which was the part that I didn't understand before) then yes, that would be the way to go. Your description helped a lot, thank you @pmuellr |
I'm not sure the original option 3 was intended to be as I described, but my description just above, is how it feels like it should eventually work anyway. I think you could imagine an option 3 where the management section was somehow updating connectors (nee actions) directly, vs managing a set of separate tls/ssl "config" SOs, so thought I'd clarify at least my thoughts on what option 3 means. |
After chatting with @legrego and @kobelb, this is a feature the alerting team will own. I've asked @gmmorris to come up with a more detailed proposal that clarifies option 3 and 4 as it seems to be the best path forward. I think there's a bigger picture that could be painted and we could consider the following in some sort of phased approach:
|
Moved from |
resolves elastic#91686 The poor email action has not had great success in setting TLS options correctly. Prior to 7.11, it was basically always setting `rejectUnauthorized` to false, so was never validating certificates. Starting in 7.11.0, it started respecting TLS certificates, but there are some simple/test servers in use that use self-signed certificates. The real fix for this will be the resolution of issue elastic#80120 , but until then, this PR does a special-case check if the `secure` option is off (so the email client connects with a plain socket and then upgrades to TLS via STARTTLS) and both the user and password for the server are not set, then it will use `rejectUnauthorized: false`. Otherwise, it uses the global configured value of this setting. This also changes some other cases, where `secure: true` often did not set any `rejectUnauthorized` property at all, and so did not get verified. Now in all cases, `rejectUnauthorized` will be set, and the value will correspond to the globally configured value, except for the special case checked here, and when a proxy is in use (that logic did not change). So it is possible this would break customers, who were using insecure servers and email action worked, but with this fix the connections will be rejected. They should have been rejected all this time though. The work-around for this problem, if we don't implement a fix like this, is that customers will need to set the global `rejectUnauthorized` to `false`, which means NONE of their TLS connections for any actions will be verified. Which seems extreme.
…se (#91760) resolves #91686 The poor email action has not had great success in setting TLS options correctly. Prior to 7.11, it was basically always setting `rejectUnauthorized` to false, so was never validating certificates. Starting in 7.11.0, it started respecting TLS certificates, but there are some simple/test servers in use that use self-signed certificates. The real fix for this will be the resolution of issue #80120 , but until then, this PR does a special-case check if the `secure` option is off (so the email client connects with a plain socket and then upgrades to TLS via STARTTLS) and both the user and password for the server are not set, then it will use `rejectUnauthorized: false`. Otherwise, it uses the global configured value of this setting. This also changes some other cases, where `secure: true` often did not set any `rejectUnauthorized` property at all, and so did not get verified. Now in all cases, `rejectUnauthorized` will be set, and the value will correspond to the globally configured value, except for the special case checked here, and when a proxy is in use (that logic did not change). So it is possible this would break customers, who were using insecure servers and email action worked, but with this fix the connections will be rejected. They should have been rejected all this time though. The work-around for this problem, if we don't implement a fix like this, is that customers will need to set the global `rejectUnauthorized` to `false`, which means NONE of their TLS connections for any actions will be verified. Which seems extreme.
…se (elastic#91760) resolves elastic#91686 The poor email action has not had great success in setting TLS options correctly. Prior to 7.11, it was basically always setting `rejectUnauthorized` to false, so was never validating certificates. Starting in 7.11.0, it started respecting TLS certificates, but there are some simple/test servers in use that use self-signed certificates. The real fix for this will be the resolution of issue elastic#80120 , but until then, this PR does a special-case check if the `secure` option is off (so the email client connects with a plain socket and then upgrades to TLS via STARTTLS) and both the user and password for the server are not set, then it will use `rejectUnauthorized: false`. Otherwise, it uses the global configured value of this setting. This also changes some other cases, where `secure: true` often did not set any `rejectUnauthorized` property at all, and so did not get verified. Now in all cases, `rejectUnauthorized` will be set, and the value will correspond to the globally configured value, except for the special case checked here, and when a proxy is in use (that logic did not change). So it is possible this would break customers, who were using insecure servers and email action worked, but with this fix the connections will be rejected. They should have been rejected all this time though. The work-around for this problem, if we don't implement a fix like this, is that customers will need to set the global `rejectUnauthorized` to `false`, which means NONE of their TLS connections for any actions will be verified. Which seems extreme.
…se (elastic#91760) resolves elastic#91686 The poor email action has not had great success in setting TLS options correctly. Prior to 7.11, it was basically always setting `rejectUnauthorized` to false, so was never validating certificates. Starting in 7.11.0, it started respecting TLS certificates, but there are some simple/test servers in use that use self-signed certificates. The real fix for this will be the resolution of issue elastic#80120 , but until then, this PR does a special-case check if the `secure` option is off (so the email client connects with a plain socket and then upgrades to TLS via STARTTLS) and both the user and password for the server are not set, then it will use `rejectUnauthorized: false`. Otherwise, it uses the global configured value of this setting. This also changes some other cases, where `secure: true` often did not set any `rejectUnauthorized` property at all, and so did not get verified. Now in all cases, `rejectUnauthorized` will be set, and the value will correspond to the globally configured value, except for the special case checked here, and when a proxy is in use (that logic did not change). So it is possible this would break customers, who were using insecure servers and email action worked, but with this fix the connections will be rejected. They should have been rejected all this time though. The work-around for this problem, if we don't implement a fix like this, is that customers will need to set the global `rejectUnauthorized` to `false`, which means NONE of their TLS connections for any actions will be verified. Which seems extreme.
…se (elastic#91760) resolves elastic#91686 The poor email action has not had great success in setting TLS options correctly. Prior to 7.11, it was basically always setting `rejectUnauthorized` to false, so was never validating certificates. Starting in 7.11.0, it started respecting TLS certificates, but there are some simple/test servers in use that use self-signed certificates. The real fix for this will be the resolution of issue elastic#80120 , but until then, this PR does a special-case check if the `secure` option is off (so the email client connects with a plain socket and then upgrades to TLS via STARTTLS) and both the user and password for the server are not set, then it will use `rejectUnauthorized: false`. Otherwise, it uses the global configured value of this setting. This also changes some other cases, where `secure: true` often did not set any `rejectUnauthorized` property at all, and so did not get verified. Now in all cases, `rejectUnauthorized` will be set, and the value will correspond to the globally configured value, except for the special case checked here, and when a proxy is in use (that logic did not change). So it is possible this would break customers, who were using insecure servers and email action worked, but with this fix the connections will be rejected. They should have been rejected all this time though. The work-around for this problem, if we don't implement a fix like this, is that customers will need to set the global `rejectUnauthorized` to `false`, which means NONE of their TLS connections for any actions will be verified. Which seems extreme.
…se (#91760) (#93133) resolves #91686 The poor email action has not had great success in setting TLS options correctly. Prior to 7.11, it was basically always setting `rejectUnauthorized` to false, so was never validating certificates. Starting in 7.11.0, it started respecting TLS certificates, but there are some simple/test servers in use that use self-signed certificates. The real fix for this will be the resolution of issue #80120 , but until then, this PR does a special-case check if the `secure` option is off (so the email client connects with a plain socket and then upgrades to TLS via STARTTLS) and both the user and password for the server are not set, then it will use `rejectUnauthorized: false`. Otherwise, it uses the global configured value of this setting. This also changes some other cases, where `secure: true` often did not set any `rejectUnauthorized` property at all, and so did not get verified. Now in all cases, `rejectUnauthorized` will be set, and the value will correspond to the globally configured value, except for the special case checked here, and when a proxy is in use (that logic did not change). So it is possible this would break customers, who were using insecure servers and email action worked, but with this fix the connections will be rejected. They should have been rejected all this time though. The work-around for this problem, if we don't implement a fix like this, is that customers will need to set the global `rejectUnauthorized` to `false`, which means NONE of their TLS connections for any actions will be verified. Which seems extreme.
…se (#91760) (#93131) resolves #91686 The poor email action has not had great success in setting TLS options correctly. Prior to 7.11, it was basically always setting `rejectUnauthorized` to false, so was never validating certificates. Starting in 7.11.0, it started respecting TLS certificates, but there are some simple/test servers in use that use self-signed certificates. The real fix for this will be the resolution of issue #80120 , but until then, this PR does a special-case check if the `secure` option is off (so the email client connects with a plain socket and then upgrades to TLS via STARTTLS) and both the user and password for the server are not set, then it will use `rejectUnauthorized: false`. Otherwise, it uses the global configured value of this setting. This also changes some other cases, where `secure: true` often did not set any `rejectUnauthorized` property at all, and so did not get verified. Now in all cases, `rejectUnauthorized` will be set, and the value will correspond to the globally configured value, except for the special case checked here, and when a proxy is in use (that logic did not change). So it is possible this would break customers, who were using insecure servers and email action worked, but with this fix the connections will be rejected. They should have been rejected all this time though. The work-around for this problem, if we don't implement a fix like this, is that customers will need to set the global `rejectUnauthorized` to `false`, which means NONE of their TLS connections for any actions will be verified. Which seems extreme.
…se (#91760) (#93132) resolves #91686 The poor email action has not had great success in setting TLS options correctly. Prior to 7.11, it was basically always setting `rejectUnauthorized` to false, so was never validating certificates. Starting in 7.11.0, it started respecting TLS certificates, but there are some simple/test servers in use that use self-signed certificates. The real fix for this will be the resolution of issue #80120 , but until then, this PR does a special-case check if the `secure` option is off (so the email client connects with a plain socket and then upgrades to TLS via STARTTLS) and both the user and password for the server are not set, then it will use `rejectUnauthorized: false`. Otherwise, it uses the global configured value of this setting. This also changes some other cases, where `secure: true` often did not set any `rejectUnauthorized` property at all, and so did not get verified. Now in all cases, `rejectUnauthorized` will be set, and the value will correspond to the globally configured value, except for the special case checked here, and when a proxy is in use (that logic did not change). So it is possible this would break customers, who were using insecure servers and email action worked, but with this fix the connections will be rejected. They should have been rejected all this time though. The work-around for this problem, if we don't implement a fix like this, is that customers will need to set the global `rejectUnauthorized` to `false`, which means NONE of their TLS connections for any actions will be verified. Which seems extreme.
This issue comment notes the need for either per-connection proxy support, or probably better would be a proxy exclusion list of some sort - perhaps a list of hostname patterns / ip-address patterns to ignore? Will need to look at how other products deal with this ... Having a specific "ignore proxy" setting per connector would work, but be annoying, but maybe the best way to start - we might always want that option independent of an exclusion list anyway. |
resolves: elastic#80120 Adds a new Kibana configuration key xpack.actions.customHostSettings which allows per-host configuration of connection settings for https and smtp for alerting actions. Initially this is just for TLS settings, expandable to other settings in the future. The purpose of these is to allow customers to provide server certificates for servers accessed by actions, whose certificate authority is not available publicly. Alternatively, a per-server rejectUnauthorized: false configuration may be used to bypass the verification step for specific servers, but require it for other servers that do not have per-host customization. Support was also added to allow per-host customization of ignoreTLS and requireTLS flags for use with the email action.
resolves: #80120 Adds a new Kibana configuration key xpack.actions.customHostSettings which allows per-host configuration of connection settings for https and smtp for alerting actions. Initially this is just for TLS settings, expandable to other settings in the future. The purpose of these is to allow customers to provide server certificates for servers accessed by actions, whose certificate authority is not available publicly. Alternatively, a per-server rejectUnauthorized: false configuration may be used to bypass the verification step for specific servers, but require it for other servers that do not have per-host customization. Support was also added to allow per-host customization of ignoreTLS and requireTLS flags for use with the email action.
…6630) resolves: elastic#80120 Adds a new Kibana configuration key xpack.actions.customHostSettings which allows per-host configuration of connection settings for https and smtp for alerting actions. Initially this is just for TLS settings, expandable to other settings in the future. The purpose of these is to allow customers to provide server certificates for servers accessed by actions, whose certificate authority is not available publicly. Alternatively, a per-server rejectUnauthorized: false configuration may be used to bypass the verification step for specific servers, but require it for other servers that do not have per-host customization. Support was also added to allow per-host customization of ignoreTLS and requireTLS flags for use with the email action.
…98674) resolves: #80120 Adds a new Kibana configuration key xpack.actions.customHostSettings which allows per-host configuration of connection settings for https and smtp for alerting actions. Initially this is just for TLS settings, expandable to other settings in the future. The purpose of these is to allow customers to provide server certificates for servers accessed by actions, whose certificate authority is not available publicly. Alternatively, a per-server rejectUnauthorized: false configuration may be used to bypass the verification step for specific servers, but require it for other servers that do not have per-host customization. Support was also added to allow per-host customization of ignoreTLS and requireTLS flags for use with the email action.
original description:
See #80120 (comment) below, I'd like to expand this to cover more ssl/tls options and more actions, beyond just cipher lists for email
The text was updated successfully, but these errors were encountered: