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

Password Protected Keystore (Feature Branch) #49210

Closed
wants to merge 115 commits into from

Conversation

williamrandolph
Copy link
Contributor

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.
  • 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.

jkakavas and others added 30 commits July 11, 2019 16:25
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: #32691
Supersedes: #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.
Mutes data frame BWC tests prior to backporting #44768
This change adjusts the changes of #44768 to account
for the backport to the 7.x branch in #44848.
remove the unreliable check for the state change

fixes #44813
Refactor left out the spatial strategy check in GeoShapeQueryBuilder.relation
setter method. This commit adds that check back in.
This PR makes two changes to FetchSourceSubPhase when _source is disabled and
we're in a nested context:
* If no source filters are provided, return early to avoid an NPE.
* If there are source filters, make sure to throw an exception.

The behavior was chosen to match what currently happens in a non-nested context.
Adds an API to clone an index. This is similar to the index split and shrink APIs, just with the
difference that the number of primary shards is kept the same. In case where the filesystem
provides hard-linking capabilities, this is a very cheap operation.

Indexing cloning can be done by running `POST my_source_index/_clone/my_target_index` and it
supports the same options as the split and shrink APIs.

Closes #44128
While joda no longer exists in the apis for 7.x, the compatibility layer
still exists with helper methods mimicking the behavior of joda for
ZonedDateTime objects returned for date fields in scripts. This layer
was originally intended to be removed in 7.0, but is now likely to exist
for the lifetime of 7.x.

This commit adds missing methods from ChronoZonedDateTime to the compat
class. These methods were not part of joda, but are needed to act like a
real ZonedDateTime.

relates #44411
Refactors RemoteClusterConnection so that it no longer blockingly connects to remote clusters.

Relates to #40150
Changes the BWC conditions for the clone index API after backport of the feature to 7.x
In order to make it easier to interpret the output of the ILM Explain
API, this commit adds two request parameters to that API:

- `only_managed`, which causes the response to only contain indices
  which have `index.lifecycle.name` set
- `only_errors`, which causes the response to contain only indices in an
  ILM error state

"Error state" is defined as either being in the `ERROR` step or having
`index.lifecycle.name` set to a policy that does not exist.
Since 7.3, it's possible to explicitly configure the SAML realm to
be used in Kibana's configuration. This in turn, eliminates the need
of properly setting `xpack.security.public.*` settings in Kibana
and largely simplifies relevant documentation.
This also changes `xpack.security.authProviders` to
`xpack.security.authc.providers` as the former was deprecated in
favor of the latter in 7.3 in Kibana
…ax_num_segments` set (#44761)

This commit changes the ForceMergeRequest.validate() method so that it does 
not accept the parameters only_expunge_deletes and max_num_segments 
to be set at the same time.

The motivation is that InternalEngine.forceMerge() just ignores the max. number 
of segments parameter when the only expunge parameter is set to true, leaving 
the wrong impression to the user that max. number of segments has been applied. 
It also changes InternalEngine.forceMerge() so that it now throws an exception 
when both parameters are set, and modifies tests where needed.

Because it changes the behavior of the REST API I marked this as >breaking. 

Closes #43102
Today the processors setting is permitted to be set to more than the
number of processors available to the JVM. The processors setting
directly sizes the number of threads in the various thread pools, with
most of these sizes being a linear function in the number of
processors. It doesn't make any sense to set processors very high as the
overhead from context switching amongst all the threads will overwhelm,
and changing the setting does not control how many physical CPU
resources there are on which to schedule the additional threads. We have
to draw a line somewhere and this commit deprecates setting processors
to more than the number of available processors. This is the right place
to draw the line given the linear growth as a function of processors in
most of the thread pools, and that some are capped at the number of
available processors already.
…edNode (#44860)

The test ShrinkIndexIT.testShrinkThenSplitWithFailedNode sometimes fails 
because the resize operation is not acknowledged (see #44736). This resize 
operation creates a new index "splitagain" and it results in a cluster state 
update (TransportResizeAction uses MetaDataCreateIndexService.createIndex() 
to create the resized index). This cluster state update is expected to be 
acknowledged by all nodes (see IndexCreationTask.onAllNodesAcked()) but 
this is not always true: the data node that was just stopped in the test before 
executing the resize operation might still be considered as a "faulty" node
 (and not yet removed from the cluster nodes) by the FollowersChecker. The 
cluster state is then acked on all nodes but one, and it results in a non 
acknowledged resize operation.

This commit adds an ensureStableCluster() check after stopping the node in 
the test. The goal is to ensure that the data node has been correctly removed 
from the cluster and that all nodes are fully connected to each before moving 
forward with the resize operation.

Closes #44736
…44806)

This PR addresses the feedback in  elastic/ml-team#175 (comment).

* Adds an example to `analyzed_fields`
* Includes `source` and `dest` objects inline in the resource page
* Lists `model_memory_limit` in the PUT API page
* Amends the `analysis` section in the resource page
* Removes Properties headings in subsections
jkakavas and others added 14 commits August 16, 2019 12:25
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.
@williamrandolph williamrandolph added >feature :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) v8.0.0 v7.6.0 labels Nov 15, 2019
@elasticmachine
Copy link
Collaborator

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

@elasticmachine
Copy link
Collaborator

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

@williamrandolph
Copy link
Contributor Author

#49268 is a cleaner version of this work.

@williamrandolph williamrandolph deleted the feature-pwd-protected-keystore branch May 6, 2020 21:48
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >feature :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Delivery Meta label for Delivery team v7.6.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.