From 7d0cb66b3768a7ddd31423e93e98735e246ab0d5 Mon Sep 17 00:00:00 2001 From: Dan Cecoi Date: Wed, 17 Apr 2024 14:03:47 +0100 Subject: [PATCH] Add support for PBKDF2, Scrypt, and Argon2 for password hashing (#4079) Signed-off-by: Dan Cecoi --- build.gradle | 2 + plugin-security.policy | 3 + .../api/AccountRestApiIntegrationTest.java | 9 +- .../test/framework/TestSecurityConfig.java | 21 +-- .../security/OpenSearchSecurityPlugin.java | 134 ++++++++++++++++++ .../dlic/rest/api/AccountApiAction.java | 14 +- .../dlic/rest/api/InternalUsersApiAction.java | 9 +- .../security/dlic/rest/support/Utils.java | 21 +-- .../security/hash/PasswordHash.java | 8 ++ .../security/hash/PasswordHashImpl.java | 129 +++++++++++++++++ .../security/support/ConfigConstants.java | 33 +++++ .../org/opensearch/security/tools/Hasher.java | 17 ++- .../opensearch/security/user/UserService.java | 14 +- .../dlic/dlsfls/DfmOverwritesAllTest.java | 3 +- .../api/AbstractApiActionValidationTest.java | 6 + ...AccountApiActionConfigValidationsTest.java | 9 +- .../InternalUsersApiActionValidationTest.java | 4 +- 17 files changed, 378 insertions(+), 58 deletions(-) create mode 100644 src/main/java/org/opensearch/security/hash/PasswordHash.java create mode 100644 src/main/java/org/opensearch/security/hash/PasswordHashImpl.java diff --git a/build.gradle b/build.gradle index a4e4a7d037..7df09e54d0 100644 --- a/build.gradle +++ b/build.gradle @@ -581,6 +581,8 @@ dependencies { implementation 'org.ldaptive:ldaptive:1.2.3' implementation 'com.nimbusds:nimbus-jose-jwt:9.37.3' + implementation 'com.password4j:password4j:1.7.3' + //JWT implementation "io.jsonwebtoken:jjwt-api:${jjwt_version}" implementation "io.jsonwebtoken:jjwt-impl:${jjwt_version}" diff --git a/plugin-security.policy b/plugin-security.policy index d4e1fe6998..5db1a7ef97 100644 --- a/plugin-security.policy +++ b/plugin-security.policy @@ -77,6 +77,9 @@ grant { //Enable this permission to debug unauthorized de-serialization attempt //permission java.io.SerializablePermission "enableSubstitution"; + + permission java.io.FilePermission "/psw4j.properties","read"; + }; grant codeBase "${codebase.netty-common}" { diff --git a/src/integrationTest/java/org/opensearch/security/api/AccountRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AccountRestApiIntegrationTest.java index 7fa298c1e4..90b925d34f 100644 --- a/src/integrationTest/java/org/opensearch/security/api/AccountRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/AccountRestApiIntegrationTest.java @@ -13,14 +13,17 @@ import org.junit.Test; import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.security.hash.PasswordHash; +import org.opensearch.security.hash.PasswordHashImpl; import org.opensearch.test.framework.TestSecurityConfig; import org.opensearch.test.framework.cluster.TestRestClient; +import java.nio.CharBuffer; + import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.not; -import static org.opensearch.security.dlic.rest.support.Utils.hash; public class AccountRestApiIntegrationTest extends AbstractApiIntegrationTest { @@ -106,11 +109,13 @@ private void verifyWrongPayload(final TestRestClient client) throws Exception { private void verifyPasswordCanBeChanged() throws Exception { final var newPassword = randomAlphabetic(10); + final PasswordHash passwordService = new PasswordHashImpl(); withUser( TEST_USER, TEST_USER_PASSWORD, client -> ok( - () -> client.putJson(accountPath(), changePasswordWithHashPayload(TEST_USER_PASSWORD, hash(newPassword.toCharArray()))) + () -> client.putJson(accountPath(), changePasswordWithHashPayload(TEST_USER_PASSWORD, + passwordService.hash(CharBuffer.wrap(newPassword.toCharArray())))) ) ); withUser( diff --git a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java index 79f10a76cf..ee88d96616 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java +++ b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java @@ -31,7 +31,8 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.nio.ByteBuffer; -import java.security.SecureRandom; +import java.nio.CharBuffer; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -46,7 +47,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.bouncycastle.crypto.generators.OpenBSDBCrypt; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.index.IndexRequest; @@ -57,6 +57,8 @@ import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.security.hash.PasswordHash; +import org.opensearch.security.hash.PasswordHashImpl; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.test.framework.cluster.OpenSearchClientProvider.UserCredentialsHolder; @@ -451,7 +453,7 @@ public Object getAttribute(String attributeName) { public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException { xContentBuilder.startObject(); - xContentBuilder.field("hash", hash(password.toCharArray())); + xContentBuilder.field("hash", hash(password)); Set roleNames = getRoleNames(); @@ -933,13 +935,12 @@ public void updateInternalUsersConfiguration(Client client, List users) { updateConfigInIndex(client, CType.INTERNALUSERS, userMap); } - static String hash(final char[] clearTextPassword) { - final byte[] salt = new byte[16]; - new SecureRandom().nextBytes(salt); - final String hash = OpenBSDBCrypt.generate((Objects.requireNonNull(clearTextPassword)), salt, 12); - Arrays.fill(salt, (byte) 0); - Arrays.fill(clearTextPassword, '\0'); - return hash; + static String hash(final String clearTextPassword) { + PasswordHash passwordService = new PasswordHashImpl(); + return passwordService.hash(( + Objects.requireNonNull( + CharBuffer.wrap(clearTextPassword.toCharArray()) + ))); } private void writeEmptyConfigToIndex(Client client, CType configType) { diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 6e3c22e695..e71eb4b5b6 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1916,6 +1916,140 @@ public List> getSettings() { ); } + + //todo: What is Property.Filtered? + //todo: Do we want these properties to be Final? + settings.add( + Setting.simpleString( + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + ) + ); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.simpleString( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR, + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES, + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION, + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH, + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_MEMORY, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_MEMORY_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_LENGTH, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_LENGTH_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_PARALLELISM, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_PARALLELISM_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.simpleString( + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_TYPE, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_TYPE_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_VERSION, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_VERSION_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + + settings.add(Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + + settings.add(Setting.simpleString( + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR_DEFAULT, + Property.NodeScope, + Property.Filtered, + Property.Final + )); + return settings; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java index 5d81dfa85d..025f4dd0a0 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java @@ -11,6 +11,7 @@ package org.opensearch.security.dlic.rest.api; +import java.nio.CharBuffer; import java.util.List; import java.util.Map; import java.util.Set; @@ -19,8 +20,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import org.apache.commons.lang3.tuple.Triple; -import org.bouncycastle.crypto.generators.OpenBSDBCrypt; - import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.core.common.Strings; @@ -32,6 +31,8 @@ import org.opensearch.security.dlic.rest.validation.RequestContentValidator; import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType; import org.opensearch.security.dlic.rest.validation.ValidationResult; +import org.opensearch.security.hash.PasswordHash; +import org.opensearch.security.hash.PasswordHashImpl; import org.opensearch.security.securityconf.Hashed; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; @@ -44,13 +45,15 @@ import static org.opensearch.security.dlic.rest.api.Responses.ok; import static org.opensearch.security.dlic.rest.api.Responses.response; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; -import static org.opensearch.security.dlic.rest.support.Utils.hash; /** * Rest API action to fetch or update account details of the signed-in user. * Currently this action serves GET and PUT request for /_opendistro/_security/api/account endpoint */ public class AccountApiAction extends AbstractApiAction { + + private final PasswordHash passwordService; + private static final List routes = addRoutesPrefix( ImmutableList.of(new Route(Method.GET, "/account"), new Route(Method.PUT, "/account")) ); @@ -62,6 +65,7 @@ public AccountApiAction( ) { super(Endpoint.ACCOUNT, clusterService, threadPool, securityApiDependencies); this.requestHandlersBuilder.configureRequestHandlers(this::accountApiRequestHandlers); + this.passwordService = new PasswordHashImpl(securityApiDependencies.settings()); } @Override @@ -132,7 +136,7 @@ ValidationResult validCurrentPassword(final SecurityConfi final var currentPassword = content.get("current_password").asText(); final var internalUserEntry = (Hashed) securityConfiguration.configuration().getCEntry(username); final var currentHash = internalUserEntry.getHash(); - if (currentHash == null || !OpenBSDBCrypt.checkPassword(currentHash, currentPassword.toCharArray())) { + if (currentHash == null || !passwordService.check(CharBuffer.wrap(currentPassword.toCharArray()), currentHash)) { return ValidationResult.error(RestStatus.BAD_REQUEST, badRequestMessage("Could not validate your current password.")); } return ValidationResult.success(securityConfiguration); @@ -148,7 +152,7 @@ ValidationResult updatePassword(final SecurityConfigurati if (Strings.isNullOrEmpty(password)) { hash = securityJsonNode.get("hash").asString(); } else { - hash = hash(password.toCharArray()); + hash = this.passwordService.hash(CharBuffer.wrap(password.toCharArray())); } if (Strings.isNullOrEmpty(hash)) { return ValidationResult.error( diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java index 3cbcc18bd9..df2806a531 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java @@ -12,6 +12,7 @@ package org.opensearch.security.dlic.rest.api; import java.io.IOException; +import java.nio.CharBuffer; import java.util.List; import java.util.Map; @@ -31,6 +32,8 @@ import org.opensearch.security.dlic.rest.validation.RequestContentValidator; import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType; import org.opensearch.security.dlic.rest.validation.ValidationResult; +import org.opensearch.security.hash.PasswordHash; +import org.opensearch.security.hash.PasswordHashImpl; import org.opensearch.security.securityconf.Hashed; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; @@ -47,7 +50,6 @@ import static org.opensearch.security.dlic.rest.api.Responses.payload; import static org.opensearch.security.dlic.rest.api.Responses.response; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; -import static org.opensearch.security.dlic.rest.support.Utils.hash; public class InternalUsersApiAction extends AbstractApiAction { @@ -57,6 +59,8 @@ protected void consumeParameters(final RestRequest request) { request.param("filterBy"); } + private final PasswordHash passwordService; + static final List RESTRICTED_FROM_USERNAME = ImmutableList.of( ":" // Not allowed in basic auth, see https://stackoverflow.com/a/33391003/533057 ); @@ -92,6 +96,7 @@ public InternalUsersApiAction( super(Endpoint.INTERNALUSERS, clusterService, threadPool, securityApiDependencies); this.userService = userService; this.requestHandlersBuilder.configureRequestHandlers(this::internalUsersApiRequestHandlers); + this.passwordService = new PasswordHashImpl(securityApiDependencies.settings()); } @Override @@ -268,7 +273,7 @@ private ValidationResult generateHashForPassword(final Se if (content.has("password")) { final var plainTextPassword = content.get("password").asText(); content.remove("password"); - content.put("hash", hash(plainTextPassword.toCharArray())); + content.put("hash", passwordService.hash(CharBuffer.wrap(plainTextPassword.toCharArray()))); } return ValidationResult.success(securityConfiguration); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/support/Utils.java b/src/main/java/org/opensearch/security/dlic/rest/support/Utils.java index ee68a629c6..11255d2884 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/support/Utils.java +++ b/src/main/java/org/opensearch/security/dlic/rest/support/Utils.java @@ -13,15 +13,14 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.nio.CharBuffer; import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; -import java.security.SecureRandom; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import com.google.common.collect.ImmutableList; @@ -31,7 +30,6 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.commons.lang3.tuple.Pair; -import org.bouncycastle.crypto.generators.OpenBSDBCrypt; import org.opensearch.ExceptionsHelper; import org.opensearch.OpenSearchParseException; @@ -54,6 +52,7 @@ import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.User; + import static org.opensearch.core.xcontent.DeprecationHandler.THROW_UNSUPPORTED_OPERATION; import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; @@ -195,21 +194,6 @@ public static Map byteArrayToMutableJsonMap(byte[] jsonBytes) th } } - /** - * This generates hash for a given password - * @param clearTextPassword plain text password for which hash should be generated. - * This will be cleared from memory. - * @return hash of the password - */ - public static String hash(final char[] clearTextPassword) { - final byte[] salt = new byte[16]; - new SecureRandom().nextBytes(salt); - final String hash = OpenBSDBCrypt.generate((Objects.requireNonNull(clearTextPassword)), salt, 12); - Arrays.fill(salt, (byte) 0); - Arrays.fill(clearTextPassword, '\0'); - return hash; - } - /** * Generate field resource paths * @param fields fields @@ -287,5 +271,4 @@ public static T withIOException(final CheckedSupplier action throw new UncheckedIOException(ioe); } } - } diff --git a/src/main/java/org/opensearch/security/hash/PasswordHash.java b/src/main/java/org/opensearch/security/hash/PasswordHash.java new file mode 100644 index 0000000000..a7f50d275d --- /dev/null +++ b/src/main/java/org/opensearch/security/hash/PasswordHash.java @@ -0,0 +1,8 @@ +package org.opensearch.security.hash; + +import java.nio.CharBuffer; + +public interface PasswordHash { + String hash(CharBuffer password); + boolean check(CharBuffer password, String hashedPassword); +} diff --git a/src/main/java/org/opensearch/security/hash/PasswordHashImpl.java b/src/main/java/org/opensearch/security/hash/PasswordHashImpl.java new file mode 100644 index 0000000000..9831732d0f --- /dev/null +++ b/src/main/java/org/opensearch/security/hash/PasswordHashImpl.java @@ -0,0 +1,129 @@ +package org.opensearch.security.hash; + +import com.password4j.*; +import com.password4j.types.Argon2; +import com.password4j.types.Bcrypt; +import org.opensearch.common.settings.Settings; +import org.opensearch.security.support.ConfigConstants; + +import java.nio.CharBuffer; +import java.util.Arrays; + +public class PasswordHashImpl implements PasswordHash { + + private final Settings settings; + + public PasswordHashImpl(Settings settings) { + this.settings = settings; + } + + public PasswordHashImpl(){ + this(Settings.EMPTY); + }; + + + @Override + public String hash(CharBuffer password) { + try { + return Password + .hash(password) + .with(getHashingFunction()) + .getResult(); + } finally { + cleanup(password); + } + } + + @Override + public boolean check(CharBuffer password, String hash) { + try { + return Password + .check(password, hash) + .with(getHashingFunction()); + } finally { + cleanup(password); + } + } + + private void cleanup(CharBuffer password) { + password.clear(); + char[] passwordOverwrite = new char[password.capacity()]; + Arrays.fill(passwordOverwrite, '\0'); + password.put(passwordOverwrite); + password.clear(); + } + + + private HashingFunction getHashingFunction() { + + String algorithm = settings.get(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM_DEFAULT); + + HashingFunction hashingFunction; + switch(algorithm){ + case "pbkdf2": + hashingFunction = getPBKDF2Function(); + break; + case "scrypt": + hashingFunction = getSCryptFunction(); + break; + case "argon2": + hashingFunction = getArgon2Function(); + break; + default: case "bcrypt": + hashingFunction = getBCryptFunction(); + break; + } + return hashingFunction; + } + + + private HashingFunction getPBKDF2Function() { + int iterations = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT); + int length = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT); + String pbkdf2Algorithm = settings.get(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM_DEFAULT); + return PBKDF2Function.getInstance(pbkdf2Algorithm, iterations, length); + } + + private HashingFunction getSCryptFunction() { + int workFactor = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR, + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR_DEFAULT); + int resources = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES, + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES_DEFAULT); + int parallelization = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION, + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION_DEFAULT); + int derivedKeyLength = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH, + ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH_DEFAULT); + //derivedKeyLength is not necessary + return ScryptFunction.getInstance(workFactor, resources, parallelization, derivedKeyLength); + } + + private HashingFunction getArgon2Function() { + int memory = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_MEMORY, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_MEMORY_DEFAULT); + int iterations = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS_DEFAULT); + int length = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_LENGTH, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_LENGTH_DEFAULT); + int parallelism = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_LENGTH, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_PARALLELISM_DEFAULT); + String type = settings.get(ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_TYPE, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_TYPE_DEFAULT); + int version = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_VERSION, + ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_VERSION_DEFAULT); + return Argon2Function.getInstance( + memory, iterations, parallelism, length, Argon2.valueOf(type.toUpperCase()), version); + } + + private HashingFunction getBCryptFunction() { + int rounds = settings.getAsInt(ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS_DEFAULT); + String minor = settings.get(ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR_DEFAULT); + return BcryptFunction.getInstance(Bcrypt.valueOf(minor), rounds); + } + +} diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index 5169d02d20..580b13a6c3 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -145,6 +145,39 @@ public class ConfigConstants { public static final String SECURITY_AUTHCZ_IMPERSONATION_DN = "plugins.security.authcz.impersonation_dn"; public static final String SECURITY_AUTHCZ_REST_IMPERSONATION_USERS = "plugins.security.authcz.rest_impersonation_user"; + public static final String SECURITY_PASSWORD_HASHING_ALGORITHM = "plugins.security.password.hashing.algorithm"; + public static final String SECURITY_PASSWORD_HASHING_ALGORITHM_DEFAULT = "bcrypt"; + public static final String SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS = "plugins.security.password.hashing.pbkdf2.iterations"; + public static final int SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT = 310000; + public static final String SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH = "plugins.security.password.hashing.pbkdf2.length"; + public static final int SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT = 512; + public static final String SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM = "plugins.security.password.hashing.pbkdf2.algorithm"; + public static final String SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM_DEFAULT = "PBKDF2WithHmacSHA256"; + public static final String SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR = "plugins.security.password.hashing.scrypt.work_factor"; + public static final int SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR_DEFAULT = 65536; + public static final String SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES = "plugins.security.password.hashing.scrypt.resources"; + public static final int SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES_DEFAULT = 8; + public static final String SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION = "plugins.security.password.hashing.scrypt.parallelization"; + public static final int SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION_DEFAULT = 1; + public static final String SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH = "plugins.security.password.hashing.scrypt.derived_key_length"; + public static final int SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH_DEFAULT = 64; + public static final String SECURITY_PASSWORD_HASHING_ARGON2_MEMORY = "plugins.security.password.hashing.argon2.memory"; + public static final int SECURITY_PASSWORD_HASHING_ARGON2_MEMORY_DEFAULT = 15360; + public static final String SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS = "plugins.security.password.hashing.argon2.iterations"; + public static final int SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS_DEFAULT = 2; + public static final String SECURITY_PASSWORD_HASHING_ARGON2_LENGTH = "plugins.security.password.hashing.argon2.length"; + public static final int SECURITY_PASSWORD_HASHING_ARGON2_LENGTH_DEFAULT = 32; + public static final String SECURITY_PASSWORD_HASHING_ARGON2_PARALLELISM = "plugins.security.password.hashing.argon2.parallelism"; + public static final int SECURITY_PASSWORD_HASHING_ARGON2_PARALLELISM_DEFAULT = 1; + public static final String SECURITY_PASSWORD_HASHING_ARGON2_TYPE = "plugins.security.password.hashing.argon2.type"; + public static final String SECURITY_PASSWORD_HASHING_ARGON2_TYPE_DEFAULT = "id"; + public static final String SECURITY_PASSWORD_HASHING_ARGON2_VERSION = "plugins.security.password.hashing.argon2.version"; + public static final int SECURITY_PASSWORD_HASHING_ARGON2_VERSION_DEFAULT = 19; + public static final String SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS = "plugins.security.password.hashing.bcrypt.rounds"; + public static final int SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS_DEFAULT = 10; + public static final String SECURITY_PASSWORD_HASHING_BCRYPT_MINOR = "plugins.security.password.hashing.bcrypt.minor"; + public static final String SECURITY_PASSWORD_HASHING_BCRYPT_MINOR_DEFAULT = "B"; + public static final String SECURITY_AUDIT_TYPE_DEFAULT = "plugins.security.audit.type"; public static final String SECURITY_AUDIT_CONFIG_DEFAULT = "plugins.security.audit.config"; public static final String SECURITY_AUDIT_CONFIG_ROUTES = "plugins.security.audit.routes"; diff --git a/src/main/java/org/opensearch/security/tools/Hasher.java b/src/main/java/org/opensearch/security/tools/Hasher.java index f19d958523..e1d1ecfaa3 100644 --- a/src/main/java/org/opensearch/security/tools/Hasher.java +++ b/src/main/java/org/opensearch/security/tools/Hasher.java @@ -27,8 +27,7 @@ package org.opensearch.security.tools; import java.io.Console; -import java.security.SecureRandom; -import java.util.Arrays; +import java.nio.CharBuffer; import java.util.Objects; import org.apache.commons.cli.CommandLine; @@ -37,7 +36,9 @@ import org.apache.commons.cli.HelpFormatter; import org.apache.commons.cli.Option; import org.apache.commons.cli.Options; -import org.bouncycastle.crypto.generators.OpenBSDBCrypt; +import org.opensearch.common.settings.Settings; +import org.opensearch.security.hash.PasswordHash; +import org.opensearch.security.hash.PasswordHashImpl; public class Hasher { @@ -82,11 +83,9 @@ public static void main(final String[] args) { } public static String hash(final char[] clearTextPassword) { - final byte[] salt = new byte[16]; - new SecureRandom().nextBytes(salt); - final String hash = OpenBSDBCrypt.generate((Objects.requireNonNull(clearTextPassword)), salt, 12); - Arrays.fill(salt, (byte) 0); - Arrays.fill(clearTextPassword, '\0'); - return hash; + //todo: figure out if this script is used for hashing a big number of passwords because if it is we shouldn't create a new PasswordHasher object for each hash + String password = new String(clearTextPassword); + PasswordHash passwordService = new PasswordHashImpl(Settings.EMPTY); + return passwordService.hash((Objects.requireNonNull(CharBuffer.wrap(password.toCharArray())))); } } diff --git a/src/main/java/org/opensearch/security/user/UserService.java b/src/main/java/org/opensearch/security/user/UserService.java index 937a5331a8..7512a50c0f 100644 --- a/src/main/java/org/opensearch/security/user/UserService.java +++ b/src/main/java/org/opensearch/security/user/UserService.java @@ -12,6 +12,7 @@ package org.opensearch.security.user; import java.io.IOException; +import java.nio.CharBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; @@ -45,6 +46,8 @@ import org.opensearch.identity.tokens.BasicAuthToken; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.configuration.ConfigurationRepository; +import org.opensearch.security.hash.PasswordHash; +import org.opensearch.security.hash.PasswordHashImpl; import org.opensearch.security.securityconf.DynamicConfigFactory; import org.opensearch.security.securityconf.Hashed; import org.opensearch.security.securityconf.impl.CType; @@ -57,7 +60,6 @@ import org.passay.EnglishCharacterData; import org.passay.PasswordGenerator; -import static org.opensearch.security.dlic.rest.support.Utils.hash; /** * This class handles user registration and operations on behalf of the Security Plugin. @@ -69,6 +71,8 @@ public class UserService { private final ConfigurationRepository configurationRepository; String securityIndex; Client client; + Settings settings; + private final PasswordHash passwordService; User tokenUser; final static String NO_PASSWORD_OR_HASH_MESSAGE = "Please specify either 'hash' or 'password' when creating a new internal user."; @@ -102,6 +106,8 @@ public UserService(ClusterService clusterService, ConfigurationRepository config ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX ); this.client = client; + this.settings = settings; + this.passwordService = new PasswordHashImpl(settings); } /** @@ -142,7 +148,7 @@ public SecurityDynamicConfiguration createOrUpdateAccount(ObjectNode contentA // service account verifyServiceAccount(securityJsonNode, accountName); String password = generatePassword(); - contentAsNode.put("hash", hash(password.toCharArray())); + contentAsNode.put("hash", passwordService.hash(CharBuffer.wrap(password.toCharArray()))); contentAsNode.put("service", "true"); } else { contentAsNode.put("service", "false"); @@ -162,7 +168,7 @@ public SecurityDynamicConfiguration createOrUpdateAccount(ObjectNode contentA final String origHash = securityJsonNode.get("hash").asString(); if (plainTextPassword != null && plainTextPassword.length() > 0) { contentAsNode.remove("password"); - contentAsNode.put("hash", hash(plainTextPassword.toCharArray())); + contentAsNode.put("hash", passwordService.hash(CharBuffer.wrap(plainTextPassword.toCharArray()))); } else if (origHash != null && origHash.length() > 0) { contentAsNode.remove("password"); } else if (plainTextPassword != null && plainTextPassword.isEmpty() && origHash == null) { @@ -275,7 +281,7 @@ public AuthToken generateAuthToken(String accountName) throws IOException { // Generate a new password for the account and store the hash of it String plainTextPassword = generatePassword(); - contentAsNode.put("hash", hash(plainTextPassword.toCharArray())); + contentAsNode.put("hash", passwordService.hash(CharBuffer.wrap(plainTextPassword.toCharArray()))); contentAsNode.put("enabled", "true"); contentAsNode.put("service", "true"); diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/DfmOverwritesAllTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/DfmOverwritesAllTest.java index 580cabc66b..0e0c838baa 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/DfmOverwritesAllTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/DfmOverwritesAllTest.java @@ -219,7 +219,8 @@ public void testDFMRestrictedAndUnrestrictedAllIndices() throws Exception { */ @Test public void testDFMRestrictedAndUnrestrictedOneIndex() throws Exception { - final Settings settings = Settings.builder().put(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, true).build(); + final Settings settings = Settings.builder().put(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, true) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM_DEFAULT).build(); setup( settings, new DynamicSecurityConfig().setConfig("securityconfig_dfm_empty_overwrites_all.yml") diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java index 4b2e9e4417..bf4a78d66e 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java @@ -28,6 +28,8 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.configuration.ConfigurationRepository; +import org.opensearch.security.hash.PasswordHash; +import org.opensearch.security.hash.PasswordHashImpl; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.threadpool.ThreadPool; @@ -62,6 +64,8 @@ public abstract class AbstractApiActionValidationTest { ObjectMapper objectMapper = DefaultObjectMapper.objectMapper; + PasswordHash passwordService; + @Before public void setup() { securityApiDependencies = new SecurityApiDependencies( @@ -73,6 +77,8 @@ public void setup() { null, Settings.EMPTY ); + + passwordService = new PasswordHashImpl(securityApiDependencies.settings()); } void setupRolesConfiguration() throws IOException { diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiActionConfigValidationsTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiActionConfigValidationsTest.java index 8af780e01a..85dacb070d 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiActionConfigValidationsTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiActionConfigValidationsTest.java @@ -14,11 +14,12 @@ import org.bouncycastle.crypto.generators.OpenBSDBCrypt; import org.opensearch.core.rest.RestStatus; -import org.opensearch.security.dlic.rest.support.Utils; import org.opensearch.security.securityconf.impl.v7.InternalUserV7; import org.mockito.Mockito; +import java.nio.CharBuffer; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -35,7 +36,7 @@ public void verifyValidCurrentPassword() { assertFalse(result.isValid()); assertEquals(RestStatus.BAD_REQUEST, result.status()); - u.setHash(Utils.hash("aaaa".toCharArray())); + u.setHash(passwordService.hash(CharBuffer.wrap("aaaa".toCharArray()))); result = accountApiAction.validCurrentPassword(SecurityConfiguration.of(requestContent(), "u", configuration)); assertTrue(result.isValid()); } @@ -59,7 +60,7 @@ public void updatePassword() { assertTrue(OpenBSDBCrypt.checkPassword(u.getHash(), "cccccc".toCharArray())); requestContent.remove("password"); - requestContent.put("hash", Utils.hash("dddddd".toCharArray())); + requestContent.put("hash", passwordService.hash(CharBuffer.wrap("dddddd".toCharArray()))); result = accountApiAction.updatePassword(SecurityConfiguration.of(requestContent, "u", configuration)); assertTrue(result.isValid()); assertTrue(OpenBSDBCrypt.checkPassword(u.getHash(), "dddddd".toCharArray())); @@ -71,7 +72,7 @@ private ObjectNode requestContent() { private InternalUserV7 createExistingUser() { final var u = new InternalUserV7(); - u.setHash(Utils.hash("sssss".toCharArray())); + u.setHash(passwordService.hash(CharBuffer.wrap("sssss".toCharArray()))); Mockito.when(configuration.getCEntry("u")).thenReturn(u); return u; } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/InternalUsersApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/InternalUsersApiActionValidationTest.java index 2af598f5d5..edc022cbf3 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/InternalUsersApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/InternalUsersApiActionValidationTest.java @@ -12,12 +12,12 @@ package org.opensearch.security.dlic.rest.api; import java.io.IOException; +import java.nio.CharBuffer; import java.util.List; import java.util.Map; import org.junit.Before; import org.junit.Test; -import org.bouncycastle.crypto.generators.OpenBSDBCrypt; import org.opensearch.core.rest.RestStatus; import org.opensearch.rest.RestRequest; @@ -98,7 +98,7 @@ public void replacePasswordWithHash() throws Exception { assertEquals(RestStatus.OK, result.status()); assertFalse(securityConfiguration.requestContent().has("password")); assertTrue(securityConfiguration.requestContent().has("hash")); - assertTrue(OpenBSDBCrypt.checkPassword(securityConfiguration.requestContent().get("hash").asText(), "aaaaaa".toCharArray())); + assertTrue(passwordService.check(CharBuffer.wrap("aaaaaa".toCharArray()), securityConfiguration.requestContent().get("hash").asText())); } @Test