Correct file ownership on node reconfiguration

[jkakavas]

When running elasticsearch-reconfigure-node to allow a node that
was installed via a package(RPM/DEB) to enroll to an existing
secured cluster, we should ensure that the file ownership is
proper so that elasticsearch can actually read the files when it
starts after reconfiguration.
This commits sets the group owner of the keystore files to
elasticsearch which is the group that we create during
installation.

Partly resolves #80990

[elasticmachine]

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

[albertzaharovits]

@jkakavas Please verify my understanding:

• the package installation creates files owned by the root user and group owned by elasticsearch
• the node-reconfigure-tool must also be run as root, in order to do its job for package installations
• when the tool is run by root, it will create files owned by root and group-owned by elasticsearch, just like the package install

For me, assuming the tools is run as root, while changing the group owner of the files it creates, is the least preferred option.
I would've implemented it by simply complaining, before the tool starts, if the effective user of the tool would create files with different ownership from the ones to be replaced. The user is the free to sudo the tool invocation.
WDYT?

NB: I'm OK with this approach as well, but when we document it we must say that this assumes the instalation is owned by root and group owned by elasticsearch, and that the tool must be invoked as root.

[jkakavas]

@jkakavas Please verify my understanding:

That is correct. This is what is happening with the changes of this PR

For me, assuming the tools is run as root, while changing the group owner of the files it creates, is the least preferred option.
I would've implemented it by simply complaining, before the tool starts, if the effective user of the tool would create files with different ownership from the ones to be replaced. The user is the free to sudo the tool invocation.

Apologies, I didn't follow this.

if the effective user of the tool would create files with different ownership from the ones to be replaced.

The files are meant to be owned by root and group owned by elasticsearch. There is no single user with which the tool can be run so that it produces such an ownership on its own.

The user is the free to sudo the tool invocation.

you mean run this as root but with sudo -g elasticsearch ? I think this is more cumbersome ( if it'd work - I need to check ) and less intuitive but I'm open to discuss

I'm OK with this approach as well, but when we document it we must say that this assumes the instalation is owned by root and group owned by elasticsearch,

We'd need to document that this tool assumes that it's being run immediately after an installation and that it needs to be run as root ( all of our CLI tools have the same requirements in packaged installations due to dir/file ownership ) of a package ( which implies the above ) but I'm ++ to describe this more explicitly.

[albertzaharovits]

For me, assuming the tools is run as root, while changing the group owner of the files it creates, is the least preferred option.
I would've implemented it by simply complaining, before the tool starts, if the effective user of the tool would create files with different ownership from the ones to be replaced. The user is the free to sudo the tool invocation.

Apologies, I didn't follow this.

In other words, I dislike that the tool hardcodes the ownership of the files that it generates.
Instead, I would favor that the tool generate the files with the ownership based on the effective user and group id, like any other process, like it does today, but complain if it detects that the would-be generated files have different permissions from the files it is replacing (like it does today for the owner, but not for the group owner).

The user is the free to sudo the tool invocation.

you mean run this as root but with sudo -g elasticsearch ? I think this is more cumbersome ( if it'd work - I need to check ) and less intuitive but I'm open to discuss

Yes, that's what I mean. The command has to be invoked with sudo anyway. I don't see why being explicit with -g elasticsearch is cumbersome.

This new code is somewhere between an install script and a cmd line utility. It is not invoked by the system package-manager in the root context, during the install (so it is not 100% an install script), but as you say, we ought document that this be manually invoked only after an install (so if it always follows an install, it is part of an install, hence it is an install script).

I think the above to me sounds closer to a cmd line utility (which shouldn't hardcode file ownership), while to you it sounds closer to an install script (where it is fine to hardcode it). I believe this is what we're arguing around.

[albertzaharovits]

Therefore, I'm OK with the proposed change, if what we're building here is an install script in disguise (because it's not quite a "script"), and we document it as such.

My suggestion is that we don't have to do that, that we instead can consider this a regular cmd line tool that has to be invoked with sudo -u root -g elasticsearch (assuming it works).

[jkakavas]

I don't feel strongly about this tbh.. I'll bring in @bytebilly for an extra opinion and some insights about the user experience. In essence, the options are two:

1. We document that this tool needs to be run as root and handle the group ownership behind the curtains ourselves. This means that the user needs to either:

• Be root and run elasticsearch-reconfigure-node
• run sudo elasticsearch-reconfigure-node

2. We document that this tool needs to be run as root with primary group being elasticsearch. This means that the user needs to run ( it does work - I tried this just now )

• sudo -g elasticsearch elasticsearch-reconfigure-node if they are root or
• sudo -u root -g elasticsearch elasticsearch-reconfigure-node if they are logged in as any other user

[bytebilly]

I'm a bit hesitant to require running sudo with -g, as it is a very uncommon pattern and may be missed by users, leading to an inconsistent result. I understand that documentation will report that, but I suspect that most of the time when people see a command starting with sudo, they just run sudo <command> because that's what they are used to do all the time.

If the problem is hardcoding the elasticsearch group name, can't we just check what the group of the current file to replace is, and set the new file with the same value? It's a bit more tricky but it could avoid hardcoding the group name.

[jkakavas]

If the problem is hardcoding the elasticsearch group name, can't we just check what the group of the current file to replace is, and set the new file with the same value? It's a bit more tricky but it could avoid hardcoding the group name.

We are hardcoding this in the package installation script too, this is why I felt it is ok to do this here. I think @albertzaharovits did an awesome job at capturing my mental model without even discussing it, I consider this a continuation of the installation process, thus in my mind this is ok to do

[albertzaharovits]

Thanks for the deliberation @jkakavas , and thanks for the input @bytebilly !
LGTM

[jkakavas]

@elasticmachine update branch

[elasticsearchmachine]

💚 Backport successful

Status	Branch	Result
✅	8.0