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.

Sign up for GitHub

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

Jump to bottom

[Monitoring] Stop using usingEphemeralEncryptionKey #88884

Merged
merged 2 commits into from
Jan 21, 2021
Merged

[Monitoring] Stop using usingEphemeralEncryptionKey #88884

merged 2 commits into from
Jan 21, 2021

Conversation

chrisronline
Copy link
Contributor

Resolves #81618

@chrisronline chrisronline added review Team:Monitoring Stack Monitoring team release_note:skip Skip the PR/issue when compiling release notes v7.12.0 8.0.0 labels Jan 20, 2021
@chrisronline chrisronline self-assigned this Jan 20, 2021
@chrisronline chrisronline requested a review from a team January 20, 2021 18:20
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@chrisronline chrisronline mentioned this pull request Jan 20, 2021
Closed
@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@wylieconlon wylieconlon added v8.0.0 and removed 8.0.0 labels Jan 20, 2021
azasypkin
azasypkin approved these changes Jan 21, 2021
View reviewed changes
Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

x-pack/plugins/monitoring/server/lib/elasticsearch/verify_alerting_security.ts
Comment on lines 44 to 47
return {
isSufficientlySecure: !isSecurityEnabled || (isSecurityEnabled && isTLSEnabled),
hasPermanentEncryptionKey: !encryptedSavedObjects?.usingEphemeralEncryptionKey,
hasPermanentEncryptionKey: Boolean(encryptedSavedObjects),
};
Copy link
Member

@azasypkin azasypkin Jan 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

optional nit: after looking at the code that uses isSufficientlySecure and hasPermanentEncryptionKey it seems you can just include Boolean(encryptedSavedObjects) condition to isSufficientlySecure and use only this property everywhere. It also makes sense since there won't be an ephemeral key anymore. But up to you, I hope in the future alerting plugin will expose something like this instead so that you can remove explicit dependency on encryptedSavedObjects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, yea this is pretty much a copy/paste from the alerting code, from here so I'd honestly defer to their judgement here too. We just need a way to detect this scenario and let users know

igoristic
igoristic approved these changes Jan 21, 2021
View reviewed changes
Copy link
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@chrisronline chrisronline merged commit d1e3ee9 into elastic:master Jan 21, 2021
@chrisronline chrisronline deleted the monitoring/encrypted_saved_objects_usage branch January 21, 2021 18:08
@chrisronline chrisronline mentioned this pull request Jan 21, 2021
Merged
chrisronline added a commit that referenced this pull request Jan 21, 2021
@chrisronline @kibanamachine
Stop using usingEphemeralEncryptionKey (#88884) (#89008)
1b882f0 
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igoristic igoristic igoristic approved these changes

@azasypkin azasypkin azasypkin approved these changes

Assignees

@chrisronline chrisronline

Labels
release_note:skip Skip the PR/issue when compiling release notes review Team:Monitoring Stack Monitoring team v7.12.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make encryptedSavedObjects plugin dependency optional and stop relying on usingEphemeralEncryptionKey
6 participants
@chrisronline @elasticmachine @kibanamachine @azasypkin @igoristic @wylieconlon