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

Remove TLS requirement for alerting when security is enabled #115234

Merged
merged 8 commits into from
Oct 19, 2021

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Oct 15, 2021

Resolves #111721

With elastic/elasticsearch#76801 now merged, alerting now works without TLS being enabled. The remaining pieces for us to change are our documentation and UI messages whenever Kibana fails to create API keys.

Note that our logic is still necessary when xpack.security.authc.api_key.enabled: false is set. But this is less likely than TLS not being set up.

The PR does the following:

  • Update documentation to remove set up requirement of TLS
  • Update documentation to mention API keys cannot be disabled, otherwise alerting will not work
  • Update UI messages to mention API keys must be enabled (instead of TLS)

Follow ups:

Previews:

API keys disabled message Screen Shot 2021-10-19 at 10 56 27 AM
API keys and encryption key message Screen Shot 2021-10-19 at 10 50 05 AM
Encription key message Screen Shot 2021-10-19 at 10 54 23 AM
  • Note: The messages are the same in the create rule flyout. To test, you'll need to access the flyout from an app.

Testing steps

Explicitly disabling API keys

  1. yarn es snapshot -E xpack.security.authc.api_key.enabled: false and yarn start
  2. Notice the new banners when navigating to the rules management page

Ephemeral encryption key + disabling API keys

  1. Change https://github.com/elastic/kibana/blob/master/x-pack/plugins/encrypted_saved_objects/server/plugin.ts#L61 to const canEncrypt = false;
  2. yarn es snapshot -E xpack.security.authc.api_key.enabled=false and yarn start
  3. Notice the new banners when navigating to the rules management page

@mikecote mikecote added release_note:enhancement v8.0.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.16.0 Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework labels Oct 15, 2021
@mikecote mikecote self-assigned this Oct 15, 2021
@@ -17,8 +17,7 @@ If you are using an *on-premises* Elastic Stack deployment:

If you are using an *on-premises* Elastic Stack deployment with <<using-kibana-with-security, *security*>>:

* You must enable Transport Layer Security (TLS) for communication <<configuring-tls-kib-es, between {es} and {kib}>>. {kib} alerting uses <<api-keys, API keys>> to secure background rule checks and actions, and API keys require {ref}/configuring-tls.html#tls-http[TLS on the HTTP interface]. A proxy will not suffice.
* If you have enabled TLS and are still unable to access Alerting, ensure that you have not {ref}/security-settings.html#api-key-service-settings[explicitly disabled API keys].
* If you are unable to access Alerting, ensure that you have not {ref}/security-settings.html#api-key-service-settings[explicitly disabled API keys].
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gchaps to review this change. It was the following before:

If you have enabled TLS and are still unable to access Alerting, ensure that you have not {ref}/security-settings.html#api-key-service-settings[explicitly disabled API keys].

id="xpack.triggersActionsUI.components.healthCheck.tlsErrorTitle"
defaultMessage="You must enable Transport Layer Security and API keys"
id="xpack.triggersActionsUI.components.healthCheck.apiKeysDisabledErrorTitle"
defaultMessage="You must enable API keys"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gchaps to review this change. It was the following before:

You must enable Transport Layer Security and API keys

defaultMessage:
'Alerting relies on API keys, which require TLS between Elasticsearch and Kibana. ',
{i18n.translate('xpack.triggersActionsUI.components.healthCheck.apiKeysDisabledError', {
defaultMessage: 'Alerting relies on API keys. ',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gchaps to review this change. It was the following before:

Alerting relies on API keys, which require TLS between Elasticsearch and Kibana.

Copy link
Contributor

Choose a reason for hiding this comment

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

This text now just repeats the title. Is there something more you can add? Or can it simply be:

You must enable API keys to use Alerting
Learn how

I'm also wondering if we should make the two messages parallel:

Additional setup required
You must enable API keys to use Alerting. Learn how.

{i18n.translate(
'xpack.triggersActionsUI.components.healthCheck.apiKeysDisabledErrorAction',
{
defaultMessage: 'Learn how to enable API keys.',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gchaps to review this change. It was the following before:

Learn how to enable TLS.

{i18n.translate(
'xpack.triggersActionsUI.components.healthCheck.apiKeysAndEncryptionError',
{
defaultMessage: 'You must enable API keys and configure an encryption key. ',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gchaps to review this change. It was the following before:

You must enable Transport Layer Security between Kibana and Elasticsearch and configure an encryption key in your kibana.yml file.

@mikecote mikecote marked this pull request as ready for review October 18, 2021 14:09
@mikecote mikecote requested review from a team as code owners October 18, 2021 14:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

LGTM functionally!

@gchaps
Copy link
Contributor

gchaps commented Oct 18, 2021

Here's the text to make the messages parallel:

Additional setup required
You must enable API keys to use Alerting. Learn more.

Additional setup required
You must enable API keys and configure an encryption key to use Alerting. Learn more.

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

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

UI copy lgtm

@mikecote mikecote added the auto-backport Deprecated - use backport:version if exact versions are needed label Oct 19, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
triggersActionsUi 789.4KB 788.8KB -668.0B

Page load bundle

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

id before after diff
core 302.9KB 303.0KB +81.0B

History

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

cc @mikecote

@mikecote mikecote merged commit c1e9fc9 into elastic:master Oct 19, 2021
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 115234

mikecote added a commit that referenced this pull request Oct 19, 2021
#115622)

* Initial commit

* Fix CI failures

* Fix test label

* Update messages

* Cleanup translations

* Fix type check

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	docs/user/alerting/alerting-setup.asciidoc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Transport Layer Security (TLS) requirement for alerting when security is enabled
7 participants