[ResponseOps][actions] add config for allow-listing email address domains #129001

pmuellr · 2022-03-31T05:22:41Z

Summary

docs preview:

Adds a new configuration setting for the actions plugin, xpack.actions.email.domain_allowlist, which is an array of domain name strings which are allowed to be sent emails by the email connector. If this setting isn't used, there is no change to the way the email connector works before this PR - email addresses are generally only validated when the connector is executed, by node-mailer. If this setting is used, for each email connector, each email address specified in the from property in config, and to, cc, and bcc properties in params, will be validated, and their domains checked against the allow list. If any email is invalid (so we can't determine the domain), or it's domain is not in the allow list, that's considered an error condition, when the validation is run:

a connector create will fail if an invalid/not allowed email is used in the config from property
a rule create/update will fail if an invalid/not allowed email is used in the email connector params to, cc, or bcc of any of it's actions
and a connector run will fail if any of from, to, cc, or bcc are invalid/not allowed.

This adds a new dependency on an npm package email-addresses, which we use to extract the domain from an email address. This also validates the email address, first. This seems like a well-maintained package and has no dependencies.

We expose the config setting down into the UX via exposeToBrowser. A chat with Kibana Security indicated they didn't feel like this was a security concern. Basically, if we WANT to be able to validate email addresses in the UX, we will have to expose SOME way of validating email addresses via HTTP, so even the most obscure way of doing this (for example, exposing a new server endpoint to validate email addresses) will end up "leaking" which domains are actually allowed. If this is still felt to be a security concern, we can rip out everything on the UX side, and let the validation only happen on the server. The same error conditions will work, but the UX is not quite as nice - "Error saving connector: blah blah" instead of nice error indicators in appropriate fields in the UX.

This required adding a new public plugin for actions - it didn't have one before. The common folder for actions is also where the new validation logic is implemented, where it calls into the email-addresses package, so it's shared between client and server.

Creating this new plugin caused some other issues, with some ServiceNow public code that was referencing the server plugin types, but using a // eslint-disable-next-line @kbn/eslint/no-restricted-paths comment to disable the compile time warning. With the new public plugin, this changed to a runtime warning message. So, the relevant types were extracted into common, and then the references to them changed. Seems like a good cleanup, and eliminated some //eslint-disable usages.

For function tests, security_and_spaces was left alone, so is the tester for NOT using the new config. spaces_only now uses the new setting, and has added tests to ensure the code works as expected.

To Verify

Add the new config, for example:

xpack.actions.email.domain_allowlist: ['elastic.co']

Then try creating a connector with a from with a different email address, and with an elastic.co address. Should only work for elastic.co. After creating a connector, try using the test page and using different values for to, cc, and bcc. Should only work with elastic.co addresses. Then add the email connector to an existing rule, and try setting the to, cc, and bcc to different values to see they work or generate errors.

Note, the email doesn't have to be a "live" one, you can use Gmail but give it bogus userid / password. In which case if you see a failure from Gmail, then the email would have validated and sent :-).

You can also test using previously created email connectors without the setting, then turn the setting on, and edit the connector to see the "new" errors.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list

Release Note

Adds a new configuration setting for the actions plugin, xpack.actions.email.domain_allowlist, which is an array of domain name strings which are allowed to be sent emails by the email connector.

…ains resolves elastic#126944 Adds a new configuration setting for the actions plugin, xpack.actions.email.domain_allowlist, which is an array of domain name strings which are allowed to be sent emails by the email connector.

pmuellr · 2022-04-21T15:38:41Z

packages/kbn-optimizer/limits.yml

@@ -1,5 +1,6 @@
 pageLoadAssetSize:
  advancedSettings: 27596
+  actions: 20000


The actual value was ~16K, figured I'd add some slop, but not quite sure if that's ok

Yeah, that's fine

pmuellr · 2022-04-21T15:41:27Z

x-pack/plugins/actions/common/servicenow_config.ts

@@ -0,0 +1,56 @@
+/*


These definitions were originally in a server directory, and accessed from a public directory, with an eslint directive to not generate a warning about that. However, when the actions plugin grew a public plugin, things got worse - we started getting a runtime message when Kibana started. So I extracted all the stuff from the server directory that the public code needed, and moved it in here.

pmuellr · 2022-04-21T15:43:49Z

x-pack/plugins/actions/server/builtin_action_types/email.test.ts

@@ -454,21 +482,9 @@ describe('execute()', () => {
        "status": "ok",
      }
    `);
+    delete sendEmailMock.mock.calls[0][1].configurationUtilities;


This is where I delete the configuration utilities from the calls structure, so we no longer compare it, so we don't have to change this code when we add new methods to the configuration utilities. I do this in some other tests that had the same pattern.

pmuellr · 2022-04-21T15:46:34Z

x-pack/plugins/triggers_actions_ui/kibana.json

  "configPath": ["xpack", "trigger_actions_ui"],
  "extraPublicDirs": ["public/common", "public/common/constants"],
-  "requiredBundles": ["alerting", "esUiShared", "kibanaReact", "kibanaUtils"]
+  "requiredBundles": ["alerting", "esUiShared", "kibanaReact", "kibanaUtils", "actions"]


I figured we need to add the actions bundle here, since it's now referencing code there. Though I think it worked at dev time without it - or maybe I was imagining that.

pmuellr · 2022-04-21T15:48:30Z

...ers_actions_ui/public/application/components/builtin_action_types/es_index/es_index.test.tsx


 const ACTION_TYPE_ID = '.index';
 let actionTypeModel: ActionTypeModel;

 beforeAll(() => {
  const actionTypeRegistry = new TypeRegistry<ActionTypeModel>();
-  registerBuiltInActionTypes({ actionTypeRegistry });
+  registerBuiltInActionTypes({ actionTypeRegistry, services: registrationServicesMock });


registerBuiltInActionTypes() takes an additional parameter now - services - so all the connector type tests need this change; more below ...

elasticmachine · 2022-04-21T15:50:35Z

Pinging @elastic/response-ops (Team:ResponseOps)

spalger · 2022-04-21T15:51:24Z

packages/kbn-optimizer/limits.yml

@@ -1,5 +1,6 @@
 pageLoadAssetSize:
  advancedSettings: 27596
+  actions: 20000


Yeah, that's fine

mikecote

Changes LGTM! Tested locally the UI form validation in rule actions & connector flyouts. Also, tested implementing the allow_list after creating a rule and connector and saw the runtime validation prevent emails from sending out. Great work 👍

mikecote · 2022-04-21T16:58:45Z

docs/settings/alert-action-settings.asciidoc

@@ -125,6 +125,11 @@ The contents of a PEM-encoded certificate file, or multiple files appended
 into a single string.  This configuration can be used for environments where
 the files cannot be made available.

+`xpack.actions.email.domain_allowlist`::
+A list of allowed email domains which can be used with the email connector.  When this setting is not used, all email domains are allowed.  When this setting is used, if any email is attempted to be sent that includes an addressee with an email domain that is not in the allowlist, the run of the connector will fail with a message indicating the emails not allowed.


Do we need to add a mention about the from email domain as well?

Ah yes, we do. I realized I hadn't dealt with from this week, so added it in as well, but forgot to update this.

pmuellr · 2022-04-25T12:21:11Z

For some reason the issue for this PR mentioned in the summary wasn't auto-linked in the issue itself; let's try it manually -here's the originating issue: #126944

ymao1

Looks great! Verified adding the domain allowlist works as expected for existing and new connectors. Left some comments about docs and more specificity in the error message.

ymao1 · 2022-04-25T13:15:57Z

docs/settings/alert-action-settings.asciidoc

@@ -125,6 +125,11 @@ The contents of a PEM-encoded certificate file, or multiple files appended
 into a single string.  This configuration can be used for environments where
 the files cannot be made available.

+`xpack.actions.email.domain_allowlist`::


The UI when creating a connector shows Email address <email> is not allowed. if the from is not allowed with a link to the email action type: https://www.elastic.co/guide/en/kibana/master/email-action-type.html but there is no mention of the domain allowlist on that page. Should we add a mention or a link to this allowlist entry?

Ya, good idea, will add a link to that.

x-pack/plugins/actions/server/builtin_action_types/email.ts

jportner · 2022-04-25T15:12:15Z

Reviewing now!

jportner

I reviewed the integration test change for the newly-exposed config setting -- LGTM.

I also took the liberty of reviewing the validation itself. I have some suggestions below!

docs/settings/alert-action-settings.asciidoc

test/plugin_functional/test_suites/core_plugins/rendering.ts

x-pack/plugins/actions/common/mustache_template.ts

x-pack/plugins/actions/common/validate_email_addresses.ts

x-pack/plugins/triggers_actions_ui/public/plugin.ts

pmuellr · 2022-04-26T12:43:08Z

@elasticmachine merge upstream

jportner

LGTM, thanks for the changes!

kibana-ci · 2022-04-26T13:56:41Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`actions`	-	12	+12
`triggersActionsUi`	365	360	-5
total			+7

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`actions`	128	191	+63

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`triggersActionsUi`	712.4KB	711.9KB	-580.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`actions`	10	11	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`actions`	-	15.0KB	+15.0KB
`triggersActionsUi`	100.0KB	99.4KB	-700.0B
total			+14.3KB

Unknown metric groups

API count

id	before	after	diff
`actions`	128	195	+67

ESLint disabled line counts

id	before	after	diff
`triggersActionsUi`	170	167	-3

Total ESLint disabled count

id	before	after	diff
`triggersActionsUi`	176	173	-3

History

💚 Build #40424 succeeded 347c19c
💚 Build #39832 succeeded 7e8cd7f
💚 Build #39651 succeeded 56da55d78f6b62c9a1f7f659d0e11e23aaa62871
💔 Build #39611 failed 74e277fe4a8dffd1a1d7554e6ba3fe8a10f4b045
💚 Build #39432 succeeded 6af357154118dcfe7ab236c5e2ec22e3d9f87e38
💔 Build #39322 failed 2b9687e8da92c2dabf5f52d1c1a1cdc42740ca23

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…ains (elastic#129001) resolves elastic#126944 Adds a new configuration setting for the actions plugin, xpack.actions.email.domain_allowlist, which is an array of domain name strings which are allowed to be sent emails by the email connector. backport of elastic#129001

…ess domains (#129001) (#131014) resolves #126944 Adds a new configuration setting for the actions plugin, xpack.actions.email.domain_allowlist, which is an array of domain name strings which are allowed to be sent emails by the email connector. backport of #129001 * additional changes after merge conflicts during cherry pick for backport

…ains (elastic#129001) resolves elastic#126944 Adds a new configuration setting for the actions plugin, xpack.actions.email.domain_allowlist, which is an array of domain name strings which are allowed to be sent emails by the email connector.

pmuellr mentioned this pull request Mar 31, 2022

Add allowed host support for email connector recipients (to, cc, bcc) #126944

Closed

5 tasks

pmuellr force-pushed the actions/126944-email-allowlist branch 2 times, most recently from 854f486 to b80b9c2 Compare April 1, 2022 20:44

pmuellr force-pushed the actions/126944-email-allowlist branch 3 times, most recently from 6ff5116 to 3c4b94c Compare April 18, 2022 17:59

pmuellr force-pushed the actions/126944-email-allowlist branch 2 times, most recently from 74e277f to 56da55d Compare April 20, 2022 22:53

pmuellr force-pushed the actions/126944-email-allowlist branch from 45f4962 to 7e8cd7f Compare April 21, 2022 14:09

pmuellr added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) release_note:enhancement Feature:Actions/Framework Issues related to the Actions Framework v8.3.0 v7.17.4 labels Apr 21, 2022

pmuellr commented Apr 21, 2022

View reviewed changes

pmuellr marked this pull request as ready for review April 21, 2022 15:50

pmuellr requested review from a team as code owners April 21, 2022 15:50

spalger approved these changes Apr 21, 2022

View reviewed changes

packages/kbn-optimizer/limits.yml

@@ -1,5 +1,6 @@

pageLoadAssetSize:

advancedSettings: 27596

actions: 20000

Copy link

Contributor

spalger Apr 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's fine

mikecote self-requested a review April 21, 2022 16:53

mikecote approved these changes Apr 21, 2022

View reviewed changes

ymao1 approved these changes Apr 25, 2022

View reviewed changes

jportner self-requested a review April 25, 2022 15:12

jportner reviewed Apr 25, 2022

View reviewed changes

changes based on PR reviews

347c19c

pmuellr requested a review from a team as a code owner April 26, 2022 06:19

Merge branch 'main' into actions/126944-email-allowlist

599093f

jportner approved these changes Apr 26, 2022

View reviewed changes

pmuellr merged commit 6ad418b into elastic:main Apr 26, 2022

pmuellr mentioned this pull request Apr 26, 2022

[ResponseOps][actions] connector validation during execution does not generate event log doc #130984

Closed

lcawl mentioned this pull request Apr 26, 2022

[DOCS] Edits xpack.actions.email.domain_allowlist #130993

Merged

This was referenced Apr 26, 2022

WOOPS - on wrong branch when I pushed - [7.17] [ResponseOps][actions] add config for allow-listing email address domains (#129001) #131013

Closed

[7.17] [ResponseOps][actions] add config for allow-listing email address domains (#129001) #131014

Merged

pmuellr added the backported label Apr 28, 2022

gmmorris added the Feature:Actions/ConnectorTypes Issues related to specific Connector Types on the Actions Framework label Jun 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ResponseOps][actions] add config for allow-listing email address domains #129001

[ResponseOps][actions] add config for allow-listing email address domains #129001

pmuellr commented Mar 31, 2022 •

edited

Loading

pmuellr Apr 21, 2022

spalger Apr 21, 2022

pmuellr Apr 21, 2022

pmuellr Apr 21, 2022 •

edited

Loading

pmuellr Apr 21, 2022

pmuellr Apr 21, 2022

elasticmachine commented Apr 21, 2022

spalger Apr 21, 2022

mikecote left a comment

mikecote Apr 21, 2022

pmuellr Apr 21, 2022

pmuellr commented Apr 25, 2022

ymao1 left a comment

ymao1 Apr 25, 2022

pmuellr Apr 25, 2022

jportner commented Apr 25, 2022

jportner left a comment

pmuellr commented Apr 26, 2022

jportner left a comment

kibana-ci commented Apr 26, 2022

API count

ESLint disabled line counts

Total ESLint disabled count

[ResponseOps][actions] add config for allow-listing email address domains #129001

[ResponseOps][actions] add config for allow-listing email address domains #129001

Conversation

pmuellr commented Mar 31, 2022 • edited Loading

Summary

To Verify

Checklist

Release Note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmuellr Apr 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Apr 21, 2022

Choose a reason for hiding this comment

mikecote left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmuellr commented Apr 25, 2022

ymao1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jportner commented Apr 25, 2022

jportner left a comment

Choose a reason for hiding this comment

pmuellr commented Apr 26, 2022

jportner left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 26, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

pmuellr commented Mar 31, 2022 •

edited

Loading

pmuellr Apr 21, 2022 •

edited

Loading