From 20c524ad994a9cc7d8757999f92f6d2fec6cb8ca Mon Sep 17 00:00:00 2001 From: Dan Cristian Cecoi Date: Mon, 10 Jun 2024 15:01:56 +0100 Subject: [PATCH] Replace BouncyCastle's OpenBSDBCrypt use with password4j for password hashing and verification (#4381) Signed-off-by: Dan Cecoi Co-authored-by: Dan Cecoi --- build.gradle | 2 +- .../api/AbstractApiIntegrationTest.java | 4 + .../api/AccountRestApiIntegrationTest.java | 6 +- .../test/framework/TestSecurityConfig.java | 16 ++- .../security/OpenSearchSecurityPlugin.java | 15 ++- .../InternalAuthenticationBackend.java | 10 +- .../dlic/rest/api/AccountApiAction.java | 14 ++- .../dlic/rest/api/InternalUsersApiAction.java | 10 +- .../dlic/rest/api/SecurityRestApiActions.java | 8 +- .../security/dlic/rest/support/Utils.java | 18 ---- .../security/hasher/BCryptPasswordHasher.java | 84 +++++++++++++++ .../security/hasher/PasswordHasher.java | 19 ++++ .../securityconf/DynamicConfigFactory.java | 7 +- .../org/opensearch/security/tools/Hasher.java | 15 +-- .../SecuritySettingsConfigurer.java | 11 +- .../opensearch/security/user/UserService.java | 19 ++-- .../security/UserServiceUnitTests.java | 5 +- .../org/opensearch/security/UtilTests.java | 9 +- .../auth/InternalAuthBackendTests.java | 3 +- .../api/AbstractApiActionValidationTest.java | 6 ++ ...AccountApiActionConfigValidationsTest.java | 35 +++--- .../InternalUsersApiActionValidationTest.java | 6 +- .../hasher/BCryptPasswordHasherTests.java | 101 ++++++++++++++++++ 23 files changed, 325 insertions(+), 98 deletions(-) create mode 100644 src/main/java/org/opensearch/security/hasher/BCryptPasswordHasher.java create mode 100644 src/main/java/org/opensearch/security/hasher/PasswordHasher.java create mode 100644 src/test/java/org/opensearch/security/hasher/BCryptPasswordHasherTests.java diff --git a/build.gradle b/build.gradle index d5976748d8..692f23cf77 100644 --- a/build.gradle +++ b/build.gradle @@ -582,7 +582,7 @@ dependencies { implementation 'org.ldaptive:ldaptive:1.2.3' implementation 'com.nimbusds:nimbus-jose-jwt:9.40' implementation 'com.rfksystems:blake2b:2.0.0' - + implementation 'com.password4j:password4j:1.8.2' //JWT implementation "io.jsonwebtoken:jjwt-api:${jjwt_version}" implementation "io.jsonwebtoken:jjwt-impl:${jjwt_version}" diff --git a/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java index d5a0e41a6d..adf10cd098 100644 --- a/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java @@ -35,6 +35,8 @@ import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.security.ConfigurationFiles; import org.opensearch.security.dlic.rest.api.Endpoint; +import org.opensearch.security.hasher.BCryptPasswordHasher; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.test.framework.TestSecurityConfig; import org.opensearch.test.framework.certificate.CertificateData; @@ -83,6 +85,8 @@ public abstract class AbstractApiIntegrationTest extends RandomizedTest { public static LocalCluster localCluster; + public static PasswordHasher passwordHasher = new BCryptPasswordHasher(); + @BeforeClass public static void startCluster() throws IOException { configurationFolder = ConfigurationFiles.createConfigurationDirectory(); diff --git a/src/integrationTest/java/org/opensearch/security/api/AccountRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AccountRestApiIntegrationTest.java index 7fa298c1e4..65bec9f788 100644 --- a/src/integrationTest/java/org/opensearch/security/api/AccountRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/AccountRestApiIntegrationTest.java @@ -20,7 +20,6 @@ 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 { @@ -110,7 +109,10 @@ private void verifyPasswordCanBeChanged() throws Exception { TEST_USER, TEST_USER_PASSWORD, client -> ok( - () -> client.putJson(accountPath(), changePasswordWithHashPayload(TEST_USER_PASSWORD, hash(newPassword.toCharArray()))) + () -> client.putJson( + accountPath(), + changePasswordWithHashPayload(TEST_USER_PASSWORD, passwordHasher.hash(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 e4e7f8f4de..0c332c54d2 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java +++ b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java @@ -31,7 +31,6 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.nio.ByteBuffer; -import java.security.SecureRandom; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -46,7 +45,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 +55,8 @@ import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.security.hasher.BCryptPasswordHasher; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.test.framework.cluster.OpenSearchClientProvider.UserCredentialsHolder; @@ -76,9 +76,10 @@ */ public class TestSecurityConfig { - private static final Logger log = LogManager.getLogger(TestSecurityConfig.class); + public static final String REST_ADMIN_REST_API_ACCESS = "rest_admin__rest_api_access"; - public final static String REST_ADMIN_REST_API_ACCESS = "rest_admin__rest_api_access"; + private static final Logger log = LogManager.getLogger(TestSecurityConfig.class); + private static final PasswordHasher passwordHasher = new BCryptPasswordHasher(); private Config config = new Config(); private Map internalUsers = new LinkedHashMap<>(); @@ -967,12 +968,7 @@ public void updateInternalUsersConfiguration(Client client, List users) { } 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; + return passwordHasher.hash(clearTextPassword); } 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 36ef7709e8..c3e00da109 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -160,6 +160,8 @@ import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.filter.SecurityFilter; import org.opensearch.security.filter.SecurityRestFilter; +import org.opensearch.security.hasher.BCryptPasswordHasher; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.http.NonSslHttpServerTransport; import org.opensearch.security.http.XFFResolver; import org.opensearch.security.identity.SecurityTokenManager; @@ -262,6 +264,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin private volatile DlsFlsRequestValve dlsFlsValve = null; private volatile Salt salt; private volatile OpensearchDynamicSetting transportPassiveAuthSetting; + private volatile PasswordHasher passwordHasher; public static boolean isActionTraceEnabled() { @@ -648,7 +651,8 @@ public List getRestHandlers( Objects.requireNonNull(auditLog), sks, Objects.requireNonNull(userService), - sslCertReloadEnabled + sslCertReloadEnabled, + passwordHasher ) ); log.debug("Added {} rest handler(s)", handlers.size()); @@ -1104,7 +1108,9 @@ public Collection createComponents( cr = ConfigurationRepository.create(settings, this.configPath, threadPool, localClient, clusterService, auditLog); - userService = new UserService(cs, cr, settings, localClient); + this.passwordHasher = new BCryptPasswordHasher(); + + userService = new UserService(cs, cr, passwordHasher, settings, localClient); final XFFResolver xffResolver = new XFFResolver(threadPool); backendRegistry = new BackendRegistry(settings, adminDns, xffResolver, auditLog, threadPool); @@ -1150,7 +1156,7 @@ public Collection createComponents( configPath, compatConfig ); - dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih); + dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih, passwordHasher); dcf.registerDCFListener(backendRegistry); dcf.registerDCFListener(compatConfig); dcf.registerDCFListener(irr); @@ -1200,6 +1206,8 @@ public Collection createComponents( components.add(si); components.add(dcf); components.add(userService); + components.add(passwordHasher); + if (!ExternalSecurityKeyStore.hasExternalSslContext(settings)) { components.add(sks); } @@ -1209,7 +1217,6 @@ public Collection createComponents( clusterService.addListener(cr); } return components; - } @Override diff --git a/src/main/java/org/opensearch/security/auth/internal/InternalAuthenticationBackend.java b/src/main/java/org/opensearch/security/auth/internal/InternalAuthenticationBackend.java index d3dba7409e..be09638d9d 100644 --- a/src/main/java/org/opensearch/security/auth/internal/InternalAuthenticationBackend.java +++ b/src/main/java/org/opensearch/security/auth/internal/InternalAuthenticationBackend.java @@ -35,11 +35,10 @@ import java.util.Map; import java.util.Map.Entry; -import org.bouncycastle.crypto.generators.OpenBSDBCrypt; - import org.opensearch.OpenSearchSecurityException; import org.opensearch.security.auth.AuthenticationBackend; import org.opensearch.security.auth.AuthorizationBackend; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.securityconf.InternalUsersModel; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; @@ -48,8 +47,13 @@ public class InternalAuthenticationBackend implements AuthenticationBackend, AuthorizationBackend { + private final PasswordHasher passwordHasher; private InternalUsersModel internalUsersModel; + public InternalAuthenticationBackend(PasswordHasher passwordHasher) { + this.passwordHasher = passwordHasher; + } + @Override public boolean exists(User user) { @@ -91,7 +95,7 @@ public boolean exists(User user) { * @return Whether the hash matches the provided password */ public boolean passwordMatchesHash(String hash, char[] array) { - return OpenBSDBCrypt.checkPassword(hash, array); + return passwordHasher.check(array, hash); } @Override 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..199f6b088a 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 @@ -19,7 +19,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; @@ -32,6 +31,7 @@ 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.hasher.PasswordHasher; import org.opensearch.security.securityconf.Hashed; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; @@ -44,13 +44,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 PasswordHasher passwordHasher; + private static final List routes = addRoutesPrefix( ImmutableList.of(new Route(Method.GET, "/account"), new Route(Method.PUT, "/account")) ); @@ -58,10 +60,12 @@ public class AccountApiAction extends AbstractApiAction { public AccountApiAction( final ClusterService clusterService, final ThreadPool threadPool, - final SecurityApiDependencies securityApiDependencies + final SecurityApiDependencies securityApiDependencies, + final PasswordHasher passwordHasher ) { super(Endpoint.ACCOUNT, clusterService, threadPool, securityApiDependencies); this.requestHandlersBuilder.configureRequestHandlers(this::accountApiRequestHandlers); + this.passwordHasher = passwordHasher; } @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 || !passwordHasher.check(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 = passwordHasher.hash(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..3155bdb740 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 @@ -31,6 +31,7 @@ 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.hasher.PasswordHasher; import org.opensearch.security.securityconf.Hashed; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; @@ -47,10 +48,11 @@ 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 { + private final PasswordHasher passwordHasher; + @Override protected void consumeParameters(final RestRequest request) { request.param("name"); @@ -87,11 +89,13 @@ public InternalUsersApiAction( final ClusterService clusterService, final ThreadPool threadPool, final UserService userService, - final SecurityApiDependencies securityApiDependencies + final SecurityApiDependencies securityApiDependencies, + final PasswordHasher passwordHasher ) { super(Endpoint.INTERNALUSERS, clusterService, threadPool, securityApiDependencies); this.userService = userService; this.requestHandlersBuilder.configureRequestHandlers(this::internalUsersApiRequestHandlers); + this.passwordHasher = passwordHasher; } @Override @@ -268,7 +272,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", passwordHasher.hash(plainTextPassword.toCharArray())); } return ValidationResult.success(securityConfiguration); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java index 4344c85b09..8ccf494d3d 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java @@ -23,6 +23,7 @@ import org.opensearch.security.auditlog.AuditLog; import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.configuration.ConfigurationRepository; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.privileges.PrivilegesEvaluator; import org.opensearch.security.ssl.SecurityKeyStore; import org.opensearch.security.ssl.transport.PrincipalExtractor; @@ -47,7 +48,8 @@ public static Collection getHandler( final AuditLog auditLog, final SecurityKeyStore securityKeyStore, final UserService userService, - final boolean certificatesReloadEnabled + final boolean certificatesReloadEnabled, + final PasswordHasher passwordHasher ) { final var securityApiDependencies = new SecurityApiDependencies( adminDns, @@ -64,7 +66,7 @@ public static Collection getHandler( settings ); return List.of( - new InternalUsersApiAction(clusterService, threadPool, userService, securityApiDependencies), + new InternalUsersApiAction(clusterService, threadPool, userService, securityApiDependencies, passwordHasher), new RolesMappingApiAction(clusterService, threadPool, securityApiDependencies), new RolesApiAction(clusterService, threadPool, securityApiDependencies), new ActionGroupsApiAction(clusterService, threadPool, securityApiDependencies), @@ -88,7 +90,7 @@ public static Collection getHandler( new TenantsApiAction(clusterService, threadPool, securityApiDependencies), new MigrateApiAction(clusterService, threadPool, securityApiDependencies), new ValidateApiAction(clusterService, threadPool, securityApiDependencies), - new AccountApiAction(clusterService, threadPool, securityApiDependencies), + new AccountApiAction(clusterService, threadPool, securityApiDependencies, passwordHasher), new NodesDnApiAction(clusterService, threadPool, securityApiDependencies), new WhitelistApiAction(clusterService, threadPool, securityApiDependencies), // FIXME change it as soon as WhitelistApiAction will be removed 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..d60e723d89 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 @@ -16,12 +16,10 @@ 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 +29,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; @@ -195,21 +192,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 diff --git a/src/main/java/org/opensearch/security/hasher/BCryptPasswordHasher.java b/src/main/java/org/opensearch/security/hasher/BCryptPasswordHasher.java new file mode 100644 index 0000000000..043c0392d9 --- /dev/null +++ b/src/main/java/org/opensearch/security/hasher/BCryptPasswordHasher.java @@ -0,0 +1,84 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.hasher; + +import java.nio.CharBuffer; +import java.security.AccessController; +import java.security.PrivilegedAction; +import java.util.Arrays; + +import org.opensearch.OpenSearchSecurityException; +import org.opensearch.SpecialPermission; + +import com.password4j.BcryptFunction; +import com.password4j.HashingFunction; +import com.password4j.Password; +import com.password4j.types.Bcrypt; + +import static org.opensearch.core.common.Strings.isNullOrEmpty; + +public class BCryptPasswordHasher implements PasswordHasher { + + private static final HashingFunction DEFAULT_BCRYPT_FUNCTION = BcryptFunction.getInstance(Bcrypt.Y, 12); + + @Override + public String hash(char[] password) { + if (password == null || password.length == 0) { + throw new OpenSearchSecurityException("Password cannot be empty or null"); + } + CharBuffer passwordBuffer = CharBuffer.wrap(password); + try { + SecurityManager securityManager = System.getSecurityManager(); + if (securityManager != null) { + securityManager.checkPermission(new SpecialPermission()); + } + return AccessController.doPrivileged( + (PrivilegedAction) () -> Password.hash(passwordBuffer).with(DEFAULT_BCRYPT_FUNCTION).getResult() + ); + } finally { + cleanup(passwordBuffer); + } + } + + @Override + public boolean check(char[] password, String hash) { + if (password == null || password.length == 0) { + throw new OpenSearchSecurityException("Password cannot be empty or null"); + } + if (isNullOrEmpty(hash)) { + throw new OpenSearchSecurityException("Hash cannot be empty or null"); + } + CharBuffer passwordBuffer = CharBuffer.wrap(password); + try { + SecurityManager securityManager = System.getSecurityManager(); + if (securityManager != null) { + securityManager.checkPermission(new SpecialPermission()); + } + return AccessController.doPrivileged( + (PrivilegedAction) () -> Password.check(passwordBuffer, hash).with(getBCryptFunctionFromHash(hash)) + ); + } finally { + cleanup(passwordBuffer); + } + } + + private void cleanup(CharBuffer password) { + password.clear(); + char[] passwordOverwrite = new char[password.capacity()]; + Arrays.fill(passwordOverwrite, '\0'); + password.put(passwordOverwrite); + } + + private HashingFunction getBCryptFunctionFromHash(String hash) { + return BcryptFunction.getInstanceFromHash(hash); + } +} diff --git a/src/main/java/org/opensearch/security/hasher/PasswordHasher.java b/src/main/java/org/opensearch/security/hasher/PasswordHasher.java new file mode 100644 index 0000000000..deaab7e073 --- /dev/null +++ b/src/main/java/org/opensearch/security/hasher/PasswordHasher.java @@ -0,0 +1,19 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.hasher; + +public interface PasswordHasher { + + String hash(char[] password); + + boolean check(char[] password, String hashedPassword); +} diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java index f046b4c114..56415ec1bf 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java @@ -50,6 +50,7 @@ import org.opensearch.security.configuration.ConfigurationChangeListener; import org.opensearch.security.configuration.ConfigurationRepository; import org.opensearch.security.configuration.StaticResourceException; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.securityconf.impl.AllowlistingSettings; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.NodesDn; @@ -128,7 +129,7 @@ public final static SecurityDynamicConfiguration addStatics(SecurityDynamicCo private final EventBus eventBus = EVENT_BUS_BUILDER.logger(new JavaLogger(DynamicConfigFactory.class.getCanonicalName())).build(); private final Settings opensearchSettings; private final Path configPath; - private final InternalAuthenticationBackend iab = new InternalAuthenticationBackend(); + private final InternalAuthenticationBackend iab; private final ClusterInfoHolder cih; SecurityDynamicConfiguration config; @@ -139,13 +140,15 @@ public DynamicConfigFactory( final Path configPath, Client client, ThreadPool threadPool, - ClusterInfoHolder cih + ClusterInfoHolder cih, + PasswordHasher passwordHasher ) { super(); this.cr = cr; this.opensearchSettings = opensearchSettings; this.configPath = configPath; this.cih = cih; + this.iab = new InternalAuthenticationBackend(passwordHasher); if (opensearchSettings.getAsBoolean(ConfigConstants.SECURITY_UNSUPPORTED_LOAD_STATIC_RESOURCES, true)) { try { diff --git a/src/main/java/org/opensearch/security/tools/Hasher.java b/src/main/java/org/opensearch/security/tools/Hasher.java index f19d958523..76a4fec5c6 100644 --- a/src/main/java/org/opensearch/security/tools/Hasher.java +++ b/src/main/java/org/opensearch/security/tools/Hasher.java @@ -27,9 +27,6 @@ package org.opensearch.security.tools; import java.io.Console; -import java.security.SecureRandom; -import java.util.Arrays; -import java.util.Objects; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; @@ -37,7 +34,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.security.hasher.BCryptPasswordHasher; +import org.opensearch.security.hasher.PasswordHasher; public class Hasher { @@ -82,11 +81,7 @@ 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; + PasswordHasher passwordHasher = new BCryptPasswordHasher(); + return passwordHasher.hash(clearTextPassword); } } diff --git a/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java b/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java index 72f0247e53..af509def1a 100644 --- a/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java +++ b/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java @@ -24,14 +24,14 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; -import org.bouncycastle.crypto.generators.OpenBSDBCrypt; import org.opensearch.common.settings.Settings; import org.opensearch.core.common.Strings; import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.dlic.rest.validation.RequestContentValidator; +import org.opensearch.security.hasher.BCryptPasswordHasher; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.support.ConfigConstants; -import org.opensearch.security.tools.Hasher; import org.yaml.snakeyaml.DumperOptions; import org.yaml.snakeyaml.Yaml; @@ -83,10 +83,12 @@ public class SecuritySettingsConfigurer { static String ADMIN_USERNAME = "admin"; private final Installer installer; + private final PasswordHasher passwordHasher; static final String DEFAULT_ADMIN_PASSWORD = "admin"; public SecuritySettingsConfigurer(Installer installer) { this.installer = installer; + this.passwordHasher = new BCryptPasswordHasher(); } /** @@ -199,8 +201,9 @@ void updateAdminPassword() throws IOException { */ private boolean isAdminPasswordSetToAdmin(String internalUsersFile) throws IOException { JsonNode internalUsers = YAML_MAPPER.readTree(new FileInputStream(internalUsersFile)); + PasswordHasher passwordHasher = new BCryptPasswordHasher(); return internalUsers.has("admin") - && OpenBSDBCrypt.checkPassword(internalUsers.get("admin").get("hash").asText(), DEFAULT_ADMIN_PASSWORD.toCharArray()); + && passwordHasher.check(DEFAULT_ADMIN_PASSWORD.toCharArray(), internalUsers.get("admin").get("hash").asText()); } /** @@ -210,7 +213,7 @@ private boolean isAdminPasswordSetToAdmin(String internalUsersFile) throws IOExc * @throws IOException while reading, writing to files */ void writePasswordToInternalUsersFile(String adminPassword, String internalUsersFile) throws IOException { - String hashedAdminPassword = Hasher.hash(adminPassword.toCharArray()); + String hashedAdminPassword = passwordHasher.hash(adminPassword.toCharArray()); if (hashedAdminPassword.isEmpty()) { System.out.println("Failure while hashing the admin password, see console for details."); diff --git a/src/main/java/org/opensearch/security/user/UserService.java b/src/main/java/org/opensearch/security/user/UserService.java index 937a5331a8..2fdd74df65 100644 --- a/src/main/java/org/opensearch/security/user/UserService.java +++ b/src/main/java/org/opensearch/security/user/UserService.java @@ -45,6 +45,7 @@ import org.opensearch.identity.tokens.BasicAuthToken; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.configuration.ConfigurationRepository; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.securityconf.DynamicConfigFactory; import org.opensearch.security.securityconf.Hashed; import org.opensearch.security.securityconf.impl.CType; @@ -57,8 +58,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. */ @@ -67,6 +66,7 @@ public class UserService { protected final Logger log = LogManager.getLogger(this.getClass()); ClusterService clusterService; private final ConfigurationRepository configurationRepository; + private final PasswordHasher passwordHasher; String securityIndex; Client client; @@ -94,9 +94,16 @@ private static CType getUserConfigName() { ); @Inject - public UserService(ClusterService clusterService, ConfigurationRepository configurationRepository, Settings settings, Client client) { + public UserService( + ClusterService clusterService, + ConfigurationRepository configurationRepository, + PasswordHasher passwordHasher, + Settings settings, + Client client + ) { this.clusterService = clusterService; this.configurationRepository = configurationRepository; + this.passwordHasher = passwordHasher; this.securityIndex = settings.get( ConfigConstants.SECURITY_CONFIG_INDEX_NAME, ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX @@ -142,7 +149,7 @@ public SecurityDynamicConfiguration createOrUpdateAccount(ObjectNode contentA // service account verifyServiceAccount(securityJsonNode, accountName); String password = generatePassword(); - contentAsNode.put("hash", hash(password.toCharArray())); + contentAsNode.put("hash", passwordHasher.hash(password.toCharArray())); contentAsNode.put("service", "true"); } else { contentAsNode.put("service", "false"); @@ -162,7 +169,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", passwordHasher.hash(plainTextPassword.toCharArray())); } else if (origHash != null && origHash.length() > 0) { contentAsNode.remove("password"); } else if (plainTextPassword != null && plainTextPassword.isEmpty() && origHash == null) { @@ -275,7 +282,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", passwordHasher.hash(plainTextPassword.toCharArray())); contentAsNode.put("enabled", "true"); contentAsNode.put("service", "true"); diff --git a/src/test/java/org/opensearch/security/UserServiceUnitTests.java b/src/test/java/org/opensearch/security/UserServiceUnitTests.java index 48c37748fc..ccd3c9848e 100644 --- a/src/test/java/org/opensearch/security/UserServiceUnitTests.java +++ b/src/test/java/org/opensearch/security/UserServiceUnitTests.java @@ -26,6 +26,8 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.security.configuration.ConfigurationRepository; +import org.opensearch.security.hasher.BCryptPasswordHasher; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.user.UserFilterType; @@ -52,7 +54,8 @@ public class UserServiceUnitTests { public void setup() throws Exception { String usersYmlFile = "./internal_users.yml"; Settings.Builder builder = Settings.builder(); - userService = new UserService(clusterService, configurationRepository, builder.build(), client); + PasswordHasher passwordHasher = new BCryptPasswordHasher(); + userService = new UserService(clusterService, configurationRepository, passwordHasher, builder.build(), client); config = readConfigFromYml(usersYmlFile, CType.INTERNALUSERS); } diff --git a/src/test/java/org/opensearch/security/UtilTests.java b/src/test/java/org/opensearch/security/UtilTests.java index 402d5dc92f..e545cde86f 100644 --- a/src/test/java/org/opensearch/security/UtilTests.java +++ b/src/test/java/org/opensearch/security/UtilTests.java @@ -29,9 +29,10 @@ import java.util.Map; import org.junit.Test; -import org.bouncycastle.crypto.generators.OpenBSDBCrypt; import org.opensearch.common.settings.Settings; +import org.opensearch.security.hasher.BCryptPasswordHasher; +import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.SecurityUtils; import org.opensearch.security.support.WildcardMatcher; @@ -50,6 +51,8 @@ static private WildcardMatcher iwc(String pattern) { return WildcardMatcher.from(pattern, false); } + static private final PasswordHasher passwordHasher = new BCryptPasswordHasher(); + @Test public void testWildcardMatcherClasses() { assertFalse(wc("a*?").test("a")); @@ -113,7 +116,7 @@ public void testEnvReplace() { assertEquals("abv${env.MYENV}xyz", SecurityUtils.replaceEnvVars("abv${env.MYENV}xyz", settings)); assertEquals("abv${envbc.MYENV}xyz", SecurityUtils.replaceEnvVars("abv${envbc.MYENV}xyz", settings)); assertEquals("abvtTtxyz", SecurityUtils.replaceEnvVars("abv${env.MYENV:-tTt}xyz", settings)); - assertTrue(OpenBSDBCrypt.checkPassword(SecurityUtils.replaceEnvVars("${envbc.MYENV:-tTt}", settings), "tTt".toCharArray())); + assertTrue(passwordHasher.check("tTt".toCharArray(), SecurityUtils.replaceEnvVars("${envbc.MYENV:-tTt}", settings))); assertEquals("abvtTtxyzxxx", SecurityUtils.replaceEnvVars("abv${env.MYENV:-tTt}xyz${env.MYENV:-xxx}", settings)); assertTrue(SecurityUtils.replaceEnvVars("abv${env.MYENV:-tTt}xyz${envbc.MYENV:-xxx}", settings).startsWith("abvtTtxyz$2y$")); assertEquals("abv${env.MYENV:tTt}xyz", SecurityUtils.replaceEnvVars("abv${env.MYENV:tTt}xyz", settings)); @@ -138,7 +141,7 @@ public void testEnvReplace() { SecurityUtils.replaceEnvVars("abv${env." + k + "}xyzabv${env." + k + "}xyz", settings) ); assertEquals("abv" + val + "xyz", SecurityUtils.replaceEnvVars("abv${env." + k + ":-k182765ggh}xyz", settings)); - assertTrue(OpenBSDBCrypt.checkPassword(SecurityUtils.replaceEnvVars("${envbc." + k + "}", settings), val.toCharArray())); + assertTrue(passwordHasher.check(val.toCharArray(), SecurityUtils.replaceEnvVars("${envbc." + k + "}", settings))); checked = true; } diff --git a/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java b/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java index b9503e79f1..3832f866cb 100644 --- a/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java +++ b/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java @@ -22,6 +22,7 @@ import org.opensearch.OpenSearchSecurityException; import org.opensearch.security.auth.internal.InternalAuthenticationBackend; +import org.opensearch.security.hasher.BCryptPasswordHasher; import org.opensearch.security.securityconf.InternalUsersModel; import org.opensearch.security.user.AuthCredentials; @@ -42,7 +43,7 @@ public class InternalAuthBackendTests { @Before public void internalAuthBackendTestsSetup() { - internalAuthenticationBackend = spy(new InternalAuthenticationBackend()); + internalAuthenticationBackend = spy(new InternalAuthenticationBackend(new BCryptPasswordHasher())); internalUsersModel = mock(InternalUsersModel.class); internalAuthenticationBackend.onInternalUsersModelChanged(internalUsersModel); } 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..e6c4bb17d0 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.hasher.BCryptPasswordHasher; +import org.opensearch.security.hasher.PasswordHasher; 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; + PasswordHasher passwordHasher; + @Before public void setup() { securityApiDependencies = new SecurityApiDependencies( @@ -73,6 +77,8 @@ public void setup() { null, Settings.EMPTY ); + + passwordHasher = new BCryptPasswordHasher(); } 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..58a37c0bff 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 @@ -11,38 +11,35 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import org.junit.Test; -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 static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; public class AccountApiActionConfigValidationsTest extends AbstractApiActionValidationTest { @Test public void verifyValidCurrentPassword() { - final var accountApiAction = new AccountApiAction(clusterService, threadPool, securityApiDependencies); + final var accountApiAction = new AccountApiAction(clusterService, threadPool, securityApiDependencies, passwordHasher); final var u = createExistingUser(); var result = accountApiAction.validCurrentPassword(SecurityConfiguration.of(requestContent(), "u", configuration)); - assertFalse(result.isValid()); - assertEquals(RestStatus.BAD_REQUEST, result.status()); + assertThat(result.isValid(), is(false)); + assertThat(RestStatus.BAD_REQUEST, is(result.status())); - u.setHash(Utils.hash("aaaa".toCharArray())); + u.setHash(passwordHasher.hash("aaaa".toCharArray())); result = accountApiAction.validCurrentPassword(SecurityConfiguration.of(requestContent(), "u", configuration)); - assertTrue(result.isValid()); + assertThat(result.isValid(), is(true)); } @Test public void updatePassword() { - final var accountApiAction = new AccountApiAction(clusterService, threadPool, securityApiDependencies); + final var accountApiAction = new AccountApiAction(clusterService, threadPool, securityApiDependencies, passwordHasher); final var requestContent = requestContent(); requestContent.remove("password"); @@ -50,19 +47,19 @@ public void updatePassword() { u.setHash(null); var result = accountApiAction.updatePassword(SecurityConfiguration.of(requestContent, "u", configuration)); - assertFalse(result.isValid()); - assertEquals(RestStatus.BAD_REQUEST, result.status()); + assertThat(result.isValid(), is(false)); + assertThat(RestStatus.BAD_REQUEST, is(result.status())); requestContent.put("password", "cccccc"); result = accountApiAction.updatePassword(SecurityConfiguration.of(requestContent, "u", configuration)); - assertTrue(result.isValid()); - assertTrue(OpenBSDBCrypt.checkPassword(u.getHash(), "cccccc".toCharArray())); + assertThat(result.isValid(), is(true)); + assertThat(passwordHasher.check("cccccc".toCharArray(), u.getHash()), is(true)); requestContent.remove("password"); - requestContent.put("hash", Utils.hash("dddddd".toCharArray())); + requestContent.put("hash", passwordHasher.hash("dddddd".toCharArray())); result = accountApiAction.updatePassword(SecurityConfiguration.of(requestContent, "u", configuration)); - assertTrue(result.isValid()); - assertTrue(OpenBSDBCrypt.checkPassword(u.getHash(), "dddddd".toCharArray())); + assertThat(result.isValid(), is(true)); + assertThat(passwordHasher.check("dddddd".toCharArray(), u.getHash()), is(true)); } private ObjectNode requestContent() { @@ -71,7 +68,7 @@ private ObjectNode requestContent() { private InternalUserV7 createExistingUser() { final var u = new InternalUserV7(); - u.setHash(Utils.hash("sssss".toCharArray())); + u.setHash(passwordHasher.hash("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..8e6b5a0bb2 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 @@ -17,12 +17,12 @@ 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; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.dlic.rest.validation.ValidationResult; +import org.opensearch.security.hasher.BCryptPasswordHasher; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.securityconf.impl.v7.InternalUserV7; @@ -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(passwordHasher.check("aaaaaa".toCharArray(), securityConfiguration.requestContent().get("hash").asText())); } @Test @@ -193,7 +193,7 @@ public void validateSecurityRolesWithImmutableRolesMappingConfig() throws Except } private InternalUsersApiAction createInternalUsersApiAction() { - return new InternalUsersApiAction(clusterService, threadPool, userService, securityApiDependencies); + return new InternalUsersApiAction(clusterService, threadPool, userService, securityApiDependencies, new BCryptPasswordHasher()); } } diff --git a/src/test/java/org/opensearch/security/hasher/BCryptPasswordHasherTests.java b/src/test/java/org/opensearch/security/hasher/BCryptPasswordHasherTests.java new file mode 100644 index 0000000000..9fad74488f --- /dev/null +++ b/src/test/java/org/opensearch/security/hasher/BCryptPasswordHasherTests.java @@ -0,0 +1,101 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.hasher; + +import java.nio.CharBuffer; + +import org.junit.Test; + +import org.opensearch.OpenSearchSecurityException; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertThrows; + +public class BCryptPasswordHasherTests { + + private final PasswordHasher passwordHasher = new BCryptPasswordHasher(); + + private final String password = "testPassword"; + private final String wrongPassword = "wrongTestPassword"; + + @Test + public void shouldMatchHashToCorrectPassword() { + String hashedPassword = passwordHasher.hash(password.toCharArray()); + assertThat(passwordHasher.check(password.toCharArray(), hashedPassword), is(true)); + } + + @Test + public void shouldNotMatchHashToWrongPassword() { + String hashedPassword = passwordHasher.hash(password.toCharArray()); + assertThat(passwordHasher.check(wrongPassword.toCharArray(), hashedPassword), is(false)); + + } + + /** + * Ensures that the hashes that were previously created by OpenBSDBCrypt are still valid + */ + @Test + public void shouldBeBackwardsCompatible() { + String legacyHash = "$2y$12$gdh2ecVBQmwpmcAeyReicuNtXyR6GMWSfXHxtcBBqFeFz2VQ8kDZe"; + assertThat(passwordHasher.check(password.toCharArray(), legacyHash), is(true)); + assertThat(passwordHasher.check(wrongPassword.toCharArray(), legacyHash), is(false)); + } + + @Test + public void shouldGenerateDifferentHashesForTheSamePassword() { + String hash1 = passwordHasher.hash(password.toCharArray()); + String hash2 = passwordHasher.hash(password.toCharArray()); + assertThat(hash1, is(not(hash2))); + } + + @Test + public void shouldHandleNullPasswordWhenHashing() { + char[] nullPassword = null; + assertThrows(OpenSearchSecurityException.class, () -> { passwordHasher.hash(nullPassword); }); + } + + @Test + public void shouldHandleNullPasswordWhenChecking() { + char[] nullPassword = null; + assertThrows(OpenSearchSecurityException.class, () -> { passwordHasher.check(nullPassword, "some hash"); }); + } + + @Test + public void shouldHandleEmptyHashWhenChecking() { + String emptyHash = ""; + assertThrows(OpenSearchSecurityException.class, () -> { passwordHasher.check(password.toCharArray(), emptyHash); }); + } + + @Test + public void shouldHandleNullHashWhenChecking() { + String nullHash = null; + assertThrows(OpenSearchSecurityException.class, () -> { passwordHasher.check(password.toCharArray(), nullHash); }); + } + + @Test + public void shouldCleanupPasswordCharArray() { + char[] password = new char[] { 'p', 'a', 's', 's', 'w', 'o', 'r', 'd' }; + passwordHasher.hash(password); + assertThat("\0\0\0\0\0\0\0\0", is(new String(password))); + } + + @Test + public void shouldCleanupPasswordCharBuffer() { + char[] password = new char[] { 'p', 'a', 's', 's', 'w', 'o', 'r', 'd' }; + CharBuffer passwordBuffer = CharBuffer.wrap(password); + passwordHasher.hash(password); + assertThat("\0\0\0\0\0\0\0\0", is(new String(password))); + assertThat("\0\0\0\0\0\0\0\0", is(passwordBuffer.toString())); + } +}