-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Conversation
Pinging @elastic/kibana-operations (Team:Operations) |
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
890523c
to
46da571
Compare
ACK: will review on Monday |
There was a problem hiding this 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.
|
||
program | ||
.command('help <command>') | ||
.description('get the help for a specific command') |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 }); |
There was a problem hiding this comment.
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 🙂 )?
There was a problem hiding this comment.
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:
...
encryptionConfig = new EncryptionConfig(); | ||
}); | ||
it('should load the current configuration', () => { | ||
expect(encryptionConfig._hasEncryptionKey('xpack.reporting.encryptionKey')).toEqual(false); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 pushed 58bdef8
src/cli_encryption_key/generate.js
Outdated
const write = await confirm('Write to kibana.yml?'); | ||
if (write) { | ||
const kibanaYML = join(getConfigDirectory(), 'kibana.yml'); | ||
appendFileSync(kibanaYML, safeDump(keys)); |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this 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
…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>
Co-authored-by: Tyler Smalley <tylersmalley@me.com>
Co-authored-by: Tyler Smalley <tylersmalley@me.com>
There was a problem hiding this 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
andinteractive
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).
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
…nto cli/kibana-encryption-key
💚 Build SucceededMetrics [docs]Distributable file count
History
To update your PR or re-run it, just comment with: |
🎈 . Planning on merging this tomorrow, giving a day in case any of the commenters wanted to take another look. Thanks everyone. |
@skh Can you confirm the doc impact here? In the Fleet docs, we say: "In the Kibana configuration, the saved objects encryption key ( I think we just need to change the statement to indicate that the user can set |
@dedemorton Yes, exactly. |
Spoke with @dedemorton, those particular docs changes will be in a separate repo. Merging away. |
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com> Co-authored-by: Tyler Smalley <tylersmalley@me.com>
7.x/7.11: 47d788d |
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
Flags
Release note:
Adds a new CLI for generating encryption keys used by Kibana.