Security auto-configuration for packaged installations #75144
Conversation
jkakavas
added
:Delivery/Packaging
jkakavas
changed the title
qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java
Outdated
Show resolved
Hide resolved
|
I've added pointers to what this PR changes, the rest is #74868 |
qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java
Outdated
Show resolved
Hide resolved
…t it in the keystore so that it can be used node-locally and so that our PostConf functionality can pick it up and set it cluster-wide on the first run
…on't run this accidently
...n/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/AutoConfigInitialNode.java
Outdated
Show resolved
Hide resolved
...n/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/AutoConfigInitialNode.java
Outdated
Show resolved
Hide resolved
distribution/src/bin/elasticsearch
Outdated
| @@ -45,6 +50,19 @@ then | |||
| fi | |||
| fi | |||
|
|
|||
| if [[ $AUTO_CONFIG = true ]]; then | |||
| # ignore failures in auto-configuration | |||
There was a problem hiding this comment.
I spoke with @colings86 about this today and we may want to revisit this decision.
There was a problem hiding this comment.
Which decision is that ?
There was a problem hiding this comment.
That if we fail to perform auto-configuration (can't write to file) that we might want to fail instead of ignore. Something about "leniency" and being "abhorent", blah blah blah 😉
There was a problem hiding this comment.
I like aphorisms as the next person but this is not about leniency. We ( and by we I mean mostly @albertzaharovits ) have spent a lot of time thinking about the cases where we should fail and when we should not. This is written in code in #74868 and Albert has the task to write this down to the design doc too. In short, we consider the case where we can't write to the config file because it is set as read-only, as a case where we have an explicit configuration decision by the user ( the file doesn't get to be read only by its own). As such we have elected to not treat this as an error and fail to start the node ( and in doing so introduce a breaking change as we allow this today ) but simply not attempt to auto-configure security for them. Authentication will be enabled but we won't enable and configure TLS. Happy to reconsider or discuss alternative approaches, @colings86 feel free to bring this up for discussion in the project or we can chat anytime
| verifySecurityAutoConfigured(es, distribution); | ||
| } | ||
|
|
||
| private static void verifySecurityAutoConfigured(Installation es, Distribution distribution) throws IOException { |
There was a problem hiding this comment.
I'd really rather not continue to add test coverage as assertions to verifyInstallation(). We should implement this as a new distinct test suite. We also need to add a lot more coverage here for all the other scenarios as this only covers the "happy path". For example:
- Verify the correct behavior (still under debate) when
elasticsearch.ymlfile is non-writable - Verify we don't auto config security if security settings already exist
- Verify we don't auto config security on an upgrade
There are likely other more specific scenarios that probably are better implemented as unit tests for the CLI tool for which no tests currently yet exist.
There was a problem hiding this comment.
We also need to add a lot more coverage here for all the other scenarios
Thanks for the feedback, I'll make sure I'll add test coverage before I raise this for review.
Verify the correct behavior (still under debate) when elasticsearch.yml file is non-writable
What is under debate regarding this ?
There are likely other more specific scenarios that probably are better implemented as unit tests for the CLI tool for which no tests currently yet exist.
Makes sense, I think it's better if we keep comments targeted in relevant PRs so that we don't miss anything.
There was a problem hiding this comment.
What is under debate regarding this ?
Like I mentioned, @colings86 and I briefly talked about possibly erroring on startup if we fail to auto-config security rather than pressing forward. No decisions were made but we might want to reconfirm that this is indeed the behavior we want.
This commit introduces TLS auto-configuration for elasticsearch nodes, during the first startup. A number of heuristics are performed in order to determine if the node should get TLS auto-configuration which can also be explicitly disallowed with the use of xpack.security.autoconfiguration.enabled setting. This affects archive installations and docker. Packaged installations are handled in #75144 and #75704 . Co-authored-by: Ioannis Kakavas <[email protected]>
|
Hi @jkakavas, I've updated the changelog YAML for you. |
This commit ensures that for packaged installations
we will run the auto-configuration code on installation (but not upgrade) time.
This is needed because we expect elasticsearch to be run as a service.
By the time the service runs, the configuration directory is not writable by the
user that runs elasticsearch so we can't persist configuration and key/certificate
material on runtime. Running auto-configuration on installation time
allows us to print information to the user that they have better chance of seeing
(barring unattended installations). We don't have the option to show output to the
user when starting the service with systemctl.
During installation we:
in the elasticsearch.keystore. This will be picked up by the node
starting and will be "promoted" to be the cluster wide elastic
password on first startup. (see Auto-configure the elastic user password #78306 )
whether we succeed and what the password of the elastic user is.