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

[Decision Issue] User Experience for RPM/DEB distributions with default admin credential changes #3916

Closed
derek-ho opened this issue Jan 4, 2024 · 8 comments
Assignees
Labels
enhancement New feature or request help wanted Community contributions are especially encouraged for these issues. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@derek-ho
Copy link
Collaborator

derek-ho commented Jan 4, 2024

Is your feature request related to a problem?
It was recently brought to our attention that the way we are expecting the initial admin password to be set (via env. variable) may be problematic for RPM/DEB distributions. As a refresher, we are expecting the initial admin password to be set via OPENSEARCH_INITIAL_ADMIN_PASSWORD variable on install for example:
sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! dpkg -i opensearch.deb (deb)
sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! yum install opensearch.rpm (rpm)

@peterzhuamazon brought up a few issues with this approach:

  • Users may have sudo access to yum, but but not sudo access to env (this would make them unable to run opensearch)
  • Users may not have experience with the command line and may instead be using GUI's to install (they would not be able to pass in their initial admin password)
  • Install demo configuration is run on the install, not on the start of the service. This means that in case of no/weak password, they would not know about this issue until they try to start the service, at which point the script has already ran and the only way out would be to uninstall/re-install - bad UX

Possible solutions (ranked from low to high effort):

  • Simply print out an error message alerting user to this change
  • Early termination of installation script when we detect that the env variable is not present
  • Inspired by another similar situation run into by the build team, have an interactive prompt which allows user to input initial admin password, we would skip this prompt if the user already has set the necessary env. variable
  • Change the spec files to not run install demo configuration until they start the service (instead of on installation) - this would be significant work, and maybe more of a long term project which requires expertise of build team
@derek-ho derek-ho added enhancement New feature or request untriaged Require the attention of the repository maintainers and may need to be prioritized labels Jan 4, 2024
@smortex
Copy link
Contributor

smortex commented Jan 4, 2024

A bunch of though from my perspective.

About being allowed to pass environment variables with sudo

Users may have sudo access to yum, but but not sudo access to env (this would make them unable to run opensearch)

I would expect users allowed to run yum as root to be basically authorized to run anything and be considered trusted users. I would also not expect untrusted users to be allowed to run any command with random environment variables (sudoers(5) describe how the running environment of run processes can be managed), env being probably one of the worst ideas that come to mind (sudo env LD_PRELOAD=/path/to/my/evil/library.so true) 🙃

So I would not consider the above an issue… but this lead to the next point:

About passing environment variables at install time / on first start

I really don't like this. It feels fragile, it's not something usual when installing a package. For less advanced users, it makes things harder (and as stated above, GUI "wrappers" may lack options to do so).

I would not consider this as a viable option (neither at install time nor at first start).

About stopping with an error at install time

I don't like this neither. Installing is basically about extracting a bunch of files and failing because some run-time requirement are not found is too early. Installation should finish successfully.

About stopping with an error at first start

This feels acceptable. If a configuration file is missing or absent, a service will not start and display an appropriate error. If a required password was not provided, an error message telling how it must be seeded is fine. Interactively prompting the user can be a pain for configuration management tools, so I would not go that way.

Service managers generally have a pre-start section to handle this (e.g. systemd has ExecStartPre=), and check service requirements.

The secret can be read from a configuration file (allowing a configuration management system to install the package, setup the config — including the password — and start the service). Passing the password in the environment can also be an option here (but it means the clear-text secret is available from the context of the application, or from /proc if user can access files there, which concerns me quite a bit).

Avoiding interruption on first start

If a password is required but not available, generating one and printing it to stdout make sense if the user has to change it on first connection.

Locating it can be a pain if there is a lot of output however, which may make this impractical.

About the demo certificates

Change the spec files to not run install demo configuration until they start the service (instead of on installation) - this would be significant work, and maybe more of a long term project which requires expertise of build team

💯

This should definitely be a separate action: installing the package should not generate them, starting the service should not generate them.

Also, if the user wants demo certificates and run a utility that configure them, it must generate the demo CA / service cert+key and admin user cert+key so that all demos do not share the same admin credentials by default.

If these files are missing, the service should refuse to start and says that it could not find the server certificate and key (indicating where they are looked for), and tell that demo PKI can be generated using the command described above.

Summary

IMHO, the least astonishment is provided by a plain configuration file containing the required secrets. The package can install a sample configuration file with commented examples so that the user has to tune the config before being able to start the service for the first time (e.g. the pre-start command check that an admin user has been properly configured).

@stephen-crawford
Copy link
Contributor

[Triage] Hi @derek-ho thank you for filing this issue. This seems well detailed and it is great to see you getting input from various people! Going to mark as triaged since the issue can be closed when a decision is reached etc.

@stephen-crawford stephen-crawford added help wanted Community contributions are especially encouraged for these issues. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Jan 8, 2024
@DarshitChanpura
Copy link
Member

Path Forward for RPM/DEB

Decisions to be made:

Should we keep install demo script inside the spec or postinstall? OR make it its own thing?

  • What are the benefits of moving the script outside?
    • Users do not have to re-download OpenSearch if they failed it. If we keep it as is, after failing, the user needs to redownload:

      • Steps for DEB:
        sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! dpkg -i opensearch-2.12.0-linux-arm64.deb
        • We can add check in preinstall script to verify whether the valid password variable was defined. If not fail it.
      • Steps for RPM:
        sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! yum install opensearch-2.12.0-linux-x64.rpm
        
        • Same as above, we add a check in pre-install section and fail if a valid password is not defined.
    • What are the cons of moving the script outside?

      • More significant changes that need to be tested, unsure about 2.12.0 release time frame

Should we improve logs to indicate what the issue is and how to solve it?

  • Yes, the install scripts should indicate the issue, so it is easier to see that admin credential changes are the cause of new startup errors

Should we support opensearch setup via GUI for people without terminal access? If yes, how?

  • Out of scope. e.g. GNOME supports creation of rcs which can have env variables defined.

Are we supporting people access to env command?

  • No, because it is assumed that people with sudo access to yum command should be granted access to env command as well.
  • If they do not, we are assuming that they can be given access with little concern, based on community feedback: GitHub Issue

Options:

1. Do Nothing:

The status quo is that if users who have access to install package, do not have access to env command, they will not be able to set up demo configuration as >= 2.12 version requires an admin password to be supplied.

Pros:

  • No change to the current implementation for bundling and installing distributions

Cons:

  • Users who do not have access to env command will not be able to use demo configuration

2. Modify the install script (DEB) and spec file (RPM) to read the password from a config file

This requires end users to create a config file that contains this admin password value. Further modification is needed to install and spec files to read this value from config file and set it in the environment to be picked up by the demo installation script.

Pros:

  • Users without sudo access to env command can now pass the password and utilize demo installation script.

Cons:

  • Storing clear-text passwords in a file is not a great security posture
  • Requires significant changes to spec and install files

3. Move demo config setup to the opensearch service startup phase

This involves changing the startup process to now have an additional check for install demo configuration as ExecStartPre=. This is a significant change to service startup flow as well as experience and would involve modifying a stable startup process.

Pros:

  • No need to re-install if installation fails
  • Organized env variables in env file

Cons:

  • Plain text password in config
  • Alters a very stable startup system

4. Update the pre-install script (PROPOSED)

This requires adding an extra check in the pre-install script to check whether the env variable is defined and if so whether the password is valid. (Yes, this will be on top of the weak password validation done by the tool itself). In addition, the script should print clear messages on what can users do next. This change is small, and the installation will quit before the package is installed leaving a clear message for the user that the password needs to be supplied to spin-up a >=2.12 cluster.

Pros:

  • Able to inform user early on. (No broken installations)
  • Don’t have to change the startup service setup

Cons:

  • Messy user experience during setup leading to potential doubts about stability of the package itself
  • Users probably won’t read doc and lead to decrease in user base
  • Need to add checks for the presence of env variable and validation check to give the best user experience

@derek-ho
Copy link
Collaborator Author

derek-ho commented Jan 8, 2024

Path Forward for RPM/DEB

Decisions to be made:

Should we keep install demo script inside the spec or postinstall? OR make it its own thing?

  • What are the benefits of moving the script outside?

    • Users do not have to re-download OpenSearch if they failed it. If we keep it as is, after failing, the user needs to redownload:

      • Steps for DEB:

        sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! dpkg -i opensearch-2.12.0-linux-arm64.deb
        • We can add check in preinstall script to verify whether the valid password variable was defined. If not fail it.
      • Steps for RPM:

        sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! yum install opensearch-2.12.0-linux-x64.rpm
        
        • Same as above, we add a check in pre-install section and fail if a valid password is not defined.
    • What are the cons of moving the script outside?

      • More significant changes that need to be tested, unsure about 2.12.0 release time frame

Should we improve logs to indicate what the issue is and how to solve it?

  • Yes, the install scripts should indicate the issue, so it is easier to see that admin credential changes are the cause of new startup errors

Should we support opensearch setup via GUI for people without terminal access? If yes, how?

  • Out of scope. e.g. GNOME supports creation of rcs which can have env variables defined.

Are we supporting people access to env command?

  • No, because it is assumed that people with sudo access to yum command should be granted access to env command as well.
  • If they do not, we are assuming that they can be given access with little concern, based on community feedback: GitHub Issue

Options:

1. Do Nothing:

The status quo is that if users who have access to install package, do not have access to env command, they will not be able to set up demo configuration as >= 2.12 version requires an admin password to be supplied.

Pros:

  • No change to the current implementation for bundling and installing distributions

Cons:

  • Users who do not have access to env command will not be able to use demo configuration

2. Modify the install script (DEB) and spec file (RPM) to read the password from a config file

This requires end users to create a config file that contains this admin password value. Further modification is needed to install and spec files to read this value from config file and set it in the environment to be picked up by the demo installation script.

Pros:

  • Users without sudo access to env command can now pass the password and utilize demo installation script.

Cons:

  • Storing clear-text passwords in a file is not a great security posture
  • Requires significant changes to spec and install files

3. Move demo config setup to the opensearch service startup phase

This involves changing the startup process to now have an additional check for install demo configuration as ExecStartPre=. This is a significant change to service startup flow as well as experience and would involve modifying a stable startup process.

Pros:

  • No need to re-install if installation fails
  • Organized env variables in env file

Cons:

  • Plain text password in config
  • Alters a very stable startup system

4. Update the pre-install script (PROPOSED)

This requires adding an extra check in the pre-install script to check whether the env variable is defined and if so whether the password is valid. (Yes, this will be on top of the weak password validation done by the tool itself). In addition, the script should print clear messages on what can users do next. This change is small, and the installation will quit before the package is installed leaving a clear message for the user that the password needs to be supplied to spin-up a >=2.12 cluster.

Pros:

  • Able to inform user early on. (No broken installations)
  • Don’t have to change the startup service setup

Cons:

  • Messy user experience during setup leading to potential doubts about stability of the package itself
  • Users probably won’t read doc and lead to decrease in user base
  • Need to add checks for the presence of env variable and validation check to give the best user experience

IMHO we cannot support password validation in the script - although we have regex that we can use, we also have an additional requirements of strength provided by a library: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java#L103, unless we convert that to a tool (which would be non-trivial), we can only check the existence and regex, but not guarantee that it would pass the password strength requirement overall.

@DarshitChanpura
Copy link
Member

IMHO we cannot support password validation in the script - although we have regex that we can use, we also have an additional requirements of strength provided by a library: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java#L103, unless we convert that to a tool (which would be non-trivial), we can only check the existence and regex, but not guarantee that it would pass the password strength requirement overall.

Agreed. To keep it uniform, let's only check if the variable is defined as part of pre-install. We can add a log if there are any errors. To add more logging and make it easier for end-users, we can also catch the error, if any, thrown by the demo install script , and print out a message redirecting users to check the install script logs. Note: This approach prereqs the user already has access to read the logs.

@DarshitChanpura
Copy link
Member

Here's the updated proposal number 4:

4. Update the pre-install script (PROPOSED)

This requires adding an extra check in the pre-install script to check whether the env variable is defined. In addition, the script should print clear messages on what can users do next. This change is small, and the installation will quit before the package is installed leaving a clear message for the user that the password needs to be supplied to spin-up a >=2.12 cluster.
In case, if a variable is defined and if it turns out to be weak, then the demo install will fail with -1. We then print out another console log from the post-install script asking users to check install_demo_configuration.log for more information.

Pros:

  • Able to inform user early on. (No broken installations if no variable is defined)
  • Able to inform user to check for logs from post-install script if there were any failures.
  • Don’t have to change the startup service setup

Cons:

  • Messy user experience during setup leading to potential doubts about stability of the package itself
  • Users probably won’t read doc and lead to decrease in user base
  • Need to add checks for the presence of env variable and a postinstall check to give the best user experience

@DarshitChanpura
Copy link
Member

DarshitChanpura commented Jan 10, 2024

FInal Implementation

  • Preinstall fails if no OPENSEARCH_INITIAL_ADMIN_PASSWORD is defined for OpenSearch >= 2.12. A clear message is printed out to let users know that this variable is required. (Package will not be installed)
ERROR: Opensearch 2.12 and later requires the env variable OPENSEARCH_INITIAL_ADMIN_PASSWORD to be defined to setup the opensearch-security demo configuration
  • Postinstall fails if a weak password is supplied, e.g. 'admin'. A message is printed in the logs to redirect users to the install_demo_configuration.log file to check what went wrong. (Package will be installed and users may have to uninstall the package (or at-least delete the config files))
ERROR: Something went wrong during demo configuration installation. Please see the logs in %{log_dir}/install_demo_configuration.log.

(${log_dir} above for DEB)

  • Installation will only be successful if OPENSEARCH_INITIAL_ADMIN_PASSWORD is defined and a strong password is supplied as the value.

Tests:

PR:

@DarshitChanpura
Copy link
Member

Feel free to re-open if any more input required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Community contributions are especially encouraged for these issues. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
None yet
Development

No branches or pull requests

4 participants