Skip to content

Commit

Permalink
Merge branch 'master' into 30951
Browse files Browse the repository at this point in the history
  • Loading branch information
elasticmachine authored Aug 10, 2020
2 parents 9ca2473 + ad8502c commit 07680a7
Show file tree
Hide file tree
Showing 86 changed files with 1,639 additions and 509 deletions.
4 changes: 2 additions & 2 deletions docs/developer/architecture/code-exploration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ Contains the Discover application and the saved search embeddable.
Embeddables are re-usable widgets that can be rendered in any environment or plugin. Developers can embed them directly in their plugin. End users can dynamically add them to any embeddable containers.
- {kib-repo}blob/{branch}/src/plugins/es_ui_shared[esUiShared]
- {kib-repo}blob/{branch}/src/plugins/es_ui_shared/README.md[esUiShared]
WARNING: Missing README.
This plugin contains reusable code in the form of self-contained modules (or libraries). Each of these modules exports a set of functionality relevant to the domain of the module.
- {kib-repo}blob/{branch}/src/plugins/expressions/README.md[expressions]
Expand Down
119 changes: 119 additions & 0 deletions rfcs/text/0012_encryption_key_rotation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
- Start Date: 2020-07-22
- RFC PR: [#72828](https://github.com/elastic/kibana/pull/72828)
- Kibana Issue: (leave this empty)

# Summary

This RFC proposes a way of the encryption key (`xpack.encryptedSavedObjects.encryptionKey`) rotation that would allow administrators to seamlessly change existing encryption key without any data loss and manual intervention.

# Basic example

When administrators decide to rotate encryption key they will have to generate a new one and move the old key(s) to the `keyRotation` section in the `kibana.yml`:

```yaml
xpack.encryptedSavedObjects:
encryptionKey: "NEW-encryption-key"
keyRotation:
decryptionOnlyKeys: ["OLD-encryption-key-1", "OLD-encryption-key-2"]
```
Before old decryption-only key is disposed administrators may want to call a dedicated and _protected_ API endpoint that will go through all registered Saved Objects with encrypted attributes and try to re-encrypt them with the primary encryption key:
```http request
POST https://localhost:5601/api/encrypted_saved_objects/rotate_key?conflicts=abort
Content-Type: application/json
Kbn-Xsrf: true
```
# Motivation
Today when encryption key changes we can no longer decrypt Saved Objects attributes that were previously encrypted with the `EncryptedSavedObjects` plugin. We handle this case in two different ways depending on whether consumers explicitly requested decryption or not:

* If consumers explicitly request decryption via `getDecryptedAsInternalUser()` we abort operation and throw exception.
* If consumers fetch Saved Objects with encrypted attributes that should be automatically decrypted (the ones with `dangerouslyExposeValue: true` marker) via standard Saved Objects APIs we don't abort operation, but rather strip all encrypted attributes from the response and record decryption error in the `error` Saved Object field.
* If Kibana tries to migrate encrypted Saved Objects at the start up time we abort operation and throw exception.

In both of these cases we throw or record error with the specific type to allow consumers to gracefully handle this scenario and either drop Saved Objects with unrecoverable encrypted attributes or facilitate the process of re-entering and re-encryption of the new values.

This approach works reasonably well in some scenarios, but it may become very troublesome if we have to deal with lots of Saved Objects. Moreover, we'd like to recommend our users to periodically rotate encryption keys even if they aren't compromised. Hence, we need to provide a way of seamless migration of the existing encrypted Saved Objects to a new encryption key.

There are two main scenarios we'd like to cover in this RFC:

## Encryption key is not available

Administrators may lose existing encryption key or explicitly decide to not use it if it was compromised and users can no longer trust encrypted content that may have been tampered with. In this scenario encrypted portion of the existing Saved Objects is considered lost, and the only way to recover from this state is a manual intervention described previously. That means `EncryptedSavedObjects` plugin consumers __should__ continue supporting this scenario even after we implement a proper encryption key rotation mechanism described in this RFC.

## Encryption key is available, but needs to be rotated

In this scenario a new encryption key (primary encryption key) will be generated, and we will use it to encrypt new or updated Saved Objects. We will still need to know the old encryption key to decrypt existing attributes, but we will no longer use this key to encrypt any of the new or existing Saved Objects. It's also should be possible to have multiple old decryption-only keys.

The old old decryption-only keys should be eventually disposed and users should have a way to make sure all existing Saved Objects are re-encrypted with the new primary encryption key.

__NOTE:__ users can get into a state when different Saved Objects are encrypted with different encryption keys even if they didn't intend to rotate the encryption key. We anticipate that it can happen during initial Elastic Stack HA setup, when by mistake or intentionally different Kibana instances were using different encryption keys. Key rotation mechanism can help to fix this issue without a data loss.

# Detailed design

The core idea is that when the encryption key needs to be rotated then a new key is generated and becomes a primary one, and the old one moves to the `keyRotation` section:

```yaml
xpack.encryptedSavedObjects:
encryptionKey: "NEW-encryption-key"
keyRotation:
decryptionOnlyKeys: ["OLD-encryption-key"]
```

As the name implies, the key from the `decryptionOnlyKeys` is only used to decrypt content that we cannot decrypt with the primary encryption key. It's allowed to have multiple decryption-only keys at the same time. When user creates a new Saved Object or updates the existing one then its content is always encrypted with the primary encryption key. Config schema won't allow having the same key in `encryptionKey` and `decryptionOnlyKeys`.

Having multiple decryption keys at the same time brings one problem though: we need to figure out which key to use to decrypt specific Saved Object. If our encryption keys could have a unique ID that we would store together with the encrypted data (we cannot use encryption key hash for that for obvious reasons) we could know for sure which key to use, but we don't have such functionality right now and it may not be the easiest one to manage through `yml` configuration anyway.

Instead, this RFC proposes to try available existing decryption keys one by one to decrypt Saved Object and always start from the primary one. This way we won't incur any penalty while decrypting Saved Objects that are already encrypted with the primary encryption key, but there will still be some cost when we have to perform multiple decryption attempts. See the [`Drawbacks`](#drawbacks) section for the details.

Technically just having `decryptionOnlyKeys` would be enough to cover the majority of the use cases, but the old decryption-only keys should be eventually disposed. At this point administrators would like to make sure _all_ Saved Objects are encrypted with the new primary encryption key. Another reason to re-encrypt all existing Saved Objects with the new key at once is to preventively reduce the performance impact of the multiple decryption attempts.

We'd like to make this process as simple as possible while meeting the following requirements:

* It should not be required to restart Kibana to perform this type of migration since Saved Objects encrypted with the another encryption key can theoretically appear at any point in time.
* It should be possible to integrate this operation into other operational flows our users may have and any user-friendly key management UIs we may introduce in this future.
* Any possible failures that may happen during this operation shouldn't make Kibana nonfunctional.
* Ordinary users should not be able to trigger this migration since it may consume a considerable amount of computing resources.

We think that the best option we have right now is a dedicated API endpoint that would trigger this migration:

```http request
POST https://localhost:5601/api/encrypted_saved_objects/rotate_key?conflicts=abort
Content-Type: application/json
Kbn-Xsrf: true
```

This will be a protected endpoint and only user with enough privileges will be able to use it.

Under the hood we'll scroll over all Saved Objects that are registered with `EncryptedSavedObjects` plugin and re-encrypt attributes only for those of them that can only be decrypted with any of the old decryption-only keys. Saved Objects that can be decrypted with the primary encryption key will be ignored. We'll also ignore the ones that cannot be decrypted with any of the available decryption keys at all, and presumably return their IDs in the response.

As for any other encryption or decryption operation we'll record relevant bits in the audit logs.

# Benefits

* The concept of decryption-only keys is easy to grasp and allows Kibana to function even if it has a mix of Saved Objects encrypted with different encryption keys.
* Support of the key rotation out of the box decreases the chances of the data loss and makes `EncryptedSavedObjects` story more secure and approachable overall.

# Drawbacks

* Multiple decryption attempts affect performance. See [the performance test results](https://github.com/elastic/kibana/pull/72420#issue-453400211) for more details, but making two decryption attempts is basically twice as slow as with a single attempt. Although it's only relevant for the encrypted Saved Objects migration performed at the start up time and batch operations that trigger automatic decryption (only for the Saved Objects registered with `dangerouslyExposeValue: true` marker that nobody is using in Kibana right now), we may have more use cases in the future.
* Historically we supported Kibana features with either configuration or dedicated UI, but in this case we want to introduce an API endpoint that _should be_ used directly. We may have a key management UI in the future though.

# Alternatives

We cannot think of any better alternative for `decryptionOnlyKeys` at the moment, but instead of API endpoint for the batch re-encryption we could potentially use another `kibana.yml` config option. For example `keyRotation.mode: onWrite | onStart | both`, but it feels a bit hacky and cannot be really integrated with anything else.

# Adoption strategy

Adoption strategy is pretty straightforward since the feature is an enhancement and doesn't bring any BWC concerns.

# How we teach this

Key rotation is a well-known paradigm. We'll update `README.md` of the `EncryptedSavedObjects` plugin and create a dedicated section in the public Kibana documentation.

# Unresolved questions

* Is it reasonable to have this feature in Basic?
* Are there any other use-cases that are not covered by the proposal?
32 changes: 32 additions & 0 deletions src/plugins/es_ui_shared/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
## ES UI shared modules

This plugin contains reusable code in the form of self-contained modules (or libraries). Each of these modules exports a set of functionality relevant to the domain of the module.

**Please note**: Modules in ES UI shared are intended for use by the ES UI Management Team (elastic/es-ui@) only. Please reach out to us if there is something you would like to contribute or use in these modules.

## Files and folders overview

- `./public` | `./server`. Folders for grouping server or public code according to the Kibana plugin pattern.
- `./__packages_do_not_import__` is where actual functionality is kept. This enables modules more control over what functionality is directly exported and prevents parts of modules to be depended on externally in unintended ways.
- `./public/index.ts` | `./server/index.ts` These files export modules (simple JavaScript objects). For example, `Monaco` is the name of a module. In this way, modules namespace all of their exports and do not have to be concerned about name collisions from other modules.

## Conventions for adding code

When adding new functionality, look at the folders in `./__packages_do_not_import__` and consider whether your functionality falls into any of those modules.

If it does not, you should create a module and expose it to public or server code (or both) following the conventions described above.

### Example

If I wanted to add functionality for calculating a Fibonacci sequence browser-side one would do the following:

1. Create a folder `./__packages_do_not_import__/math`. The name of the folder should be a snake_case version of the module name. In this case `Math` -> `math`. Another case, `IndexManagement` -> `index_management`.
2. Write your function in `./__packages_do_not_import__/math/calculate_fibonacci.ts`, adding any relevant tests in the same folder.
3. Export functionality intended _for consumers_ from `./__packages_do_not_import__/math/index.ts`.
4. Create a folder `./public/math`.
5. Export all functionality from `./__packages_do_not_import__/math` in `./public/math/index.ts`.
6. In `./public/index.ts` import `./public/math` using `import * as Math from './public/math;`. The name (`Math`) given here is really important and will be what consumers depend on.
7. Add the `Math` module to the list of exported modules in `./public/index.ts`, e.g. `export { <...other modules>, Math }`
8. Use `Math` in your public side code elsewhere!

This example assumes no other appropriate home for such a function exists.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

import React from 'react';
import { mount } from 'enzyme';
// @ts-expect-error untyped local
import { ExportApp } from '../export_app';
import { ExportApp } from '../export_app.component';
import { CanvasWorkpad } from '../../../../../types';

jest.mock('style-it', () => ({
it: (css: string, Component: any) => Component,
Expand All @@ -23,7 +23,7 @@ jest.mock('../../../../components/link', () => ({

describe('<ExportApp />', () => {
test('renders as expected', () => {
const sampleWorkpad = {
const sampleWorkpad = ({
id: 'my-workpad-abcd',
css: '',
pages: [
Expand All @@ -34,7 +34,7 @@ describe('<ExportApp />', () => {
elements: [3, 4, 5, 6],
},
],
};
} as any) as CanvasWorkpad;

const page1 = mount(
<ExportApp workpad={sampleWorkpad} selectedPageIndex={0} initializeWorkpad={() => {}} />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, useEffect } from 'react';
import PropTypes from 'prop-types';
// @ts-expect-error untyped library
import Style from 'style-it';
// @ts-expect-error untyped local
import { WorkpadPage } from '../../../components/workpad_page';
import { Link } from '../../../components/link';
import { CanvasWorkpad } from '../../../../types';

interface Props {
workpad: CanvasWorkpad;
selectedPageIndex: number;
initializeWorkpad: () => void;
}

export const ExportApp: FC<Props> = ({ workpad, selectedPageIndex, initializeWorkpad }) => {
const { id, pages, height, width } = workpad;
const activePage = pages[selectedPageIndex];
const pageElementCount = activePage.elements.length;

useEffect(() => initializeWorkpad());

return (
<div className="canvasExport" data-shared-page={selectedPageIndex + 1}>
<div className="canvasExport__stage">
<div className="canvasLayout__stageHeader">
<Link name="loadWorkpad" params={{ id }}>
Edit Workpad
</Link>
</div>
{Style.it(
workpad.css,
<div className="canvasExport__stageContent" data-shared-items-count={pageElementCount}>
<WorkpadPage
isSelected
key={activePage.id}
pageId={activePage.id}
height={height}
width={width}
registerLayout={() => {}}
unregisterLayout={() => {}}
/>
</div>
)}
</div>
</div>
);
};

ExportApp.propTypes = {
workpad: PropTypes.shape({
id: PropTypes.string.isRequired,
pages: PropTypes.array.isRequired,
}).isRequired,
selectedPageIndex: PropTypes.number.isRequired,
initializeWorkpad: PropTypes.func.isRequired,
};
59 changes: 0 additions & 59 deletions x-pack/plugins/canvas/public/apps/export/export/export_app.js

This file was deleted.

21 changes: 21 additions & 0 deletions x-pack/plugins/canvas/public/apps/export/export/export_app.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { connect } from 'react-redux';
import { initializeWorkpad } from '../../../state/actions/workpad';
import { getWorkpad, getSelectedPageIndex } from '../../../state/selectors/workpad';
import { ExportApp as Component } from './export_app.component';
import { State } from '../../../../types';

export const ExportApp = connect(
(state: State) => ({
workpad: getWorkpad(state),
selectedPageIndex: getSelectedPageIndex(state),
}),
(dispatch) => ({
initializeWorkpad: () => dispatch(initializeWorkpad()),
})
)(Component);
30 changes: 0 additions & 30 deletions x-pack/plugins/canvas/public/apps/export/export/index.js

This file was deleted.

Loading

0 comments on commit 07680a7

Please sign in to comment.