-
Notifications
You must be signed in to change notification settings - Fork 281
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
Demo configuration script requires admin password #3329
Demo configuration script requires admin password #3329
Conversation
900c4db
to
bd10ee3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how that password from the config is used, how were you planning on resolving it?
I feel like changes around default credentials or default setup should be focused around The configuration repositories startup workflow so only if its a new security index the default admin user / password is added. Within this thread:
security/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java
Line 121 in 72c6186
bgThread = new Thread(() -> { |
Hi @peternied, thanks for taking a minute to look things over. I am still working on how to use it. It is not used right now because I have not found where I can populate it and guarantee the configurations are live before it tries to access them. It is in draft because I am not quite ready for review--not that I don't appreciate you looking--I just am still working on it and wanted to be able to more easily review changes for my own work then what IntelliJ offers for the git diffs. |
I tried adding modifying the configuration repository to include a method that creates an admin user and updates the configuration for the internal users on bootstrap. Unfortunately, this led to several issues. This is what I implemented:
But we can see in the screenshot one issue--the nodes all run the same configuration repository start process and without a hardcoded hash, they all use their own salt for the password and therefore get different outputs. This would probably work since when a request hits a node it would be able to resolve the password of its own salt, but I think we should avoid them being different. I am not sure the exact issues that could arise, but I suspect allowing the saved hashes to vary is a bad idea. I am going to have to find another way to access the settings and make the configuration change as part of the registration process. |
Seems like something is wrong with how the update to the configuration is happening:
I would expect the admin user to be presented in the check after the index update but it is not. So for some reason, it does not get updated. I also notice that the configuration loader complains that the nodes are not connected:
|
Shard indexing pressure may be killing the node during the write?
|
1e587a1 Is a dynamic insertion of an admin user into the internal user configuration during the node start process. Unfortunately, it does not work because we have no guarantees around the timing or order of the processes. |
Signed-off-by: Stephen Crawford <[email protected]>
e276de6
to
37f5fc4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about only supporting these 3 scenarios - are there others that I am missing?
No password found:
> ./install_demo_configuration.sh
> Unable to find admin password for cluster, please run `export CLUSTER_ADMIN_PASSWORD=$(openssl rand -base64 48 | cut -c1-16)` or create a file {OPENSEARCH_ROOT}/CLUSTER_ADMIN_PASSWORD with a single line that contains the password followed by a newline
Password via environment variable
> export CLUSTER_ADMIN_PASSWORD=thisIsMyOwnPassword
> ./install_demo_configuration.sh
...
> Found admin password in environment variable, please run `echo $CLUSTER_ADMIN_PASSWORD` to see the password
...
# SUCCESS can start ./bin/opensearch and use that password
Password via file
> openssl rand -base64 48 | cut -c1-16 > {OPENSEARCH_ROOT}/CLUSTER_ADMIN_PASSWORD
> ./install_demo_configuration.sh
...
> Found admin password in password file, please run `cat {OPENSEARCH_ROOT}/CLUSTER_ADMIN_PASSWORD` to see the password
...
# SUCCESS can start ./bin/opensearch and use that password
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about only supporting these 3 scenarios - are there others that I am missing?
No password found:
> ./install_demo_configuration.sh
> Unable to find admin password for cluster, please run `export CLUSTER_ADMIN_PASSWORD=$(openssl rand -base64 48 | cut -c1-16)` or create a file {OPENSEARCH_ROOT}/CLUSTER_ADMIN_PASSWORD with a single line that contains the password followed by a newline
Password via environment variable
> export CLUSTER_ADMIN_PASSWORD=thisIsMyOwnPassword
> ./install_demo_configuration.sh
...
> Found admin password in environment variable, please run `echo $CLUSTER_ADMIN_PASSWORD` to see the password
...
# SUCCESS can start ./bin/opensearch and use that password
Password via file
> openssl rand -base64 48 | cut -c1-16 > {OPENSEARCH_ROOT}/CLUSTER_ADMIN_PASSWORD
> ./install_demo_configuration.sh
...
> Found admin password in password file, please run `cat {OPENSEARCH_ROOT}/CLUSTER_ADMIN_PASSWORD` to see the password
...
# SUCCESS can start ./bin/opensearch and use that password
Hi @peternied, that is basically the scenarios I am trying to cover now. The issue is that if we want to remove the default admin password from the internal configuration, we need to be able to provide one for the tests to use. We could swap the tests to use an account other than admin which still had the all access permissions, but I am not sure whether that would be considered just as bad. If you look at the changes I have made this far, you can see the trouble I am running into. I have successfully added logic for a configuration setting to be read from opensearch.yml and used during the install script. I can similarly look for an environment variable--I have not set a name for it yet--should the config value not be present. The problem is that this logic only executes with the installation script and the tests do not run the installation script. We need something that will always add the admin user to the internal configuration whether it is a test or not. Otherwise, every test relying on SingleClusterTest and executing a request with There is no way to run the tests with no default password and no step of test execution which will create the new admin user they require. |
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
I think you can support those 3 scenarios without any Java code changes. Try using exactly the input in those examples to get those results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Darn, might need to figure out a way to generate the password, maybe reuse the hasher? This raises a question I'll look into, why are we using an algorithm that isn't a standard part of openssl.
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
@DarshitChanpura @willyborankin @reta Could I get another look at this change? |
admin:admin
default credentials with operator specified password This change requires an alternative to the default credentials for the admin user. The credentials can be provided to the script via: - `initialAdminPassword` environment variable - a file with a single line that contains the password. The admin password for the cluster will be printed to the console output of the `tools/install_demo_configuration.(bat|sh)` Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Peter Nied <[email protected]> Co-authored-by: Peter Nied <[email protected]> (cherry picked from commit 8628a89) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Backport 8628a89 from #3329. ### Related Issues - Related #3285 Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Peter Nied <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Peter Nied <[email protected]>
Description
This change introduces an alternative to the default credentials
admin:admin
the security plugin currently uses.In this implementation,
a new setting is added to the opensearch.yml file. This setting,During the set up of the security demo config, it will ask user to provide an initial admin password. User can either provide that by defining an environment variable 'initialAdminPassword' with a password string, or create a file 'initialAdminPassword.txt' with a single line that contains the password string and place it under the config folder. The absent of this 'initialAdminPassword' will lead to the failure of the demo config script.plugins.security.authcz.admin.password
is parsed on plugin start up and then propagated into the internal user store of each node when they launch.The setting is used for input into a method in the
UserService
which updates the internal user store entry for theadmin
account. It will replace thehash
field of the account with the hash of the password provided in the yml file.Issues Resolved
admin:admin
default credentials with configuration file password #3285Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.