Password Protected Keystore (Feature Branch) (cleaned up) #49268

williamrandolph · 2019-11-18T20:49:56Z

Note: This is a cleaned-up version of #49210: the branch there was somehow polluted by commits from a poorly-performed merge, so I have rebased just the commits relevant to the feature branch. This PR is pointed at a new feature branch in the elastic/elasticsearch repo that, at the time of this writing, shares a head with master. The effect of merging this PR will be to re-create the feature branch in the elasticsearch repo.

There are a few remaining issues to work on once the feature branch is recreated. Reviewers should check over recent commits on this branch. The cherry-picked commits from the old feature branch work have already passed review.

TODO:

mute currently failing/flaky tests
fix docs build

Description

Elasticsearch uses a custom on-disk keystore for secure settings such as passwords and SSL certificates. Up until now, this prevented users with command-line access from viewing secure files by listing commands, but nothing prevented such users from changing values in the keystore, or removing values from it. Furthermore, the values were only obfuscated by a hash; no user-specific secret protected the secure settings.

This PR changes all of that by adding password-protection to the keystore. This will not be a breaking change for users. If a keystore has no password, there won’t be any new prompts. A user must choose to password-protect their keystore in order to see any new behavior.

This is a large feature branch, but the major pieces of production code are straightforward:

The keystore CLI has been extended to allow users to create a password-protected keystore, to add password-protection to an existing keystore, and to change or remove a password from an existing keystore. Once a keystore has a password, the CLI will prompt for a password when listing, adding, or removing secure settings. There are new subcommands for the keystore CLI: “passwd” is for changing a keystore’s password, and “has-passwd” is for checking whether a keystore is password-protected. The common code for password handling in keystore CLI commands is contained in a new BaseKeystoreCommand class.
The Elasticsearch startup scripts for Windows and Bash have been modified to read passwords on standard input. In the systemd case, a small entrypoint script can read the password from a file and pass it to the startup script. In the Docker case, the Docker entrypoint script can retrieve a password from an environment variable and provide it to the Elasticsearch startup script.
The REST API command for reloading secure settings now requires a password when a node’s keystore is password-protected. A call to the reload_secure_settings with a password is allowed in two cases: (1) When run against the local node ( single node cluster, or specifically targeting the local cluster via _local filter or nodeId) regardless of the TLS configuration in the transport layer. (2) When run for more nodes than the local one (some or all cluster) iff TLS is enabled in the transport layer.
The Command and Terminal classes have been modified to support finer-grained control over reading from standard input and writing errors to the terminal. Specifically, we read passwords into character arrays (wrapped as “secure strings”) up to a maximum length of 128 (arbitrarily chosen).
Much of the new code in this PR is test code. Many of our already-existing keystore unit tests now use password-protected keystrokes. We also have extensive “qa” tests: a “KeystoreManagementTests” class that can run for both archive (zip, tar.gz) and package (rpm, deb) distributions as part of the packaging test matrix. Unfortunately for reviewers, a decent amount of refactoring went into making this work, and it happened on the feature branch rather than in the main repo.

There is documentation on the feature branch for the Keystore CLI and for reloading secure settings. There is no documentation on the various ways of providing a password at Elasticsearch startup; such documentation must be added before the release, but it does not necessarily block the feature branch.

If a password is not set, we assume an empty string to be compatible with previous behavior. Only allow the reload to be broadcast to other nodes if TLS is enabled for the transport layer.

This change adds support for keystore passphrases to all subcommands of the elasticsearch-keystore cli tool and adds a subcommand for changing the passphrase of an existing keystore. The work to read the passphrase in Elasticsearch when loading, which will be addressed in a different PR. Subcommands of elasticsearch-keystore can handle (open and create) passphrase protected keystores When reading a keystore, a user is only prompted for a passphrase only if the keystore is passphrase protected. When creating a keystore, a user is allowed (default behavior) to create one with an empty passphrase Passphrase can be set to be empty when changing/setting it for an existing keystore Relates to: elastic#32691 Supersedes: elastic#37472

Turns out that the behavior of `-f` for the add and add-file sub commands where it would also forcibly create the keystore if it didn't exist, was by design - although undocumented. This change restores that behavior auto-creating a keystore that is not password protected if the force flag is used. The force OptionSpec is moved to the BaseKeyStoreCommand as we will presumably want to maintain the same behavior in any other command that takes a force option.

This change ensures that `elasticsearch-setup-passwords` and `elasticsearch-saml-metadata` can handle a password protected elasticsearch.keystore. For setup passwords the user would be prompted to add the elasticsearch keystore password upon running the tool. There is no option to pass the password as a parameter as we assume the user is present in order to enter the desired passwords for the built-in users. For saml-metadata, we prompt for the keystore password at all times even though we'd only need to read something from the keystore when there is a signing or encryption configuration.

Adds a sentence in the documentation of `elasticsearch-setup-passwords` and `elasticsearch-saml-metadata` to describe that users would be prompted for the keystore's password when running these CLI tools, when the keystore is password protected. Co-Authored-By: Lisa Cawley <lcawley@elastic.co>

This commit allows a user to provide a keystore password on Elasticsearch startup, but only prompts when the keystore exists and is encrypted. The entrypoint in Java code is standard input. When the Bootstrap class is checking for secure keystore settings, it checks whether or not the keystore is encrypted. If so, we read one line from standard input and use this as the password. For simplicity's sake, we allow a maximum passphrase length of 128 characters. (This is an arbitrary limit and could be increased or eliminated. It is also enforced in the keystore tools, so that a user can't create a password that's too long to enter at startup.) In order to provide a password on standard input, we have to account for four different ways of starting Elasticsearch: the bash startup script, the Windows batch startup script, systemd startup, and docker startup. We use wrapper scripts to reduce systemd and docker to the bash case: in both cases, a wrapper script can read a passphrase from the filesystem and pass it to the bash script. In order to simplify testing the need for a passphrase, I have added a has-passwd command to the keystore tool. This command can run silently, and exit with status 0 when the keystore has a password. It exits with status 1 if the keystore doesn't exist or exists and is unencrypted. A good deal of the code-change in this commit has to do with refactoring packaging tests to cleanly use the same tests for both the "archive" and the "package" cases. This required not only moving tests around, but also adding some convenience methods for an abstraction layer over distribution-specific commands. I will write some user-facing documentation for these changes in a follow-up commit.

This commit adds relevant parts in the elasticsearch-keystore sub-commands reference docs and in the reload secure settings API doc.

The feature branch for the password-protected keystore, due to an accident, contains a large number of unrelated commits. In order to get a cleaner merge, I've cherry-picked the main commits that went into the feature branch against a branch derived from master — essentially, a rebase onto master. This commit cleans up issues that came up with gradle precommit.

elasticmachine · 2019-11-19T11:32:30Z

Pinging @elastic/es-security (:Security/Authentication)

elasticmachine · 2019-11-19T11:32:32Z

Pinging @elastic/es-core-infra (:Core/Infra/Packaging)

williamrandolph · 2019-11-20T14:47:11Z

@elasticmachine run elasticsearch-ci/packaging-matrix

williamrandolph · 2019-11-21T21:48:13Z

At this point, I have things mostly in the same state as the previous feature branch. There are three things that remain to work on:

Documentation for providing passwords at startup. (Documentation for keystore passwords at startup #49338)
Handle the Docker case in keystore tests. (This work hasn't been done because the Docker packaging tests were developed in parallel to the work on this branch.) (Make KeystoreManagementTests work in the docker case #49469)
Fix the currently flaky expect-based tests, and get them working on Windows. This may involve using a different Java library to simulate an interactive terminal. (Add Windows-compatible testing for interacting with CLI over TTY #49340)

These items are distinct enough that they should have their own PRs.

This cleaned-up feature branch should be the target for those issues. Is it all right to push this work to the elastic/elasticsearch repo as a new version of the old feature branch? Any thoughts, @jkakavas , @rjernst , @jasontedor ?

williamrandolph · 2019-11-22T16:29:06Z

@elasticmachine run elasticsearch-ci/packaging-matrix

jkakavas

LGTM

williamrandolph · 2019-12-02T14:57:09Z

@elasticmachine run elasticsearch-ci/packaging-matrix

williamrandolph · 2019-12-02T21:26:52Z

@elasticmachine run elasticsearch-ci/1

williamrandolph · 2019-12-02T22:00:01Z

@elasticmachine run elasticsearch-ci/packaging-matrix

williamrandolph · 2019-12-05T21:06:17Z

@elasticmachine run elasticsearch-ci/packaging-matrix

williamrandolph · 2019-12-06T12:16:30Z

@elasticmachine run elasticsearch-ci/packaging-matrix

This refactor bridges some gaps between a long-running feature branch (#49268) and the master branch. First of all, this PR gives our PackagingTestCase class some methods to start and stop Elasticsearch that will switch on packaging type and delegate to the appropriate utility class for deb/RPM packages, archive installations, and Docker. These methods should be very useful as we continue group tests by function rather than by package or platform type. Second, the password-protected keystore tests have a particular need to read the output of Elasticsearch startup commands. In order to make this easer to do, some commands now return Shell.Result objects so that tests can check over output to the shell. To that end, there's also an assertElasticsearchFailure method that will handle checking for startup failures for the various distribution types. There is an update to the Powershell startup script for archives that asynchronously redirects the output of the Powershell process to files that we can read for errors. Finally, we use the ES_STARTUP_SLEEP_TIME environment variable to make sure that our startup commands wait long enough before exiting for errors to make it to the standard output and error streams.

This refactor bridges some gaps between a long-running feature branch (elastic#49268) and the master branch. First of all, this PR gives our PackagingTestCase class some methods to start and stop Elasticsearch that will switch on packaging type and delegate to the appropriate utility class for deb/RPM packages, archive installations, and Docker. These methods should be very useful as we continue group tests by function rather than by package or platform type. Second, the password-protected keystore tests have a particular need to read the output of Elasticsearch startup commands. In order to make this easer to do, some commands now return Shell.Result objects so that tests can check over output to the shell. To that end, there's also an assertElasticsearchFailure method that will handle checking for startup failures for the various distribution types. There is an update to the Powershell startup script for archives that asynchronously redirects the output of the Powershell process to files that we can read for errors. Finally, we use the ES_STARTUP_SLEEP_TIME environment variable to make sure that our startup commands wait long enough before exiting for errors to make it to the standard output and error streams.

…ected-keystore-cleanup

This refactor bridges some gaps between a long-running feature branch (#49268) and the master branch. First of all, this PR gives our PackagingTestCase class some methods to start and stop Elasticsearch that will switch on packaging type and delegate to the appropriate utility class for deb/RPM packages, archive installations, and Docker. These methods should be very useful as we continue group tests by function rather than by package or platform type. Second, the password-protected keystore tests have a particular need to read the output of Elasticsearch startup commands. In order to make this easer to do, some commands now return Shell.Result objects so that tests can check over output to the shell. To that end, there's also an assertElasticsearchFailure method that will handle checking for startup failures for the various distribution types. There is an update to the Powershell startup script for archives that asynchronously redirects the output of the Powershell process to files that we can read for errors. Finally, we use the ES_STARTUP_SLEEP_TIME environment variable to make sure that our startup commands wait long enough before exiting for errors to make it to the standard output and error streams.

…ected-keystore-cleanup

williamrandolph · 2019-12-11T15:20:00Z

@elasticmachine run elasticsearch-ci/packaging-matrix

williamrandolph · 2019-12-11T20:24:28Z

@elasticmachine run elasticsearch-ci/packaging-matrix

) * Reload secure settings with password (elastic#43197) If a password is not set, we assume an empty string to be compatible with previous behavior. Only allow the reload to be broadcast to other nodes if TLS is enabled for the transport layer. * Add passphrase support to elasticsearch-keystore (elastic#38498) This change adds support for keystore passphrases to all subcommands of the elasticsearch-keystore cli tool and adds a subcommand for changing the passphrase of an existing keystore. The work to read the passphrase in Elasticsearch when loading, which will be addressed in a different PR. Subcommands of elasticsearch-keystore can handle (open and create) passphrase protected keystores When reading a keystore, a user is only prompted for a passphrase only if the keystore is passphrase protected. When creating a keystore, a user is allowed (default behavior) to create one with an empty passphrase Passphrase can be set to be empty when changing/setting it for an existing keystore Relates to: elastic#32691 Supersedes: elastic#37472 * Restore behavior for force parameter (elastic#44847) Turns out that the behavior of `-f` for the add and add-file sub commands where it would also forcibly create the keystore if it didn't exist, was by design - although undocumented. This change restores that behavior auto-creating a keystore that is not password protected if the force flag is used. The force OptionSpec is moved to the BaseKeyStoreCommand as we will presumably want to maintain the same behavior in any other command that takes a force option. * Handle pwd protected keystores in all CLI tools (elastic#45289) This change ensures that `elasticsearch-setup-passwords` and `elasticsearch-saml-metadata` can handle a password protected elasticsearch.keystore. For setup passwords the user would be prompted to add the elasticsearch keystore password upon running the tool. There is no option to pass the password as a parameter as we assume the user is present in order to enter the desired passwords for the built-in users. For saml-metadata, we prompt for the keystore password at all times even though we'd only need to read something from the keystore when there is a signing or encryption configuration. * Modify docs for setup passwords and saml metadata cli (elastic#45797) Adds a sentence in the documentation of `elasticsearch-setup-passwords` and `elasticsearch-saml-metadata` to describe that users would be prompted for the keystore's password when running these CLI tools, when the keystore is password protected. Co-Authored-By: Lisa Cawley <lcawley@elastic.co> * Elasticsearch keystore passphrase for startup scripts (elastic#44775) This commit allows a user to provide a keystore password on Elasticsearch startup, but only prompts when the keystore exists and is encrypted. The entrypoint in Java code is standard input. When the Bootstrap class is checking for secure keystore settings, it checks whether or not the keystore is encrypted. If so, we read one line from standard input and use this as the password. For simplicity's sake, we allow a maximum passphrase length of 128 characters. (This is an arbitrary limit and could be increased or eliminated. It is also enforced in the keystore tools, so that a user can't create a password that's too long to enter at startup.) In order to provide a password on standard input, we have to account for four different ways of starting Elasticsearch: the bash startup script, the Windows batch startup script, systemd startup, and docker startup. We use wrapper scripts to reduce systemd and docker to the bash case: in both cases, a wrapper script can read a passphrase from the filesystem and pass it to the bash script. In order to simplify testing the need for a passphrase, I have added a has-passwd command to the keystore tool. This command can run silently, and exit with status 0 when the keystore has a password. It exits with status 1 if the keystore doesn't exist or exists and is unencrypted. A good deal of the code-change in this commit has to do with refactoring packaging tests to cleanly use the same tests for both the "archive" and the "package" cases. This required not only moving tests around, but also adding some convenience methods for an abstraction layer over distribution-specific commands. I will write some user-facing documentation for these changes in a follow-up commit. * Adjust docs for password protected keystore (elastic#45054) This commit adds relevant parts in the elasticsearch-keystore sub-commands reference docs and in the reload secure settings API doc. * Cleanup after feature branch reconstruction The feature branch for the password-protected keystore, due to an accident, contains a large number of unrelated commits. In order to get a cleaner merge, I've cherry-picked the main commits that went into the feature branch against a branch derived from master — essentially, a rebase onto master. We've ignored some tests that will addressed in follow-up PRs to the feature branch.

This refactor bridges some gaps between a long-running feature branch (elastic#49268) and the master branch. First of all, this PR gives our PackagingTestCase class some methods to start and stop Elasticsearch that will switch on packaging type and delegate to the appropriate utility class for deb/RPM packages, archive installations, and Docker. These methods should be very useful as we continue group tests by function rather than by package or platform type. Second, the password-protected keystore tests have a particular need to read the output of Elasticsearch startup commands. In order to make this easer to do, some commands now return Shell.Result objects so that tests can check over output to the shell. To that end, there's also an assertElasticsearchFailure method that will handle checking for startup failures for the various distribution types. There is an update to the Powershell startup script for archives that asynchronously redirects the output of the Powershell process to files that we can read for errors. Finally, we use the ES_STARTUP_SLEEP_TIME environment variable to make sure that our startup commands wait long enough before exiting for errors to make it to the standard output and error streams.

jkakavas and others added 8 commits November 18, 2019 13:40

Reload secure settings with password (elastic#43197)

93485d6

If a password is not set, we assume an empty string to be compatible with previous behavior. Only allow the reload to be broadcast to other nodes if TLS is enabled for the transport layer.

Adjust docs for password protected keystore (elastic#45054)

f38a911

This commit adds relevant parts in the elasticsearch-keystore sub-commands reference docs and in the reload secure settings API doc.

williamrandolph mentioned this pull request Nov 18, 2019

Password Protected Keystore (Feature Branch) #49210

Closed

cbuescher added :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) >feature v7.6.0 v8.0.0 labels Nov 19, 2019

williamrandolph added 2 commits November 19, 2019 10:54

Fix test failures after rebase

0a91e3a

Add null check for docker case

ee1d089

Add pause for tty tests to emit status

63763b5

williamrandolph mentioned this pull request Nov 21, 2019

Allow the keystore to be password protected #32691

Closed

williamrandolph removed >feature v7.6.0 v8.0.0 labels Nov 22, 2019

williamrandolph changed the base branch from master to feature-pwd-protected-keystore-2 November 22, 2019 19:04

williamrandolph requested a review from jkakavas November 22, 2019 19:09

williamrandolph added 2 commits November 22, 2019 15:05

Merge branch 'master' into feature-pwd-protected-keystore-cleanup

3536251

Match style for other nodes docs

590171b

Muting flaky feature tests

3b79cf8

jkakavas approved these changes Nov 26, 2019

View reviewed changes

Merging upstream changes from master

961881f

Adjust code from master for tests

26ca909

williamrandolph mentioned this pull request Dec 6, 2019

[WIP] Refactor utility code in qa:os: tests #49945

Merged

williamrandolph mentioned this pull request Dec 9, 2019

Refactor utility code in qa:os: tests (#49945) #50000

Merged

williamrandolph added 2 commits December 9, 2019 17:16

Merge master into feature branch

8e8271d

Merge branch 'feature-pwd-protected-keystore-2' into feature-pwd-prot…

a40da44

…ected-keystore-cleanup

williamrandolph added 2 commits December 11, 2019 09:54

Ignore failing test

7002570

Merge branch 'feature-pwd-protected-keystore-2' into feature-pwd-prot…

9d413fc

…ected-keystore-cleanup

Temporarily ignore failing Docker test

27fce2d

williamrandolph merged commit b0be180 into elastic:feature-pwd-protected-keystore-2 Dec 11, 2019

williamrandolph deleted the feature-pwd-protected-keystore-cleanup branch December 11, 2019 22:13

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Password Protected Keystore (Feature Branch) (cleaned up) #49268

Password Protected Keystore (Feature Branch) (cleaned up) #49268

williamrandolph commented Nov 18, 2019 •

edited

Loading

elasticmachine commented Nov 19, 2019

elasticmachine commented Nov 19, 2019

williamrandolph commented Nov 20, 2019

williamrandolph commented Nov 21, 2019 •

edited

Loading

williamrandolph commented Nov 22, 2019

jkakavas left a comment

williamrandolph commented Dec 2, 2019

williamrandolph commented Dec 2, 2019

williamrandolph commented Dec 2, 2019

williamrandolph commented Dec 5, 2019

williamrandolph commented Dec 6, 2019

williamrandolph commented Dec 11, 2019

williamrandolph commented Dec 11, 2019

Password Protected Keystore (Feature Branch) (cleaned up) #49268

Password Protected Keystore (Feature Branch) (cleaned up) #49268

Conversation

williamrandolph commented Nov 18, 2019 • edited Loading

Description

elasticmachine commented Nov 19, 2019

elasticmachine commented Nov 19, 2019

williamrandolph commented Nov 20, 2019

williamrandolph commented Nov 21, 2019 • edited Loading

williamrandolph commented Nov 22, 2019

jkakavas left a comment

Choose a reason for hiding this comment

williamrandolph commented Dec 2, 2019

williamrandolph commented Dec 2, 2019

williamrandolph commented Dec 2, 2019

williamrandolph commented Dec 5, 2019

williamrandolph commented Dec 6, 2019

williamrandolph commented Dec 11, 2019

williamrandolph commented Dec 11, 2019

williamrandolph commented Nov 18, 2019 •

edited

Loading

williamrandolph commented Nov 21, 2019 •

edited

Loading