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

[cli] Add bin/kibana-encryption-keys #82838

Merged
merged 39 commits into from
Nov 19, 2020

Conversation

jbudz
Copy link
Member

@jbudz jbudz commented Nov 6, 2020

This introduces a new CLI for generating encryption keys. Currently
Kibana makes use of several encryption keys, in some cases randomly
generated on startup. Usually we want these statically defined in
kibana.yml, and this CLI targets making this process easier. This will
output keys for xpack.encryptedSavedObjects.encryptionKey,
xpack.reporting.encryptionKey, and xpack.security.encryptionkey to
stdout along with instructions on how to add this to your configuration.

Closes #65788

Usage

node scripts/kibana_encryption_key generate
node scripts/kibana_encryption_key generate  --interactive

Flags

  • interactive: prompt to write to kibana.yml
  • quiet: only output yml configuration that can be piped
  • force: regenerate all keys (opposed to only missing)

Release note:
Adds a new CLI for generating encryption keys used by Kibana.

@jbudz jbudz requested a review from a team November 6, 2020 14:47
@jbudz jbudz requested review from a team as code owners November 6, 2020 14:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Nov 6, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

This introduces a new CLI for generating encryption keys.  Currently
Kibana makes use of several encryption keys, in some cases randomly
generated on startup.  Usually we want these statically defined in
kibana.yml, and this CLI targets making this process easier.  This will
output keys for xpack.encryptedSavedObjects.encryptionKey,
xpack.reporting.encryptionKey, and xpack.security.encryptionkey to
stdout along with instructions on how to add this to your configuration.

Closes elastic#65788
@azasypkin azasypkin self-requested a review November 6, 2020 17:09
@azasypkin
Copy link
Member

ACK: will review on Monday

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.

It looks good to me from the security/usability stand point, but left a couple of questions.

src/cli_encryption_key/cli_encryption_key.js Outdated Show resolved Hide resolved

program
.command('help <command>')
.description('get the help for a specific command')
Copy link
Member

Choose a reason for hiding this comment

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

optional nit: should this description start from the capital letter as well? Like descriptions for the rest of the "commands"?

generate  [options]  Generates encryption keys
help  <command>      get the help for a specific command

Copy link
Member Author

@jbudz jbudz Nov 9, 2020

Choose a reason for hiding this comment

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

👍 pushed with 6b8e023

~/dev/kibana(cli/kibana-encryption-key) » node scripts/kibana_encrpytion_key.js --help         jon@win1d

  Usage: bin/kibana-encryption-key [command] [options]

  A tool for managing encryption keys

  Commands:
    generate  [options]  Generates encryption keys
    help  <command>      Get the help for a specific command

.command('help <command>')
.description('get the help for a specific command')
.action(function (cmdName) {
const cmd = _.find(program.commands, { _name: cmdName });
Copy link
Member

Choose a reason for hiding this comment

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

question: any reason we cannot just use Object.values(program.commands).find((command) => command._name === cmdName) and get rid of lodash here (yeah, just trying to remove lodash from everywhere I can 🙂 )?

Copy link
Member Author

@jbudz jbudz Nov 9, 2020

Choose a reason for hiding this comment

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

Pushed with 569a8c0

~/dev/kibana(cli/kibana-encryption-key*) » node scripts/kibana_encrpytion_key.js foo       1 ↵ jon@win1d

 ERROR  unknown command foo

  Usage: bin/kibana-encryption-key [command] [options]

  A tool for managing encryption keys

  Commands:
    generate  [options]  Generates encryption keys
    help  <command>      Get the help for a specific command

---------------------------------------------------------------------------------------------------------
~/dev/kibana(cli/kibana-encryption-key*) » node scripts/kibana_encrpytion_key.js generate 64 ↵ jon@win1d
Generating missing encryption keys.  Add these to kibana.yml:
...

src/cli_encryption_key/encryption_config.js Outdated Show resolved Hide resolved
encryptionConfig = new EncryptionConfig();
});
it('should load the current configuration', () => {
expect(encryptionConfig._hasEncryptionKey('xpack.reporting.encryptionKey')).toEqual(false);
Copy link
Member

Choose a reason for hiding this comment

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

note: as an idea, we could just mock readFileSync and crypto.randomBytes and avoid relying on private fields/methods in tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 pushed 58bdef8

const write = await confirm('Write to kibana.yml?');
if (write) {
const kibanaYML = join(getConfigDirectory(), 'kibana.yml');
appendFileSync(kibanaYML, safeDump(keys));
Copy link
Member

Choose a reason for hiding this comment

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

question: it seems interactive with force can lead to an invalid config because of duplicated entries. What is our thinking/plan here?

I expected interactive to be a bit more interactive to be honest. Something like, it could ask whether user wants to auto-generate a key for a particular setting or not, and warn if force is used and the key is already specified in kibana.yml.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm up for adding a more nuanced interactive mode. Any thoughts @legrego @kobelb @tylersmalley?

For the conflict between interactive and force - the sticking point was key rotation. We'll want to be careful about any removal, e.g. dropping a reporting encryption key could cause reports to be lost.

Copy link
Member

Choose a reason for hiding this comment

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

it seems interactive with force can lead to an invalid config because of duplicated entries.

Yeah we definitely don't want to leave our users with invalid kibana.yml files.

I'm up for adding a more nuanced interactive mode.

This makes sense to me, and would be consistent with some of our other CLI utilities, like elasticsearch-setup-passwords.

For the conflict between interactive and force - the sticking point was key rotation. We'll want to be careful about any removal, e.g. dropping a reporting encryption key could cause reports to be lost.

I don't fully understand this point. Isn't --force designed to replace the existing keys? If so, then I think replacing these keys with --interactive would make sense. We could have a more strongly worded confirmation message when both --interactive and --force are provided, but otherwise I'd think that replacing the existing keys is expected behavior.

Copy link
Member Author

@jbudz jbudz Nov 9, 2020

Choose a reason for hiding this comment

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

Works for me. I'll make another iteration on the interactive portions.

I don't fully understand this point. Isn't --force designed to replace the existing keys? If so, then I think replacing these keys with --interactive would make sense. We could have a more strongly worded confirmation message when both --interactive and --force are provided, but otherwise I'd think that replacing the existing keys is expected behavior.

My original planning was that it would not infrequently cause someone to back out to rotate. Opposed to warning followed by a failure to start would force an acknowledgement via manual triage. Mostly I didn't want to delete anything that could still be sensitive or difficult e.g. technically valid yaml for an encryption key could span two lines, so if --force caused deletions we'll have to figure out deletes beyond appending to file. We could remove the force flag entirely.

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed 6240c79 for UX feedback. It includes prompts for each setting.

Any thoughts on the force flag? I'm tempted to remove it altogether, key removal will require us to manage yaml parsing in a way that doesn't interfere with comments. I can edit the message to tell the user to rotate keys that are set if there isn't anything to generate

Copy link
Member

Choose a reason for hiding this comment

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

I pushed 6240c79 for UX feedback. It includes prompts for each setting.
Any thoughts on the force flag? I'm tempted to remove it altogether, key removal will require us to manage yaml parsing in a way that doesn't interfere with comments. I can edit the message to tell the user to rotate keys that are set if there isn't anything to generate

Yeah, I'm also worried about editing user's kibana.yml, but how about generating a sample kibana.yaml instead, something like this one generated by elasticsearch-certutil?

In the latest version of the PR we have this:

Generating settings for:
xpack.encryptedSavedObjects.encryptionKey
xpack.reporting.encryptionKey
xpack.security.encryptionKey

Set xpack.encryptedSavedObjects.encryptionKey? [y/N] 
Set xpack.reporting.encryptionKey? [y/N] 
Set xpack.security.encryptionKey? [y/N] 
Write 0 setting to kibana.yml? [y/N]

Maybe we can borrow some ideas from the elasticsearch-certutil here as well and have something similar to this (text is up to review by Docs Team and/or native speakers 🙂 )?

## Kibana Encryption Key Generation Utility

The 'generate' command guides you through the process of generating encryption
keys for the ........

This tool will ask you a number of questions in order to generate the right
set of keys for your needs.

Generate xpack.encryptedSavedObjects.encryptionKey? [y/N] y
Generate xpack.reporting.encryptionKey? [y/N] N
Generate xpack.security.encryptionKey? [y/N] y

The following encryption keys were generated:

xpack.encryptedSavedObjects.encryptionKey
xpack.security.encryptionKey

Save generated keys to a sample Kibana configuration file? Otherwise settings will be outputed here [y/N] y

What filename should be used for the sample Kibana config file? [/home/azasypkin/..../sample-kibana.yml]

----OR-----

xpack.encryptedSavedObjects.encryptionKey: 487f11e261c30cdca21a815521fe08cc
xpack.security.encryptionKey: 7b3b357dc84d4689bcf554ed1e599aed

Explanation of the key rotation in case xpack.encryptedSavedObjects.encryptionKey is generated can either go to a sample Kibana yml as a comment section or we can output it to STDOUT (with a link to https://www.elastic.co/guide/en/kibana/master/xpack-security-secure-saved-objects.html#encryption-key-rotation maybe)

What do you all think?

Copy link
Member Author

@jbudz jbudz Nov 11, 2020

Choose a reason for hiding this comment

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

Thanks, I like that idea. I'll do another loop and get tests updated.

Copy link
Member Author

@jbudz jbudz Nov 16, 2020

Choose a reason for hiding this comment

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

Here's the latest:

auto

## Kibana Encryption Key Generation Utility

The 'generate' command guides you through the process of setting encryption keys for:
xpack.encryptedSavedObjects.encryptionKey
    Used to encrypt stored objects such as dashboards and visualizations
    https://www.elastic.co/guide/en/kibana/current/xpack-security-secure-saved-objects.html#xpack-security-secure-saved-objects
xpack.reporting.encryptionKey
    Used to encrypt saved reports
    https://www.elastic.co/guide/en/kibana/current/reporting-settings-kb.html#general-reporting-settings
xpack.security.encryptionKey
    Used to encrypt session information
    https://www.elastic.co/guide/en/kibana/current/security-settings-kb.html#security-session-and-cookie-settings

Already defined settings are ignored and can be regenerated using the --force flag.  Check the documentation links for instructions on how to rotate encryption keys.
Definitions should be set in the kibana.yml used configure Kibana.

Settings:
xpack.encryptedSavedObjects.encryptionKey: 37538fff48dd0929c6c736caaec3645f
xpack.reporting.encryptionKey: 6ff208df97f25c14b943e7f69b00141b
xpack.security.encryptionKey: cb38085389e3f12b29423e675f05ed11

interactive:

## Kibana Encryption Key Generation Utility

The 'generate' command guides you through the process of setting encryption keys for:
xpack.encryptedSavedObjects.encryptionKey
    Used to encrypt stored objects such as dashboards and visualizations
    https://www.elastic.co/guide/en/kibana/current/xpack-security-secure-saved-objects.html#xpack-security-secure-saved-objects
xpack.reporting.encryptionKey
    Used to encrypt saved reports
    https://www.elastic.co/guide/en/kibana/current/reporting-settings-kb.html#general-reporting-settings
xpack.security.encryptionKey
    Used to encrypt session information
    https://www.elastic.co/guide/en/kibana/current/security-settings-kb.html#security-session-and-cookie-settings

Already defined settings are ignored and can be regenerated using the --force flag.  Check the documentation links for instructions o
n how to rotate encryption keys.
Definitions should be set in the kibana.yml used configure Kibana.

This tool will ask you a number of questions in order to generate the right set of keys for your needs.

Set xpack.encryptedSavedObjects.encryptionKey? [y/N] y
Set xpack.reporting.encryptionKey? [y/N] n
Set xpack.security.encryptionKey? [y/N] y

The following keys were generated:
xpack.encryptedSavedObjects.encryptionKey
xpack.security.encryptionKey

Save generated keys to a sample Kibana configuration file? [y/N] n

Settings:
xpack.encryptedSavedObjects.encryptionKey: a4c722998ded91ab56e73bafdf743d14
xpack.security.encryptionKey: d87732b078669dd714acbb3af8fb8af6

I'm wondering if the command generate should be replaced auto and interactive similar to the elasticsearch util. The quiet flag could be moved to auto to avoid any confusion. Originally there was talks of managing key rotation but to avoid any deletion errors that sounds like a less likely feature.

Copy link
Member Author

Choose a reason for hiding this comment

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

@gchaps if you have a chance, do you have any thoughts on the readability of the code blocks above?

src/cli_encryption_key/encryption_config.test.js Outdated Show resolved Hide resolved
src/cli_encryption_key/generate.test.js Outdated Show resolved Hide resolved
src/cli_encryption_key/generate.test.js Outdated Show resolved Hide resolved
src/cli_encryption_key/generate.test.js Outdated Show resolved Hide resolved
Copy link
Contributor

@skh skh left a comment

Choose a reason for hiding this comment

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

Ingest management / Fleet change looks good. I think this has implications for our documentation too -> cc @dedemorton

@ph
Copy link
Contributor

ph commented Nov 9, 2020

@nchaulet Could you take a look? Nevermind, @skh did it, I didn't see her comment in the projects, sorry for the noise. :(

jbudz and others added 8 commits November 9, 2020 08:52
…key')

Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
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! Just two notes:

  • It seems unlike other scripts we don't make kibana-encryption-keys executable in a dist version.
kibana-8.0.0-SNAPSHOT-linux-x86_64:
drwxr-xr-x 2 azasypkin users 4096 Nov 18 12:39 .
drwxr-xr-x 9 azasypkin users 4096 Nov 18 12:39 ..
-rwxr-xr-x 1 azasypkin users  850 Nov 18 12:39 kibana
-rw-r--r-- 1 azasypkin users  783 Nov 18 12:39 kibana-encryption-keys
-rwxr-xr-x 1 azasypkin users  776 Nov 18 12:39 kibana-keystore
-rwxr-xr-x 1 azasypkin users  813 Nov 18 12:39 kibana-plugin
  • It'd be nice if we could also output the docs for the keys to sample kibana.yml as comments, but up to you.

I'm wondering if the command generate should be replaced auto and interactive similar to the elasticsearch util. The quiet flag could be moved to auto to avoid any confusion.

I don't have a strong opinion on that, but if we decide to use auto and interactive as commands we may want to include generate into the name of the tool (similar to setup in elasticsearch-setup-passwords where auto and interactive are used).

src/cli_encryption_keys/interactive.js Outdated Show resolved Hide resolved
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 42858 42865 +7
oss 22474 22481 +7

History

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

@jbudz
Copy link
Member Author

jbudz commented Nov 18, 2020

🎈 . Planning on merging this tomorrow, giving a day in case any of the commenters wanted to take another look. Thanks everyone.

@dedemorton
Copy link
Contributor

@skh Can you confirm the doc impact here?

In the Fleet docs, we say:

"In the Kibana configuration, the saved objects encryption key (xpack.encryptedSavedObjects.encryptionKey) must be set to any alphanumeric value of at least 32 characters. Fleet requires this setting in order to save API keys and encrypt them in Kibana."

I think we just need to change the statement to indicate that the user can set xpack.encryptedSavedObjects.encryptionKey or run the bin/kibana-encryption-key command. Is that correct?

@skh
Copy link
Contributor

skh commented Nov 19, 2020

@dedemorton Yes, exactly.

@tylersmalley tylersmalley changed the title [cli] Add bin/kibana-encryption-key [cli] Add bin/kibana-encryption-keys Nov 19, 2020
@jbudz
Copy link
Member Author

jbudz commented Nov 19, 2020

Spoke with @dedemorton, those particular docs changes will be in a separate repo. Merging away.

@jbudz jbudz merged commit 6c23302 into elastic:master Nov 19, 2020
@jbudz jbudz deleted the cli/kibana-encryption-key branch November 19, 2020 18:41
jbudz added a commit that referenced this pull request Nov 19, 2020
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
Co-authored-by: Tyler Smalley <tylersmalley@me.com>
@jbudz
Copy link
Member Author

jbudz commented Nov 19, 2020

7.x/7.11: 47d788d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported release_note:enhancement Team:Fleet Team label for Observability Data Collection Fleet team Team:Operations Team label for Operations Team v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ephemeral encrypted saved-object encryption key