Set elastic password from stored hash #76319
Conversation
In package installations, we will be generating the password of the elastic user on installation and we will be storing the hash of it to the elasticsearch.keystore. This change ensures that this password hash will be picked up by the Security plugin in the starting node and will be set as the password of the elastic user in the security index.
| protected void possiblySetElasticPassword(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) { | ||
| if (previousState.equals(SecurityIndexManager.State.UNRECOVERED_STATE) | ||
| && currentState.equals(SecurityIndexManager.State.UNRECOVERED_STATE) == false | ||
| && securityIndex.get().indexExists() == false |
There was a problem hiding this comment.
what should happen if security index gets deleted ? Would bootstrapping this again to set value be valuable/surprising/wrong?
There was a problem hiding this comment.
The idea around securityIndex.get().indexExists() == false is that we should be only setting this as the password of the elastic user, the first time the node starts after a package installation and only if the password of the elastic user hasn't been already set ( which would have triggered the auto-creation of the security index ) Since we are not triggering the creation of the security index during package installation and this would run early enough in the node lifecycle, there should be nothing unrelated that has auto-created the security index.
There was a problem hiding this comment.
I understand that the goal here is to have the elastic user's password hash in the .security index as soon as the index is created.
For this I think it is better to insert this action in the SecurityIndexManager#prepareIndexIfNeededThenExecute, right before calling the execute runnable argument.
Doing it in the action listener adds a level of ambiguity about when the password hash is actually going to take effect, i.e. the keystore seed is a valid password before the index is created, and also after the index is created but before the change password is executed (because cluster change listeners are executed on a different thread, and because the change password action is also "enqued" to be executed).
There was a problem hiding this comment.
For this I think it is better to insert this action in the SecurityIndexManager#prepareIndexIfNeededThenExecute, right before calling the execute runnable argument.
This is part of the alternative approach you are discussing in the other comment , right ?
| @@ -612,6 +619,21 @@ protected Clock getClock() { | |||
| return components; | |||
| } | |||
|
|
|||
| protected void possiblySetElasticPassword(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) { | |||
| if (previousState.equals(SecurityIndexManager.State.UNRECOVERED_STATE) | |||
There was a problem hiding this comment.
wait until the state of the security index has been recovered from either the disk or a cluster state update.
There was a problem hiding this comment.
I rummaged on how best to define this new feature for the elastic password, outside of "it is used by package installations with security auto-configured, and it only works in those circumstances (which might themselves not be obvious in a year's time)".
Let's discuss it, we might need a higher bandwidth to ensure it covers all the corner cases.
| protected void possiblySetElasticPassword(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) { | ||
| if (previousState.equals(SecurityIndexManager.State.UNRECOVERED_STATE) | ||
| && currentState.equals(SecurityIndexManager.State.UNRECOVERED_STATE) == false | ||
| && securityIndex.get().indexExists() == false |
There was a problem hiding this comment.
I understand that the goal here is to have the elastic user's password hash in the .security index as soon as the index is created.
For this I think it is better to insert this action in the SecurityIndexManager#prepareIndexIfNeededThenExecute, right before calling the execute runnable argument.
Doing it in the action listener adds a level of ambiguity about when the password hash is actually going to take effect, i.e. the keystore seed is a valid password before the index is created, and also after the index is created but before the change password is executed (because cluster change listeners are executed on a different thread, and because the change password action is also "enqued" to be executed).
| final ChangePasswordRequest request = new ChangePasswordRequest(); | ||
| request.username("elastic"); | ||
| request.passwordHash(elasticPasswordHash.get().getChars()); | ||
| nativeUsersStore.get().changePassword(request, ActionListener.wrap( |
There was a problem hiding this comment.
If this fails the first time we're in trouble, the "promised" password is never going to be set.
There was a problem hiding this comment.
True, because something else might have created the security index until the second time this node starts.. We could in theory extend the check to be "security index doesn't exist or the doc for the elastic user doesn't exist"
| // Store this because when the listener we register will be called, secure settings will be closed | ||
| if (ELASTIC_PASSWORD_HASH.exists(settings)) { | ||
| elasticPasswordHash.set(ELASTIC_PASSWORD_HASH.get(settings)); | ||
| securityIndex.get().addStateListener(this::possiblySetElasticPassword); |
There was a problem hiding this comment.
As discussed already on another channel, I think we need to better define the "lifecycle" of this new password setting. Right now, it is: "the auto-configured elastic password becomes effective soon after the Security index is created, hopefully".
We've discussed already that I was thinking that it has to be effective as soon as the node can handle requests. We can tease it apart, but, let me propose another alternative:
Currently, user authentication does not trigger the creation of the Security index. It doesn't have to. But I propose we make this new auto-configured password be different in this regard. When a client uses the auto-configured password to authenticate elastic, we try to create the index (if it doesn't already exist) and set the elastic user password, before we confirm the authentication as successful. If any operation fails, ie index creation, indexing, or racing with another node that sets a different password, authentication fails. If the index already exists, no magic happens.
The benefit of this approach is that the client is never going to receive an authentication failure for the "promised" password. They can still get an error (ie fail to create the index) but this is not technically breaking the promise, the node is broken, for other APIs too It also somewhat solves the issue with a different value across the nodes: only the password of the first successful authentication is going to be valid henceforth (at least it is behavior that we can easily explain in words)
I also think it's also worth it to make the auto-configured password take precedence over the bootstrap.password and keystore.seed (ie they are ignored if there is an auto configured password hash). This is a tangent conversation, and I can be convinced otherwise. My reasoning is that it is confusing to technically have multiple passwords valid for the same user at a point in time.
Let me know what you think.
There was a problem hiding this comment.
We've discussed already that I was thinking that it has to be effective as soon as the node can handle requests. We can tease it apart, but, let me propose another alternative:
Yeap, I have the code to validate the password of the elastic user against the hash in the keystore (until it is set in the index) in another branch as we discussed, I'll update this PR shortly
Right now, it is: "the auto-configured elastic password becomes effective soon after the Security index is created, hopefully".
It's not "created", it's "recovered" but I'm being pedantic :) We've agreed that this won't be the case ( see above ). It is more like "the auto-configured elastic password is effective immediately and will be moved to the security index after the Security index is recovered, hopefully"
let me propose another alternative:
...
They can still get an error (ie fail to create the index) but this is not technically breaking the promise
wouldn't this prevent the user to authenticate with the "promised" password before the node has read the cluster state or if it fails to do so ?
solves the issue with a different value across the nodes
I don't think this issue can manifest in packaged installations. It would need two nodes configured to be members of the same cluster (sharing configuration) starting up for the first time at the same time. But:
- For any subsequent node to join an existing cluster, the first node needs to be up and running (so the password would be already set ). It might be the case that we failed to set the password during the boot of the first node, one can rightfully argue, but
- For any subsequent node to join an existing cluster, the user needs to run an external tool to consume the enrollment token and reconfigure the node. This external tool can very well remove the autoconfiguration.password_hash from that node's keystore so that it wouldn't even try to promote this to the index.
++ |
In package installations, we will be generating the password of
the elastic user on installation and we will be storing the hash
of it to the elasticsearch.keystore.
This change ensures that this password hash will be picked up by
the Security plugin in the starting node and will be set as the
password of the elastic user in the security index.