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

Don't generate xpack.encryptedSavedObjects.encryptionKey #79943

Closed
LeeDr opened this issue Oct 7, 2020 · 21 comments · Fixed by #81511
Closed

Don't generate xpack.encryptedSavedObjects.encryptionKey #79943

LeeDr opened this issue Oct 7, 2020 · 21 comments · Fixed by #81511
Labels
blocked blocker Feature:Fleet Fleet team's agent central management project Team:QA Team label for QA Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.12.0

Comments

@LeeDr
Copy link

LeeDr commented Oct 7, 2020

If a user doesn't specify xpack.reporting.encryptionKey in their kibana.yml (or keystore), they might lose reports or the queue or reports when they restart Kibana. That doesn't seem too catastrophic.

But if a user doesn't specify xpack.encryptedSavedObjects.encryptionKey and we generate one, and we encrypt savedObjects, they will be partially broken when they restart Kibana. If we can't identify some use case where it makes sense to randomly encrypt some saved object properties we should stop generating one.
Any code that was checking plugins.encryptedSavedObjects.usingEphemeralEncryptionKey can just check if there is a xpack.encryptedSavedObjects.encryptionKey.

The current generated key introduces a risk to users that our Support or Dev won't be able to help with after the fact.

We could potentially find problems by disabling the automatic key generation in a PR and see if anything breaks in tests. We'd also have to make sure the tests don't use the test key of aaaaaaaa's and also that the key isn't in the config file for the test. And even then, we don't restart Kibana during tests, so it wouldn't find some potential problems.

Blocked by:

Decision: we decided to stop generating ephemeral encryption key and drop usingEphemeralEncryptionKey property completely. Once this issue is resolved encryptedSavedObjects will be automatically disabled if encryption key isn't specified and hence all dependent plugins that want to guide their users in this case should make this dependency optional (see issues above).

@LeeDr LeeDr added discuss Team:QA Team label for QA Team Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Fleet Fleet team's agent central management project Team:KibanaTelemetry Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Oct 7, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-qa (Team:QA)

@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-telemetry (Team:KibanaTelemetry)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Feature:Fleet)

@LeeDr LeeDr changed the title [DISCUSS] Don't generate xpack.reporting.encryptionKey [DISCUSS] Don't generate xpack.encryptedSavedObjects.encryptionKey Oct 7, 2020
@mikecote
Copy link
Contributor

From an alerting perspective, we are ok with the removal of generating an encryption key as we disable the alerting framework in such case.

The security team has an issue opened on how to solve this issue and the CLI tool option is currently being explored.

@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@legrego
Copy link
Member

legrego commented Oct 14, 2020

For context, there's an older discussion here which weighs the pros and cons of en ephemeral encryption key: #56448.

@peterschretlen mentions here that he favored the ephemeral key at the time, but this was quite a while ago, so I'm not sure if our thinking has changed.

I'm currently favoring removal of the ephemeral key, especially since users have no recourse if they accidentally setup their instance using such a key. We support key rotation, but only if you know the existing keys. Since we don't expose or persist this key, we aren't able to even offer this. It appears that our solutions are preventing their features from being accessed when an ephemeral key is in use, so the benefit of having this automatically generated is unclear to me.

@azasypkin are you aware of any benefits to the ephemeral key that I'm overlooking here?

@azasypkin
Copy link
Member

azasypkin commented Oct 14, 2020

Any code that was checking plugins.encryptedSavedObjects.usingEphemeralEncryptionKey can just check if there is a xpack.encryptedSavedObjects.encryptionKey.

We won't expose encryption key to other plugins. Although they can still access it in multiple ways, these ways are hacks and we'll remove them as we harden Kibana security.

@azasypkin are you aware of any benefits to the ephemeral key that I'm overlooking here?

I cannot think of any from UX standpoint, but the implementation for ESO consumers may be a bit tricky. I see multiple options, but let me describe just the one that seems to be the easiest, the safest and logically correct:

If encryption key isn't specified in a non-dev mode ESO plugin should be disabled completely since plugin becomes useless and non-functional anyway (assuming config schema allows us to do that, need to double check):

  • It will work out of the box since all dependent plugins will be either disabled by the Core (required dependency) or will know that ESO isn't available (optional dependency).
  • There is no risk that some plugin may ignore "ephemeral" flag and use non-recoverable key to store data.

Here is what users will miss if we go this way without changes in ESO consumers:

server    log   [09:17:44.483] [info][plugins-service] Plugin "alerts" has been disabled since the following direct or transitive dependencies are missing or disabled: [encryptedSavedObjects, actions]
server    log   [09:17:44.496] [info][plugins-service] Plugin "uptime" has been disabled since the following direct or transitive dependencies are missing or disabled: [alerts]
server    log   [09:17:44.499] [info][plugins-service] Plugin "actions" has been disabled since the following direct or transitive dependencies are missing or disabled: [encryptedSavedObjects]
server    log   [09:17:44.501] [info][plugins-service] Plugin "case" has been disabled since the following direct or transitive dependencies are missing or disabled: [actions]
server    log   [09:17:44.502] [info][plugins-service] Plugin "infra" has been disabled since the following direct or transitive dependencies are missing or disabled: [alerts]
server    log   [09:17:44.503] [info][plugins-service] Plugin "ingestManager" has been disabled since the following direct or transitive dependencies are missing or disabled: [encryptedSavedObjects]
server    log   [09:17:44.504] [info][plugins-service] Plugin "monitoring" has been disabled since the following direct or transitive dependencies are missing or disabled: [alerts, actions, encryptedSavedObjects]
server    log   [09:17:44.505] [info][plugins-service] Plugin "securitySolution" has been disabled since the following direct or transitive dependencies are missing or disabled: [actions, alerts]
server    log   [09:17:44.522] [info][plugins-service] Plugin "stackAlerts" has been disabled since the following direct or transitive dependencies are missing or disabled: [alerts]

Looks like quite a lot of disabled plugins, and it may result in to suboptimal UX. If any of these plugins want to improve it and guide user they should make ESO an optional dependency and treat its unavailability as they treat usingEphemeralEncryptionKey right now. This will also provide a consistent UX if ESO plugin is explicitly disabled for some reason.

The only concern I have with this approach is that if ESO plugin isn't enabled, no one will be stripping encrypted content when SO APIs are used for types with encrypted content directly (export/import?). Not that it worries me too much, but wanted to mention in case I'm missing some other implications.

@legrego
Copy link
Member

legrego commented Oct 14, 2020

Thanks @azasypkin -

I agree with your assessment. Disabling the ESO plugin today would result in a lot of disabled functionality in Kibana, which makes for a poor getting started experience. Declaring ESO as an optional dependency will at least surface these features in the UI for our users to discover.

@LeeDr
Copy link
Author

LeeDr commented Oct 14, 2020

I think I'm reading all the above to mean that we can't possibly encrypt anything with the ephemeral key because the ESO plugin checks?

If so, I'm good with that and we can close this.

If not, how will we know if anything DOES get encrypted with an ephemeral key? We already had 1 blocker issue because of this "We have a blocking issue for 7.6 where the detection engine stops working after Kibana restarts, because an encryption key is being reset."

@legrego
Copy link
Member

legrego commented Oct 15, 2020

I think I'm reading all the above to mean that we can't possibly encrypt anything with the ephemeral key because the ESO plugin checks?

The ESO plugin knows if the encryption key is ephemeral, but it does not prevent consumers from using it today. It appears that all current consumers stop themselves from using the ephemeral key though, so we effectively aren't using it from what I've seen so far. That won't stop a plugin from trying to use it tomorrow though.

If not, how will we know if anything DOES get encrypted with an ephemeral key?

There are two ways that come to mind based on the current implementation, neither of which are great:

  1. Inspect the Kibana index looking for encrypted saved object attributes
  2. Parse Kibana's audit log, assuming it's been enabled from the start

@azasypkin
Copy link
Member

I think we all are on the same page here and agree that ephemeral key needs to go away. But we need to coordinate that effort with multiple teams that depend on ESO explicitly:

@elastic/kibana-alerting-services @elastic/ingest-management @elastic/stack-monitoring-ui @elastic/siem @elastic/endpoint-app-team would you have any issues with making "encryptedSavedObjects" plugin as an optional dependency (except for security_solution where ESO is already an optional dependency) and stop using usingEphemeralEncryptionKey?

If plugin is available it'd always mean that a proper non-ephemeral encryption key was set, otherwise you may need to limit certain functionality and warn/guide the users. We'll soon have proper docs for you to reference your users to from Kibana or from your app-specific docs.

If there are no objections, let's move forward with the proposed approach. I don't see this change as breaking, so we should be able to make it in a minor:

Step 1. Dependent plugins stop using usingEphemeralEncryptionKey and make "encryptedSavedObjects" an optional dependency
Step 2. "encryptedSavedObjects" plugin removes usingEphemeralEncryptionKey from its setup contract and starts disabling itself if encryption key isn't set (in dev mode we'll continue generating a fixed 'a'.repeat(32) key as before).

@nchaulet
Copy link
Member

In Fleet we are currently not enabling Fleet APIs and warning the user if usingEphemeralEncryptionKey is true.

I think we will be able to do the same if the plugin is optional and not set

@chrisronline
Copy link
Contributor

No issues from stack monitoring. The only reliance on encryptedSavedObjects for us is the dependency alerts have on it. We don't use encryptedSavedObjects deliberately.

@pmuellr
Copy link
Member

pmuellr commented Oct 15, 2020

This seems workable for alerts and actions (two different alerting plugins that deal with this, the same way). @mikecote ?

I think we'll need a two-stage approach here; we will need to adapt our plugins to use ESO as optional, and change our internal setting of whether ESO is viable to also be based on whether the plugin is available; after we've done that, ESO can remove the usingEphemeralEncryptionKey property, and hopefully that would be a clean delete of that reference in the plugins that reference it. I'm going to open an issue now for alerting, assuming we're going to move in this direction.

@mikecote
Copy link
Contributor

Some research required from alerting's perspective but some thoughts below:

I can't recall how likely it is that alerts exist when an ephemeral encryption key is used. If it's possible, there may be a concern in regards to migrations if we're making ESO plugin optional. We have some migrations that need to decrypt ESO and some that don't need to which may cause some to never execute after the user sets an encryption key.

@dhurley14
Copy link
Contributor

Access to the Detections feature in the security solution is blocked if a user is using ephemeralEncryptionKeys

so the security solution should be fine to remove it outside of any issues that the Kibana Alerting team brought up in the above comment.

@azasypkin azasypkin changed the title [DISCUSS] Don't generate xpack.encryptedSavedObjects.encryptionKey Don't generate xpack.encryptedSavedObjects.encryptionKey Oct 26, 2020
@azasypkin
Copy link
Member

Thanks everyone for the response! I've updated issue description with the decision we all have made and filed all necessary issues for the corresponding teams that haven't filed them yet:

The issues are marked as blockers for 7.11, but please let me know if you're not sure about that milestone. This issue is a bit tricky since we all need to make that change in a given release because we cannot release it partially.

@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Oct 27, 2020
@mikecote
Copy link
Contributor

The issues are marked as blockers for 7.11, but please let me know if you're not sure about that milestone.

After discussing with the @elastic/kibana-alerting-services team, we feel we have a lot on the 7.11 plate already with GA and a few post GA items to work on. Would it be possible to target 7.12 instead?

@azasypkin
Copy link
Member

After discussing with the @elastic/kibana-alerting-services team, we feel we have a lot on the 7.11 plate already with GA and a few post GA items to work on. Would it be possible to target 7.12 instead?

No objections from my side. If no one else has concerns too, I'll re-label all related issues with 7.12 later today.

@azasypkin azasypkin added v7.12.0 and removed v7.11.0 labels Oct 29, 2020
@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked blocker Feature:Fleet Fleet team's agent central management project Team:QA Team label for QA Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.12.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.