From 6f4b59d77e9790e1519c2df1a8319d050e135f99 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 11 Jul 2024 13:15:16 -0400 Subject: [PATCH] [Backport 2.x] Add support for PBKDF2 for password hashing & add support for configuring BCrypt and PBKDF2 (#4551) Signed-off-by: Dan Cecoi Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] Co-authored-by: Dan Cecoi --- .../api/AbstractApiIntegrationTest.java | 10 +- .../InternalUsersRestApiIntegrationTest.java | 4 - .../hash/BCryptCustomConfigHashingTests.java | 92 +++++++++ .../hash/BCryptDefaultConfigHashingTests.java | 70 +++++++ .../security/hash/HashingTests.java | 81 ++++++++ .../hash/PBKDF2CustomConfigHashingTests.java | 97 +++++++++ .../hash/PBKDF2DefaultConfigHashingTests.java | 85 ++++++++ .../test/framework/TestSecurityConfig.java | 27 ++- .../security/OpenSearchSecurityPlugin.java | 58 +++++- .../security/auditlog/impl/AuditMessage.java | 8 +- .../hasher/AbstractPasswordHasher.java | 79 ++++++++ .../security/hasher/BCryptPasswordHasher.java | 31 +-- .../security/hasher/PBKDF2PasswordHasher.java | 82 ++++++++ .../security/hasher/PasswordHasher.java | 17 ++ .../hasher/PasswordHasherFactory.java | 93 +++++++++ .../security/support/ConfigConstants.java | 19 ++ .../org/opensearch/security/tools/Hasher.java | 186 +++++++++++++++--- .../SecuritySettingsConfigurer.java | 16 +- .../security/UserServiceUnitTests.java | 9 +- .../org/opensearch/security/UtilTests.java | 6 +- .../auditlog/AbstractAuditlogiUnitTest.java | 6 +- .../RestApiComplianceAuditlogTest.java | 153 ++++++++++++-- .../auth/InternalAuthBackendTests.java | 12 +- .../api/AbstractApiActionValidationTest.java | 7 +- .../InternalUsersApiActionValidationTest.java | 14 +- .../hasher/AbstractPasswordHasherTests.java | 92 +++++++++ .../hasher/BCryptPasswordHasherTests.java | 93 +++------ .../hasher/PBKDF2PasswordHasherTests.java | 60 ++++++ .../hasher/PasswordHasherFactoryTests.java | 186 ++++++++++++++++++ .../security/tools/HasherTests.java | 145 ++++++++++++++ .../auditlog/internal_users_pbkdf2.yml | 46 +++++ 31 files changed, 1715 insertions(+), 169 deletions(-) create mode 100644 src/integrationTest/java/org/opensearch/security/hash/BCryptCustomConfigHashingTests.java create mode 100644 src/integrationTest/java/org/opensearch/security/hash/BCryptDefaultConfigHashingTests.java create mode 100644 src/integrationTest/java/org/opensearch/security/hash/HashingTests.java create mode 100644 src/integrationTest/java/org/opensearch/security/hash/PBKDF2CustomConfigHashingTests.java create mode 100644 src/integrationTest/java/org/opensearch/security/hash/PBKDF2DefaultConfigHashingTests.java create mode 100644 src/main/java/org/opensearch/security/hasher/AbstractPasswordHasher.java create mode 100644 src/main/java/org/opensearch/security/hasher/PBKDF2PasswordHasher.java create mode 100644 src/main/java/org/opensearch/security/hasher/PasswordHasherFactory.java create mode 100644 src/test/java/org/opensearch/security/hasher/AbstractPasswordHasherTests.java create mode 100644 src/test/java/org/opensearch/security/hasher/PBKDF2PasswordHasherTests.java create mode 100644 src/test/java/org/opensearch/security/hasher/PasswordHasherFactoryTests.java create mode 100644 src/test/java/org/opensearch/security/tools/HasherTests.java create mode 100644 src/test/resources/auditlog/internal_users_pbkdf2.yml diff --git a/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java index bdba22e103..40aabeb7b2 100644 --- a/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java @@ -30,14 +30,16 @@ import org.opensearch.common.CheckedConsumer; import org.opensearch.common.CheckedSupplier; +import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.xcontent.ToXContent; 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.hasher.PasswordHasherFactory; import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.support.ConfigConstants; import org.opensearch.test.framework.TestSecurityConfig; import org.opensearch.test.framework.certificate.CertificateData; import org.opensearch.test.framework.cluster.ClusterManager; @@ -78,6 +80,10 @@ public abstract class AbstractApiIntegrationTest extends RandomizedTest { public static final ToXContentObject EMPTY_BODY = (builder, params) -> builder.startObject().endObject(); + public static final PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher( + Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build() + ); + public static Path configurationFolder; public static ImmutableMap.Builder clusterSettings = ImmutableMap.builder(); @@ -86,8 +92,6 @@ 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/InternalUsersRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/InternalUsersRestApiIntegrationTest.java index 22a1216407..d024495125 100644 --- a/src/integrationTest/java/org/opensearch/security/api/InternalUsersRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/InternalUsersRestApiIntegrationTest.java @@ -32,8 +32,6 @@ import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.dlic.rest.api.Endpoint; -import org.opensearch.security.hasher.BCryptPasswordHasher; -import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.test.framework.TestSecurityConfig; import org.opensearch.test.framework.cluster.TestRestClient; import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; @@ -59,8 +57,6 @@ public class InternalUsersRestApiIntegrationTest extends AbstractConfigEntityApi private final static String SOME_ROLE = "some-role"; - private final PasswordHasher passwordHasher = new BCryptPasswordHasher(); - static { testSecurityConfig.withRestAdminUser(REST_API_ADMIN_INTERNAL_USERS_ONLY, restAdminPermission(Endpoint.INTERNALUSERS)) .user(new TestSecurityConfig.User(SERVICE_ACCOUNT_USER).attr("service", "true").attr("enabled", "true")) diff --git a/src/integrationTest/java/org/opensearch/security/hash/BCryptCustomConfigHashingTests.java b/src/integrationTest/java/org/opensearch/security/hash/BCryptCustomConfigHashingTests.java new file mode 100644 index 0000000000..95f05a7c0d --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/hash/BCryptCustomConfigHashingTests.java @@ -0,0 +1,92 @@ +/* + * 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.hash; + +import java.util.List; +import java.util.Map; + +import org.apache.http.HttpStatus; +import org.awaitility.Awaitility; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static org.hamcrest.Matchers.equalTo; +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; +import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; + +public class BCryptCustomConfigHashingTests extends HashingTests { + + private static LocalCluster cluster; + + private static String minor; + + private static int rounds; + + @BeforeClass + public static void startCluster() { + minor = randomFrom(List.of("A", "B", "Y")); + rounds = randomIntBetween(4, 10); + + TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS) + .hash(generateBCryptHash("secret", minor, rounds)); + cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) + .authc(AUTHC_HTTPBASIC_INTERNAL) + .users(ADMIN_USER) + .anonymousAuth(false) + .nodeSettings( + Map.of( + ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED, + List.of("user_" + ADMIN_USER.getName() + "__" + ALL_ACCESS.getName()), + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, + ConfigConstants.BCRYPT, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR, + minor, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS, + rounds + ) + ) + .build(); + cluster.before(); + + try (TestRestClient client = cluster.getRestClient(ADMIN_USER.getName(), "secret")) { + Awaitility.await() + .alias("Load default configuration") + .until(() -> client.securityHealth().getTextFromJsonBody("/status"), equalTo("UP")); + } + } + + @Test + public void shouldAuthenticateWithCorrectPassword() { + String hash = generateBCryptHash(PASSWORD, minor, rounds); + createUserWithHashedPassword(cluster, "user_2", hash); + testPasswordAuth(cluster, "user_2", PASSWORD, HttpStatus.SC_OK); + + createUserWithPlainTextPassword(cluster, "user_3", PASSWORD); + testPasswordAuth(cluster, "user_3", PASSWORD, HttpStatus.SC_OK); + } + + @Test + public void shouldNotAuthenticateWithIncorrectPassword() { + String hash = generateBCryptHash(PASSWORD, minor, rounds); + createUserWithHashedPassword(cluster, "user_4", hash); + testPasswordAuth(cluster, "user_4", "wrong_password", HttpStatus.SC_UNAUTHORIZED); + + createUserWithPlainTextPassword(cluster, "user_5", PASSWORD); + testPasswordAuth(cluster, "user_5", "wrong_password", HttpStatus.SC_UNAUTHORIZED); + } +} diff --git a/src/integrationTest/java/org/opensearch/security/hash/BCryptDefaultConfigHashingTests.java b/src/integrationTest/java/org/opensearch/security/hash/BCryptDefaultConfigHashingTests.java new file mode 100644 index 0000000000..86adc728ea --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/hash/BCryptDefaultConfigHashingTests.java @@ -0,0 +1,70 @@ +/* + * 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.hash; + +import java.util.List; +import java.util.Map; + +import org.apache.http.HttpStatus; +import org.junit.ClassRule; +import org.junit.Test; + +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; + +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; +import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; + +public class BCryptDefaultConfigHashingTests extends HashingTests { + + private static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS); + + @ClassRule + public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) + .authc(AUTHC_HTTPBASIC_INTERNAL) + .users(ADMIN_USER) + .anonymousAuth(false) + .nodeSettings( + Map.of(ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED, List.of("user_" + ADMIN_USER.getName() + "__" + ALL_ACCESS.getName())) + ) + .build(); + + @Test + public void shouldAuthenticateWithCorrectPassword() { + String hash = generateBCryptHash( + PASSWORD, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS_DEFAULT + ); + createUserWithHashedPassword(cluster, "user_2", hash); + testPasswordAuth(cluster, "user_2", PASSWORD, HttpStatus.SC_OK); + + createUserWithPlainTextPassword(cluster, "user_3", PASSWORD); + testPasswordAuth(cluster, "user_3", PASSWORD, HttpStatus.SC_OK); + } + + @Test + public void shouldNotAuthenticateWithIncorrectPassword() { + String hash = generateBCryptHash( + PASSWORD, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS_DEFAULT + ); + createUserWithHashedPassword(cluster, "user_4", hash); + testPasswordAuth(cluster, "user_4", "wrong_password", HttpStatus.SC_UNAUTHORIZED); + + createUserWithPlainTextPassword(cluster, "user_5", PASSWORD); + testPasswordAuth(cluster, "user_5", "wrong_password", HttpStatus.SC_UNAUTHORIZED); + } +} diff --git a/src/integrationTest/java/org/opensearch/security/hash/HashingTests.java b/src/integrationTest/java/org/opensearch/security/hash/HashingTests.java new file mode 100644 index 0000000000..4330af4ad1 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/hash/HashingTests.java @@ -0,0 +1,81 @@ +/* + * 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.hash; + +import java.nio.CharBuffer; + +import com.carrotsearch.randomizedtesting.RandomizedTest; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.apache.http.HttpStatus; +import org.junit.runner.RunWith; + +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +import com.password4j.BcryptFunction; +import com.password4j.CompressedPBKDF2Function; +import com.password4j.Password; +import com.password4j.types.Bcrypt; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; + +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class HashingTests extends RandomizedTest { + + private static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS); + + static final String PASSWORD = "top$ecret1234!"; + + public void createUserWithPlainTextPassword(LocalCluster cluster, String username, String password) { + try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) { + TestRestClient.HttpResponse httpResponse = client.putJson( + "_plugins/_security/api/internalusers/" + username, + String.format("{\"password\": \"%s\",\"opendistro_security_roles\": []}", password) + ); + assertThat(httpResponse.getStatusCode(), equalTo(HttpStatus.SC_CREATED)); + } + } + + public void createUserWithHashedPassword(LocalCluster cluster, String username, String hashedPassword) { + try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) { + TestRestClient.HttpResponse httpResponse = client.putJson( + "_plugins/_security/api/internalusers/" + username, + String.format("{\"hash\": \"%s\",\"opendistro_security_roles\": []}", hashedPassword) + ); + assertThat(httpResponse.getStatusCode(), equalTo(HttpStatus.SC_CREATED)); + } + } + + public void testPasswordAuth(LocalCluster cluster, String username, String password, int expectedStatusCode) { + try (TestRestClient client = cluster.getRestClient(username, password)) { + TestRestClient.HttpResponse response = client.getAuthInfo(); + response.assertStatusCode(expectedStatusCode); + } + } + + public static String generateBCryptHash(String password, String minor, int rounds) { + return Password.hash(CharBuffer.wrap(password.toCharArray())) + .with(BcryptFunction.getInstance(Bcrypt.valueOf(minor), rounds)) + .getResult(); + } + + public static String generatePBKDF2Hash(String password, String algorithm, int iterations, int length) { + return Password.hash(CharBuffer.wrap(password.toCharArray())) + .with(CompressedPBKDF2Function.getInstance(algorithm, iterations, length)) + .getResult(); + } + +} diff --git a/src/integrationTest/java/org/opensearch/security/hash/PBKDF2CustomConfigHashingTests.java b/src/integrationTest/java/org/opensearch/security/hash/PBKDF2CustomConfigHashingTests.java new file mode 100644 index 0000000000..9f2ceb2e93 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/hash/PBKDF2CustomConfigHashingTests.java @@ -0,0 +1,97 @@ +/* + * 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.hash; + +import java.util.List; +import java.util.Map; + +import org.apache.http.HttpStatus; +import org.awaitility.Awaitility; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static org.hamcrest.Matchers.equalTo; +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; +import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; + +public class PBKDF2CustomConfigHashingTests extends HashingTests { + + public static LocalCluster cluster; + + private static final String PASSWORD = "top$ecret1234!"; + + private static String function; + private static int iterations, length; + + @BeforeClass + public static void startCluster() { + + function = randomFrom(List.of("SHA224", "SHA256", "SHA384", "SHA512")); + iterations = randomFrom(List.of(32000, 64000, 128000, 256000)); + length = randomFrom(List.of(128, 256, 512)); + + TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS) + .hash(generatePBKDF2Hash("secret", function, iterations, length)); + cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) + .authc(AUTHC_HTTPBASIC_INTERNAL) + .users(ADMIN_USER) + .anonymousAuth(false) + .nodeSettings( + Map.of( + ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED, + List.of("user_" + ADMIN_USER.getName() + "__" + ALL_ACCESS.getName()), + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, + ConfigConstants.PBKDF2, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, + function, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS, + iterations, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH, + length + ) + ) + .build(); + cluster.before(); + + try (TestRestClient client = cluster.getRestClient(ADMIN_USER.getName(), "secret")) { + Awaitility.await() + .alias("Load default configuration") + .until(() -> client.securityHealth().getTextFromJsonBody("/status"), equalTo("UP")); + } + } + + @Test + public void shouldAuthenticateWithCorrectPassword() { + String hash = generatePBKDF2Hash(PASSWORD, function, iterations, length); + createUserWithHashedPassword(cluster, "user_1", hash); + testPasswordAuth(cluster, "user_1", PASSWORD, HttpStatus.SC_OK); + + createUserWithPlainTextPassword(cluster, "user_2", PASSWORD); + testPasswordAuth(cluster, "user_2", PASSWORD, HttpStatus.SC_OK); + } + + @Test + public void shouldNotAuthenticateWithIncorrectPassword() { + String hash = generatePBKDF2Hash(PASSWORD, function, iterations, length); + createUserWithHashedPassword(cluster, "user_3", hash); + testPasswordAuth(cluster, "user_3", "wrong_password", HttpStatus.SC_UNAUTHORIZED); + + createUserWithPlainTextPassword(cluster, "user_4", PASSWORD); + testPasswordAuth(cluster, "user_4", "wrong_password", HttpStatus.SC_UNAUTHORIZED); + } +} diff --git a/src/integrationTest/java/org/opensearch/security/hash/PBKDF2DefaultConfigHashingTests.java b/src/integrationTest/java/org/opensearch/security/hash/PBKDF2DefaultConfigHashingTests.java new file mode 100644 index 0000000000..2becee1f36 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/hash/PBKDF2DefaultConfigHashingTests.java @@ -0,0 +1,85 @@ +/* + * 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.hash; + +import java.util.List; +import java.util.Map; + +import org.apache.http.HttpStatus; +import org.junit.ClassRule; +import org.junit.Test; + +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; + +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; +import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; + +public class PBKDF2DefaultConfigHashingTests extends HashingTests { + + private static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS) + .hash( + generatePBKDF2Hash( + "secret", + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT + ) + ); + + @ClassRule + public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) + .authc(AUTHC_HTTPBASIC_INTERNAL) + .users(ADMIN_USER) + .anonymousAuth(false) + .nodeSettings( + Map.of( + ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED, + List.of("user_" + ADMIN_USER.getName() + "__" + ALL_ACCESS.getName()), + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, + ConfigConstants.PBKDF2 + ) + ) + .build(); + + @Test + public void shouldAuthenticateWithCorrectPassword() { + String hash = generatePBKDF2Hash( + PASSWORD, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT + ); + createUserWithHashedPassword(cluster, "user_1", hash); + testPasswordAuth(cluster, "user_1", PASSWORD, HttpStatus.SC_OK); + + createUserWithPlainTextPassword(cluster, "user_2", PASSWORD); + testPasswordAuth(cluster, "user_2", PASSWORD, HttpStatus.SC_OK); + } + + @Test + public void shouldNotAuthenticateWithIncorrectPassword() { + String hash = generatePBKDF2Hash( + PASSWORD, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT + ); + createUserWithHashedPassword(cluster, "user_3", hash); + testPasswordAuth(cluster, "user_3", "wrong_password", HttpStatus.SC_UNAUTHORIZED); + + createUserWithPlainTextPassword(cluster, "user_4", PASSWORD); + testPasswordAuth(cluster, "user_4", "wrong_password", HttpStatus.SC_UNAUTHORIZED); + } +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java index f29edeee24..81604d1376 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java +++ b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java @@ -50,14 +50,16 @@ import org.opensearch.action.index.IndexRequest; import org.opensearch.action.update.UpdateRequest; import org.opensearch.client.Client; +import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.common.Strings; 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.hasher.PasswordHasherFactory; import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.support.ConfigConstants; import org.opensearch.test.framework.cluster.OpenSearchClientProvider.UserCredentialsHolder; import static org.apache.http.HttpHeaders.AUTHORIZATION; @@ -79,7 +81,10 @@ public class TestSecurityConfig { public static final 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 static final PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher( + Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build() + ); private Config config = new Config(); private Map internalUsers = new LinkedHashMap<>(); @@ -414,6 +419,8 @@ public static final class User implements UserCredentialsHolder, ToXContentObjec private String description; + private String hash; + public User(String name) { this(name, null); } @@ -458,6 +465,11 @@ public User attr(String key, String value) { return this; } + public User hash(String hash) { + this.hash = hash; + return this; + } + public String getName() { return name; } @@ -478,8 +490,11 @@ public Object getAttribute(String attributeName) { public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException { xContentBuilder.startObject(); - xContentBuilder.field("hash", hash(password.toCharArray())); - + if (this.hash == null) { + xContentBuilder.field("hash", hashPassword(password)); + } else { + xContentBuilder.field("hash", hash); + } Set roleNames = getRoleNames(); if (!roleNames.isEmpty()) { @@ -968,8 +983,8 @@ public void updateInternalUsersConfiguration(Client client, List users) { updateConfigInIndex(client, CType.INTERNALUSERS, userMap); } - static String hash(final char[] clearTextPassword) { - return passwordHasher.hash(clearTextPassword); + static String hashPassword(final String clearTextPassword) { + return passwordHasher.hash(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 76734e4e81..80232af37b 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -158,8 +158,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.hasher.PasswordHasherFactory; import org.opensearch.security.http.NonSslHttpServerTransport; import org.opensearch.security.http.XFFResolver; import org.opensearch.security.identity.SecurityTokenManager; @@ -1105,7 +1105,7 @@ public Collection createComponents( cr = ConfigurationRepository.create(settings, this.configPath, threadPool, localClient, clusterService, auditLog); - this.passwordHasher = new BCryptPasswordHasher(); + this.passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); userService = new UserService(cs, cr, passwordHasher, settings, localClient); @@ -1306,6 +1306,60 @@ public List> getSettings() { ) ); + settings.add( + Setting.simpleString( + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM_DEFAULT, + Property.NodeScope, + Property.Final + ) + ); + + settings.add( + Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS_DEFAULT, + Property.NodeScope, + Property.Final + ) + ); + + settings.add( + Setting.simpleString( + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR_DEFAULT, + Property.NodeScope, + Property.Final + ) + ); + + settings.add( + Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT, + Property.NodeScope, + Property.Final + ) + ); + + settings.add( + Setting.intSetting( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT, + Property.NodeScope, + Property.Final + ) + ); + + settings.add( + Setting.simpleString( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION_DEFAULT, + Property.NodeScope, + Property.Final + ) + ); + if (!SSLConfig.isSslOnlyMode()) { settings.add( Setting.listSetting( diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java index 338ce222e5..859b0f9e4e 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java @@ -73,8 +73,10 @@ public final class AuditMessage { ); @VisibleForTesting - public static final Pattern BCRYPT_HASH = Pattern.compile("\\$2[ayb]\\$.{56}"); - private static final String BCRYPT_HASH_REPLACEMENT_VALUE = "__HASH__"; + public static final String BCRYPT_REGEX = "\\$2[ayb]\\$.{56}"; + public static final String PBKDF2_REGEX = "\\$\\d+\\$\\d+\\$[A-Za-z0-9+/]+={0,2}\\$[A-Za-z0-9+/]+={0,2}"; + public static final Pattern HASH_REGEX_PATTERN = Pattern.compile(BCRYPT_REGEX + "|" + PBKDF2_REGEX); + private static final String HASH_REPLACEMENT_VALUE = "__HASH__"; private static final String INTERNALUSERS_DOC_ID = CType.INTERNALUSERS.toLCString(); public static final String FORMAT_VERSION = "audit_format_version"; @@ -238,7 +240,7 @@ public void addUnescapedJsonToRequestBody(String source) { private String redactSecurityConfigContent(String content, String id) { if (content != null && INTERNALUSERS_DOC_ID.equals(id)) { - content = BCRYPT_HASH.matcher(content).replaceAll(BCRYPT_HASH_REPLACEMENT_VALUE); + content = HASH_REGEX_PATTERN.matcher(content).replaceAll(HASH_REPLACEMENT_VALUE); } return content; } diff --git a/src/main/java/org/opensearch/security/hasher/AbstractPasswordHasher.java b/src/main/java/org/opensearch/security/hasher/AbstractPasswordHasher.java new file mode 100644 index 0000000000..151f657de1 --- /dev/null +++ b/src/main/java/org/opensearch/security/hasher/AbstractPasswordHasher.java @@ -0,0 +1,79 @@ +/* + * 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.util.Arrays; + +import org.opensearch.OpenSearchSecurityException; + +import com.password4j.HashingFunction; + +import static org.opensearch.core.common.Strings.isNullOrEmpty; + +/** + * Abstract implementation of PasswordHasher interface + */ +abstract class AbstractPasswordHasher implements PasswordHasher { + + /** + * The hashing function used by the hasher. + */ + HashingFunction hashingFunction; + + /** + * {@inheritDoc} + */ + public abstract String hash(char[] password); + + /** + * {@inheritDoc} + */ + public abstract boolean check(char[] password, String hash); + + /** + * Clears the given password buffer to prevent sensitive data from being left in memory. + * + * @param password the CharBuffer containing the password to clear + */ + protected void cleanup(CharBuffer password) { + password.clear(); + char[] passwordOverwrite = new char[password.capacity()]; + Arrays.fill(passwordOverwrite, '\0'); + password.put(passwordOverwrite); + } + + /** + * Checks if the given password is null or empty and throws an exception if it is. + * + * @param password the password to check + * @throws OpenSearchSecurityException if the password is null or empty + */ + protected void checkPasswordNotNullOrEmpty(char[] password) { + if (password == null || password.length == 0) { + throw new OpenSearchSecurityException("Password cannot be empty or null"); + } + } + + /** + * Checks if the given hash is null or empty and throws an exception if it is. + * + * @param hash the hash to check + * @throws OpenSearchSecurityException if the hash is null or empty + */ + protected void checkHashNotNullOrEmpty(String hash) { + if (isNullOrEmpty(hash)) { + throw new OpenSearchSecurityException("Hash cannot be empty or null"); + } + } + +} diff --git a/src/main/java/org/opensearch/security/hasher/BCryptPasswordHasher.java b/src/main/java/org/opensearch/security/hasher/BCryptPasswordHasher.java index 043c0392d9..10533850a1 100644 --- a/src/main/java/org/opensearch/security/hasher/BCryptPasswordHasher.java +++ b/src/main/java/org/opensearch/security/hasher/BCryptPasswordHasher.java @@ -14,9 +14,7 @@ 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; @@ -24,17 +22,15 @@ import com.password4j.Password; import com.password4j.types.Bcrypt; -import static org.opensearch.core.common.Strings.isNullOrEmpty; +class BCryptPasswordHasher extends AbstractPasswordHasher { -public class BCryptPasswordHasher implements PasswordHasher { - - private static final HashingFunction DEFAULT_BCRYPT_FUNCTION = BcryptFunction.getInstance(Bcrypt.Y, 12); + BCryptPasswordHasher(String minor, int logRounds) { + this.hashingFunction = BcryptFunction.getInstance(Bcrypt.valueOf(minor), logRounds); + } @Override public String hash(char[] password) { - if (password == null || password.length == 0) { - throw new OpenSearchSecurityException("Password cannot be empty or null"); - } + checkPasswordNotNullOrEmpty(password); CharBuffer passwordBuffer = CharBuffer.wrap(password); try { SecurityManager securityManager = System.getSecurityManager(); @@ -42,7 +38,7 @@ public String hash(char[] password) { securityManager.checkPermission(new SpecialPermission()); } return AccessController.doPrivileged( - (PrivilegedAction) () -> Password.hash(passwordBuffer).with(DEFAULT_BCRYPT_FUNCTION).getResult() + (PrivilegedAction) () -> Password.hash(passwordBuffer).with(hashingFunction).getResult() ); } finally { cleanup(passwordBuffer); @@ -51,12 +47,8 @@ public String hash(char[] password) { @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"); - } + checkPasswordNotNullOrEmpty(password); + checkHashNotNullOrEmpty(hash); CharBuffer passwordBuffer = CharBuffer.wrap(password); try { SecurityManager securityManager = System.getSecurityManager(); @@ -71,13 +63,6 @@ public boolean check(char[] password, String hash) { } } - 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/PBKDF2PasswordHasher.java b/src/main/java/org/opensearch/security/hasher/PBKDF2PasswordHasher.java new file mode 100644 index 0000000000..36828371ad --- /dev/null +++ b/src/main/java/org/opensearch/security/hasher/PBKDF2PasswordHasher.java @@ -0,0 +1,82 @@ +/* + * 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 org.opensearch.SpecialPermission; + +import com.password4j.CompressedPBKDF2Function; +import com.password4j.HashingFunction; +import com.password4j.Password; + +class PBKDF2PasswordHasher extends AbstractPasswordHasher { + + private static final int DEFAULT_SALT_LENGTH = 128; + + @SuppressWarnings("removal") + PBKDF2PasswordHasher(String function, int iterations, int length) { + SecurityManager securityManager = System.getSecurityManager(); + if (securityManager != null) { + securityManager.checkPermission(new SpecialPermission()); + } + this.hashingFunction = AccessController.doPrivileged( + (PrivilegedAction) () -> CompressedPBKDF2Function.getInstance(function, iterations, length) + ); + } + + @Override + @SuppressWarnings("removal") + public String hash(char[] password) { + checkPasswordNotNullOrEmpty(password); + CharBuffer passwordBuffer = CharBuffer.wrap(password); + try { + SecurityManager securityManager = System.getSecurityManager(); + if (securityManager != null) { + securityManager.checkPermission(new SpecialPermission()); + } + return AccessController.doPrivileged( + (PrivilegedAction) () -> Password.hash(passwordBuffer) + .addRandomSalt(DEFAULT_SALT_LENGTH) + .with(hashingFunction) + .getResult() + ); + } finally { + cleanup(passwordBuffer); + } + } + + @SuppressWarnings("removal") + @Override + public boolean check(char[] password, String hash) { + checkPasswordNotNullOrEmpty(password); + checkHashNotNullOrEmpty(hash); + 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(getPBKDF2FunctionFromHash(hash)) + ); + } finally { + cleanup(passwordBuffer); + } + } + + private HashingFunction getPBKDF2FunctionFromHash(String hash) { + return CompressedPBKDF2Function.getInstanceFromHash(hash); + } +} diff --git a/src/main/java/org/opensearch/security/hasher/PasswordHasher.java b/src/main/java/org/opensearch/security/hasher/PasswordHasher.java index deaab7e073..b115feac58 100644 --- a/src/main/java/org/opensearch/security/hasher/PasswordHasher.java +++ b/src/main/java/org/opensearch/security/hasher/PasswordHasher.java @@ -11,9 +11,26 @@ package org.opensearch.security.hasher; +/** + * Interface representing a password hasher which provides methods + * to hash a password and check a password against a hashed password. + */ public interface PasswordHasher { + /** + * Generates a hashed representation of the given password. + * + * @param password the password to hash + * @return a hashed representation of the password + */ String hash(char[] password); + /** + * Checks if the given password matches the provided hashed password. + * + * @param password the password to check + * @param hashedPassword the hashed password to check against + * @return true if the password matches the hashed password, false otherwise + */ boolean check(char[] password, String hashedPassword); } diff --git a/src/main/java/org/opensearch/security/hasher/PasswordHasherFactory.java b/src/main/java/org/opensearch/security/hasher/PasswordHasherFactory.java new file mode 100644 index 0000000000..ad2dc7eb57 --- /dev/null +++ b/src/main/java/org/opensearch/security/hasher/PasswordHasherFactory.java @@ -0,0 +1,93 @@ +/* + * 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.util.Set; + +import org.opensearch.common.settings.Settings; +import org.opensearch.security.support.ConfigConstants; + +import static org.opensearch.security.support.ConfigConstants.BCRYPT; +import static org.opensearch.security.support.ConfigConstants.PBKDF2; + +public class PasswordHasherFactory { + + private static final Set ALLOWED_BCRYPT_MINORS = Set.of("A", "B", "Y"); + + public static PasswordHasher createPasswordHasher(Settings settings) { + String algorithm = settings.get( + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, + ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM_DEFAULT + ); + + PasswordHasher passwordHasher; + switch (algorithm.toLowerCase()) { + case BCRYPT: + passwordHasher = getBCryptHasher(settings); + break; + case PBKDF2: + passwordHasher = getPBKDF2Hasher(settings); + break; + default: + throw new IllegalArgumentException(String.format("Password hashing algorithm '%s' not supported.", algorithm)); + } + return passwordHasher; + } + + private static PasswordHasher getBCryptHasher(Settings settings) { + 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 + ).toUpperCase(); + + if (rounds < 4 || rounds > 31) { + throw new IllegalArgumentException(String.format("BCrypt rounds must be between 4 and 31. Got: %d", rounds)); + } + if (!ALLOWED_BCRYPT_MINORS.contains(minor)) { + throw new IllegalArgumentException(String.format("BCrypt minor must be 'A', 'B', or 'Y'. Got: %s", minor)); + } + return new BCryptPasswordHasher(minor, rounds); + } + + private static PasswordHasher getPBKDF2Hasher(Settings settings) { + String pbkdf2Function = settings.get( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION_DEFAULT + ).toUpperCase(); + + 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 + ); + + if (!pbkdf2Function.matches("SHA(1|224|256|384|512)")) { + throw new IllegalArgumentException( + String.format("PBKDF2 function must be one of SHA1, SHA224, SHA256, SHA384, or SHA512. Got: %s", pbkdf2Function) + ); + } + if (iterations <= 0) { + throw new IllegalArgumentException(String.format("PBKDF2 iterations must be a positive integer. Got: %d", iterations)); + } + if (length <= 0) { + throw new IllegalArgumentException(String.format("PBKDF2 length must be a positive integer. Got: %d", length)); + } + return new PBKDF2PasswordHasher(pbkdf2Function, iterations, length); + } +} diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index 0c383d463e..d2026c00e0 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -38,6 +38,8 @@ import org.opensearch.common.settings.Settings; import org.opensearch.security.auditlog.impl.AuditCategory; +import com.password4j.types.Hmac; + public class ConfigConstants { public static final String OPENDISTRO_SECURITY_CONFIG_PREFIX = "_opendistro_security_"; @@ -142,6 +144,23 @@ 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 BCRYPT = "bcrypt"; + public static final String PBKDF2 = "pbkdf2"; + + 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 = 12; + 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 = "Y"; + + 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 = 600_000; + 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 = 256; + public static final String SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION = "plugins.security.password.hashing.pbkdf2.function"; + public static final String SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION_DEFAULT = Hmac.SHA256.name(); + 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 76a4fec5c6..21ad0a62df 100644 --- a/src/main/java/org/opensearch/security/tools/Hasher.java +++ b/src/main/java/org/opensearch/security/tools/Hasher.java @@ -28,51 +28,72 @@ import java.io.Console; -import org.apache.commons.cli.CommandLine; -import org.apache.commons.cli.CommandLineParser; -import org.apache.commons.cli.DefaultParser; -import org.apache.commons.cli.HelpFormatter; -import org.apache.commons.cli.Option; -import org.apache.commons.cli.Options; - -import org.opensearch.security.hasher.BCryptPasswordHasher; +import org.apache.commons.cli.*; + +import org.opensearch.common.settings.Settings; import org.opensearch.security.hasher.PasswordHasher; +import org.opensearch.security.hasher.PasswordHasherFactory; +import org.opensearch.security.support.ConfigConstants; public class Hasher { - public static void main(final String[] args) { + private static final String PASSWORD_OPTION = "p"; + private static final String ENV_OPTION = "env"; + private static final String ALGORITHM_OPTION = "a"; + private static final String ROUNDS_OPTION = "r"; + private static final String FUNCTION_OPTION = "f"; + private static final String LENGTH_OPTION = "l"; + private static final String ITERATIONS_OPTION = "i"; + private static final String MINOR_OPTION = "min"; + private static final String HELP_OPTION = "h"; - final Options options = new Options(); + public static void main(final String[] args) { final HelpFormatter formatter = new HelpFormatter(); - options.addOption(Option.builder("p").argName("password").hasArg().desc("Cleartext password to hash").build()); - options.addOption( - Option.builder("env") - .argName("name environment variable") - .hasArg() - .desc("name environment variable to read password from") - .build() - ); - + formatter.setOptionComparator(null); + Options options = buildOptions(); final CommandLineParser parser = new DefaultParser(); try { final CommandLine line = parser.parse(options, args); + final char[] password; + + if (line.hasOption(HELP_OPTION)) { + formatter.printHelp("hash.sh", options, true); + System.exit(0); + } - if (line.hasOption("p")) { - System.out.println(hash(line.getOptionValue("p").toCharArray())); - } else if (line.hasOption("env")) { - final String pwd = System.getenv(line.getOptionValue("env")); + if (line.hasOption(PASSWORD_OPTION)) { + password = line.getOptionValue(PASSWORD_OPTION).toCharArray(); + } else if (line.hasOption(ENV_OPTION)) { + final String pwd = System.getenv(line.getOptionValue(ENV_OPTION)); if (pwd == null || pwd.isEmpty()) { - throw new Exception("No environment variable '" + line.getOptionValue("env") + "' set"); + throw new Exception("No environment variable '" + line.getOptionValue(ENV_OPTION) + "' set"); } - System.out.println(hash(pwd.toCharArray())); + password = pwd.toCharArray(); } else { final Console console = System.console(); if (console == null) { throw new Exception("Cannot allocate a console"); } - final char[] passwd = console.readPassword("[%s]", "Password:"); - System.out.println(hash(passwd)); + password = console.readPassword("[%s]", "Password:"); } + if (line.hasOption(ALGORITHM_OPTION)) { + String algorithm = line.getOptionValue(ALGORITHM_OPTION); + Settings settings; + switch (algorithm.toLowerCase()) { + case ConfigConstants.BCRYPT: + settings = getBCryptSettings(line); + break; + case ConfigConstants.PBKDF2: + settings = getPBKDF2Settings(line); + break; + default: + throw new Exception("Unsupported hashing algorithm: " + algorithm); + } + System.out.println(hash(password, settings)); + } else { + System.out.println(hash(password)); + } + } catch (final Exception exp) { System.err.println("Parsing failed. Reason: " + exp.getMessage()); formatter.printHelp("hash.sh", options, true); @@ -81,7 +102,116 @@ public static void main(final String[] args) { } public static String hash(final char[] clearTextPassword) { - PasswordHasher passwordHasher = new BCryptPasswordHasher(); + return hash(clearTextPassword, Settings.EMPTY); + } + + public static String hash(final char[] clearTextPassword, Settings settings) { + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); return passwordHasher.hash(clearTextPassword); } + + private static Settings getBCryptSettings(CommandLine line) throws ParseException { + Settings.Builder settings = Settings.builder(); + settings.put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT); + if (line.hasOption(ROUNDS_OPTION)) { + settings.put( + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS, + ((Number) line.getParsedOptionValue(ROUNDS_OPTION)).intValue() + ); + } + if (line.hasOption(MINOR_OPTION)) { + settings.put(ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR, line.getOptionValue(MINOR_OPTION).toUpperCase()); + } + return settings.build(); + } + + private static Settings getPBKDF2Settings(CommandLine line) throws ParseException { + Settings.Builder settings = Settings.builder(); + settings.put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2); + if (line.hasOption(FUNCTION_OPTION)) { + settings.put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, line.getOptionValue(FUNCTION_OPTION)); + } + if (line.hasOption(LENGTH_OPTION)) { + settings.put( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH, + ((Number) line.getParsedOptionValue(LENGTH_OPTION)).intValue() + ); + } + if (line.hasOption(ITERATIONS_OPTION)) { + settings.put( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS, + ((Number) line.getParsedOptionValue(ITERATIONS_OPTION)).intValue() + ); + } + return settings.build(); + } + + private static Options buildOptions() { + final Options options = new Options(); + options.addOption(Option.builder(HELP_OPTION).longOpt("help").desc("Display the help information").argName("help").build()); + options.addOption( + Option.builder(PASSWORD_OPTION).longOpt("password").argName("password").hasArg().desc("Cleartext password to hash").build() + ); + options.addOption( + Option.builder(ENV_OPTION) + .argName("Environment variable name") + .hasArg() + .desc("Environment variable name to read password from") + .build() + ); + options.addOption( + Option.builder(ALGORITHM_OPTION) + .longOpt("algorithm") + .argName("hashing algorithm") + .hasArg() + .desc("Algorithm to use for password hashing. Valid values are: BCrypt | PBKDF2. Default: BCrypt") + .build() + ); + options.addOption( + Option.builder(ROUNDS_OPTION) + .longOpt("rounds") + .desc("Number of rounds to use in logarithmic form. Valid values are: 4 to 31. Default: 12") + .hasArg() + .argName("rounds (BCrypt)") + .type(Number.class) + .build() + ); + options.addOption( + Option.builder(MINOR_OPTION) + .longOpt("minor") + .desc("Version of BCrypt algorithm to use. Valid values are: A | B | Y. Default: Y") + .hasArg() + .argName("minor (BCrypt)") + .build() + ); + options.addOption( + Option.builder(LENGTH_OPTION) + .longOpt("length") + .desc("Desired length of the final derived key. Default: 256") + .hasArg() + .argName("length (PBKDF2)") + .type(Number.class) + .build() + ); + options.addOption( + Option.builder(FUNCTION_OPTION) + .longOpt("function") + .desc( + "Pseudo-random function applied to the password. Valid values are SHA1 | SHA224 | SHA256 | SHA384 | SHA512. Default: SHA256" + ) + .hasArg() + .argName("function (PBDKF2)") + .build() + ); + options.addOption( + Option.builder(ITERATIONS_OPTION) + .longOpt("iterations") + .desc("Number of times the pseudo-random function is applied to the password. Default: 600000") + .hasArg() + .argName("iterations (PBKDF2)") + .type(Number.class) + .build() + ); + return options; + } } 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 af509def1a..d97fe2d1bc 100644 --- a/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java +++ b/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java @@ -29,16 +29,14 @@ 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.hasher.PasswordHasherFactory; import org.opensearch.security.support.ConfigConstants; import org.yaml.snakeyaml.DumperOptions; import org.yaml.snakeyaml.Yaml; import static org.opensearch.security.DefaultObjectMapper.YAML_MAPPER; -import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH; -import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX; /** * This class updates the security related configuration, as needed. @@ -88,7 +86,9 @@ public class SecuritySettingsConfigurer { public SecuritySettingsConfigurer(Installer installer) { this.installer = installer; - this.passwordHasher = new BCryptPasswordHasher(); + this.passwordHasher = PasswordHasherFactory.createPasswordHasher( + Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build() + ); } /** @@ -144,8 +144,11 @@ void updateAdminPassword() throws IOException { try { final PasswordValidator passwordValidator = PasswordValidator.of( Settings.builder() - .put(SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}") - .put(SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, DEFAULT_PASSWORD_MIN_LENGTH) + .put( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, + "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}" + ) + .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, DEFAULT_PASSWORD_MIN_LENGTH) .build() ); @@ -201,7 +204,6 @@ 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") && passwordHasher.check(DEFAULT_ADMIN_PASSWORD.toCharArray(), internalUsers.get("admin").get("hash").asText()); } diff --git a/src/test/java/org/opensearch/security/UserServiceUnitTests.java b/src/test/java/org/opensearch/security/UserServiceUnitTests.java index 0cc758eff9..578273d291 100644 --- a/src/test/java/org/opensearch/security/UserServiceUnitTests.java +++ b/src/test/java/org/opensearch/security/UserServiceUnitTests.java @@ -28,10 +28,11 @@ 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.hasher.PasswordHasherFactory; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.UserFilterType; import org.opensearch.security.user.UserService; @@ -66,9 +67,9 @@ public class UserServiceUnitTests { @Before public void setup() throws Exception { String usersYmlFile = "./internal_users.yml"; - Settings.Builder builder = Settings.builder(); - PasswordHasher passwordHasher = new BCryptPasswordHasher(); - userService = new UserService(clusterService, configurationRepository, passwordHasher, builder.build(), client); + Settings settings = Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build(); + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); + userService = new UserService(clusterService, configurationRepository, passwordHasher, settings, 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 cb98790632..195297a440 100644 --- a/src/test/java/org/opensearch/security/UtilTests.java +++ b/src/test/java/org/opensearch/security/UtilTests.java @@ -31,8 +31,8 @@ import org.junit.Test; import org.opensearch.common.settings.Settings; -import org.opensearch.security.hasher.BCryptPasswordHasher; import org.opensearch.security.hasher.PasswordHasher; +import org.opensearch.security.hasher.PasswordHasherFactory; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.SecurityUtils; import org.opensearch.security.support.WildcardMatcher; @@ -52,7 +52,9 @@ static private WildcardMatcher iwc(String pattern) { return WildcardMatcher.from(pattern, false); } - static private final PasswordHasher passwordHasher = new BCryptPasswordHasher(); + static private final PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher( + Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build() + ); @Test public void testWildcardMatcherClasses() { diff --git a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java index b671378ad4..b4965d60ad 100644 --- a/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java +++ b/src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java @@ -39,6 +39,10 @@ protected String getResourceFolder() { } protected final void setup(Settings settings) throws Exception { + setup(settings, new DynamicSecurityConfig()); + } + + protected final void setup(Settings settings, DynamicSecurityConfig securityConfig) throws Exception { final Settings.Builder auditConfigSettings = Settings.builder(); final Settings.Builder defaultNodeSettings = Settings.builder(); // Separate the cluster defaults from audit settings that will be applied after the cluster is up @@ -56,7 +60,7 @@ protected final void setup(Settings settings) throws Exception { }); final Settings nodeSettings = defaultNodeSettings(defaultNodeSettings.build()); - setup(Settings.EMPTY, new DynamicSecurityConfig(), nodeSettings, init); + setup(Settings.EMPTY, securityConfig, nodeSettings, init); rh = restHelper(); updateAuditConfig(auditConfigSettings.build()); } diff --git a/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java index 25295a2e05..e07ff5e113 100644 --- a/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java @@ -1,13 +1,13 @@ /* - * 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. - */ +* 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.auditlog.compliance; @@ -24,6 +24,7 @@ import org.opensearch.security.auditlog.impl.AuditMessage; import org.opensearch.security.auditlog.integration.TestAuditlogImpl; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.security.test.helper.cluster.ClusterConfiguration; import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; @@ -36,7 +37,6 @@ public class RestApiComplianceAuditlogTest extends AbstractAuditlogiUnitTest { @Test public void testRestApiRolesEnabled() throws Exception { - Settings additionalSettings = Settings.builder() .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) .put(ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED, "opendistro_security_all_access") @@ -226,25 +226,147 @@ public void testBCryptHashRedaction() throws Exception { rh.executeGetRequest("/_opendistro/_security/api/internalusers"); }); - Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(message1.toString()).matches()); + Assert.assertFalse(AuditMessage.HASH_REGEX_PATTERN.matcher(message1.toString()).matches()); // read internal user worf and verify no BCrypt hash is present in audit logs final AuditMessage message2 = TestAuditlogImpl.doThenWaitForMessage(() -> { rh.executeGetRequest("/_opendistro/_security/api/internalusers/worf"); - Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(TestAuditlogImpl.sb.toString()).matches()); + Assert.assertFalse(AuditMessage.HASH_REGEX_PATTERN.matcher(TestAuditlogImpl.sb.toString()).matches()); }); - Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(message2.toString()).matches()); + Assert.assertFalse(AuditMessage.HASH_REGEX_PATTERN.matcher(message2.toString()).matches()); // create internal user and verify no BCrypt hash is present in audit logs final AuditMessage message3 = TestAuditlogImpl.doThenWaitForMessage(() -> { rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"some new user password\"}"); }); - Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(message3.toString()).matches()); + Assert.assertFalse(AuditMessage.HASH_REGEX_PATTERN.matcher(message3.toString()).matches()); + } + + @Test + public void testPBKDF2HashRedaction() { + final Settings settings = Settings.builder() + .put("plugins.security.audit.type", TestAuditlogImpl.class.getName()) + .put(ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED, "opendistro_security_all_access") + .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_REST, false) + .put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, false) + .put(ConfigConstants.SECURITY_COMPLIANCE_HISTORY_INTERNAL_CONFIG_ENABLED, true) + .put(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_LOG_DIFFS, true) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .build(); + final DynamicSecurityConfig securityConfig = new DynamicSecurityConfig().setSecurityInternalUsers("internal_users_pbkdf2.yml"); + setupAndReturnAuditMessages(settings, securityConfig); + rh.sendAdminCertificate = true; + rh.keystore = "kirk-keystore.jks"; + + // read internal users and verify no PBKDF2 hash is present in audit logs + final AuditMessage message1 = TestAuditlogImpl.doThenWaitForMessage(() -> { + rh.executeGetRequest("/_opendistro/_security/api/internalusers"); + }); + + Assert.assertFalse( + message1.toString() + .contains( + "$3$1331439861760512$wBFrJJIAokWuJxlO6BQPLashXgznvR4tRmXk3aEy9SpHWrb9kFjPPLByZZzMLBNQFjhepgbngYh7RfMh8TrPLw==$vqGlzGsxqGf9TgfxhORjdoqRFB3npvBd9B0GAtBMg9mD2zBbSTohRYlOxUL7UQLma66zZdD67c4RNE9BKelabw==" + ) + ); + Assert.assertTrue(message1.toString().contains("__HASH__")); + + // read internal user and verify no PBKDF2 hash is present in audit logs + final AuditMessage message2 = TestAuditlogImpl.doThenWaitForMessage(() -> { + rh.executeGetRequest("/_opendistro/_security/api/internalusers/user1"); + }); + + Assert.assertFalse( + message2.toString() + .contains( + "$3$1331439861760512$wBFrJJIAokWuJxlO6BQPLashXgznvR4tRmXk3aEy9SpHWrb9kFjPPLByZZzMLBNQFjhepgbngYh7RfMh8TrPLw==$vqGlzGsxqGf9TgfxhORjdoqRFB3npvBd9B0GAtBMg9mD2zBbSTohRYlOxUL7UQLma66zZdD67c4RNE9BKelabw==" + ) + ); + Assert.assertTrue(message2.toString().contains("__HASH__")); + + // create internal user and verify no PBKDF2 hash is present in audit logs + final AuditMessage message3 = TestAuditlogImpl.doThenWaitForMessage(() -> { + rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"some new user password\"}"); + }); + + Assert.assertFalse( + message3.toString() + .contains( + "$3$1331439861760512$wBFrJJIAokWuJxlO6BQPLashXgznvR4tRmXk3aEy9SpHWrb9kFjPPLByZZzMLBNQFjhepgbngYh7RfMh8TrPLw==$vqGlzGsxqGf9TgfxhORjdoqRFB3npvBd9B0GAtBMg9mD2zBbSTohRYlOxUL7UQLma66zZdD67c4RNE9BKelabw==" + ) + ); + Assert.assertTrue(message3.toString().contains("__HASH__")); + + // test with various users and different PBKDF2 hash formats to make sure they all get redacted + final AuditMessage message4 = TestAuditlogImpl.doThenWaitForMessage(() -> { + rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user1", "user1")); + }); + + Assert.assertFalse( + message4.toString() + .contains( + "$1$4294967296128$VmnDMbQ4wLiFUq178RKvj+EYfdb3Q26qCiDcJDoCxpYnKpyuG0JhTC2wpUkMUveV5RmBFzldKQkdqZEfE0XAgg==$9u3aMWc6VP2oGkXei7UaXA==" + ) + ); + Assert.assertTrue(message4.toString().contains("__HASH__")); + + final AuditMessage message5 = TestAuditlogImpl.doThenWaitForMessage(() -> { + rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user2", "user2")); + }); + + Assert.assertFalse( + message5.toString() + .contains( + "$2$214748364800224$eQgqv2RI6yo95yeVnM5sfwUCwxHo6re0w+wpx6ZqZtHQV+dzlyP6YFitjNG2mlaTkg0pR56xArQaAgapdVcBQQ==$tGHWhoc83cd5nZ7QIZKPORjW/N5jklhYhRgXpw==" + ) + ); + Assert.assertTrue(message5.toString().contains("__HASH__")); + + final AuditMessage message6 = TestAuditlogImpl.doThenWaitForMessage(() -> { + rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user3", "user3")); + }); + + Assert.assertFalse( + message6.toString() + .contains( + "$3$322122547200256$5b3wEAsMc05EZFxfncCUfZRERgvbwlBhYXd5vVR14kNJtmhXSpYMzydRZxO9096IPTkc47doH4hIdKX6LTguLg==$oQQvAtUyOC6cwdAYi5WeIM7rGUN9l3IdJ9y2RNxZCWo=" + ) + ); + Assert.assertTrue(message6.toString().contains("__HASH__")); + + final AuditMessage message7 = TestAuditlogImpl.doThenWaitForMessage(() -> { + rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user4", "user4")); + }); + + Assert.assertFalse( + message7.toString() + .contains( + "$4$429496729600384$+SNSgbZD67a1bd92iuEiHCq5pvvrCx3HrNIf5hbGIJdxgXegpWilpB6vUGvYigegAUzZqE9iIsL4pSJztUNJYw==$lTxZ7tax6dBQ0r4qPJpc8d7YuoTBiUujY9HJeAZvARXMjIgvnJwa6FeYugttOKc0" + ) + ); + Assert.assertTrue(message7.toString().contains("__HASH__")); + + final AuditMessage message8 = TestAuditlogImpl.doThenWaitForMessage(() -> { + rh.executeGetRequest("/_opendistro/_security/api/internalusers", encodeBasicHeader("user5", "user5")); + }); + + Assert.assertFalse( + message8.toString() + .contains( + "$5$644245094400512$HQe/MOv/NAlgodNhqTmjqj5jGxBwG5xuRaxKwn7r4nlUba1kj/CYnpdFaXGvVeRxt2NLm8fbekS6NYonv358Ew==$1sDx+0tMbtGzU6jlQg/Dyt30Yxuy5RdNmP9B1EzMTxYWi8k1xg2gXLy7w1XbetEC8UD/lpyXJPlaoxXpsaADyA==" + ) + ); + Assert.assertTrue(message8.toString().contains("__HASH__")); + } private List setupAndReturnAuditMessages(Settings settings) { + return setupAndReturnAuditMessages(settings, new DynamicSecurityConfig()); + } + + private List setupAndReturnAuditMessages(Settings settings, DynamicSecurityConfig dynamicSecurityConfig) { // When OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_EXTERNAL_CONFIG_ENABLED is set to true, there is: // - 1 message with COMPLIANCE_INTERNAL_CONFIG_WRITE as category. // - 1 message with COMPLIANCE_EXTERNAL_CONFIG as category for each node. @@ -256,7 +378,7 @@ private List setupAndReturnAuditMessages(Settings settings) { int expectedMessageCount = externalConfigEnabled ? (numNodes + 1) : 1; final List messages = TestAuditlogImpl.doThenWaitForMessages(() -> { try { - setup(settings); + setup(settings, dynamicSecurityConfig); } catch (final Exception ex) { throw new RuntimeException(ex); } @@ -276,4 +398,5 @@ private List setupAndReturnAuditMessages(Settings settings) { } return messages; } + } diff --git a/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java b/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java index 3832f866cb..3c4e55ca16 100644 --- a/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java +++ b/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java @@ -21,9 +21,11 @@ import org.junit.Test; import org.opensearch.OpenSearchSecurityException; +import org.opensearch.common.settings.Settings; import org.opensearch.security.auth.internal.InternalAuthenticationBackend; -import org.opensearch.security.hasher.BCryptPasswordHasher; +import org.opensearch.security.hasher.PasswordHasherFactory; import org.opensearch.security.securityconf.InternalUsersModel; +import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.AuthCredentials; import org.mockito.Mockito; @@ -43,7 +45,13 @@ public class InternalAuthBackendTests { @Before public void internalAuthBackendTestsSetup() { - internalAuthenticationBackend = spy(new InternalAuthenticationBackend(new BCryptPasswordHasher())); + internalAuthenticationBackend = spy( + new InternalAuthenticationBackend( + PasswordHasherFactory.createPasswordHasher( + Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build() + ) + ) + ); 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 065df4e5a5..b91374e725 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,10 +28,11 @@ 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.hasher.PasswordHasherFactory; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.support.ConfigConstants; import org.opensearch.threadpool.ThreadPool; import org.mockito.Mock; @@ -79,7 +80,9 @@ public void setup() { Settings.EMPTY ); - passwordHasher = new BCryptPasswordHasher(); + passwordHasher = PasswordHasherFactory.createPasswordHasher( + Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build() + ); } void setupRolesConfiguration() throws IOException { 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 46d115f79e..3676dc0274 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 @@ -18,15 +18,17 @@ import org.junit.Before; import org.junit.Test; +import org.opensearch.common.settings.Settings; 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.hasher.PasswordHasherFactory; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.securityconf.impl.v7.InternalUserV7; import org.opensearch.security.securityconf.impl.v7.RoleV7; +import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.UserService; import org.opensearch.security.util.FakeRestRequest; @@ -193,7 +195,15 @@ public void validateSecurityRolesWithImmutableRolesMappingConfig() throws Except } private InternalUsersApiAction createInternalUsersApiAction() { - return new InternalUsersApiAction(clusterService, threadPool, userService, securityApiDependencies, new BCryptPasswordHasher()); + return new InternalUsersApiAction( + clusterService, + threadPool, + userService, + securityApiDependencies, + PasswordHasherFactory.createPasswordHasher( + Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT).build() + ) + ); } } diff --git a/src/test/java/org/opensearch/security/hasher/AbstractPasswordHasherTests.java b/src/test/java/org/opensearch/security/hasher/AbstractPasswordHasherTests.java new file mode 100644 index 0000000000..065220a9d1 --- /dev/null +++ b/src/test/java/org/opensearch/security/hasher/AbstractPasswordHasherTests.java @@ -0,0 +1,92 @@ +/* + * 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 abstract class AbstractPasswordHasherTests { + + PasswordHasher passwordHasher; + + final String password = "testPassword"; + 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)); + + } + + @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())); + } + +} diff --git a/src/test/java/org/opensearch/security/hasher/BCryptPasswordHasherTests.java b/src/test/java/org/opensearch/security/hasher/BCryptPasswordHasherTests.java index 9fad74488f..ee950f1058 100644 --- a/src/test/java/org/opensearch/security/hasher/BCryptPasswordHasherTests.java +++ b/src/test/java/org/opensearch/security/hasher/BCryptPasswordHasherTests.java @@ -11,35 +11,23 @@ package org.opensearch.security.hasher; -import java.nio.CharBuffer; - +import org.junit.Before; import org.junit.Test; -import org.opensearch.OpenSearchSecurityException; +import org.opensearch.security.support.ConfigConstants; 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 { +import static org.hamcrest.Matchers.startsWith; - 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)); +public class BCryptPasswordHasherTests extends AbstractPasswordHasherTests { + @Before + public void setup() { + passwordHasher = new BCryptPasswordHasher( + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS_DEFAULT + ); } /** @@ -53,49 +41,24 @@ public void shouldBeBackwardsCompatible() { } @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); }); + public void shouldGenerateAValidHashForParameters() { + PasswordHasher hasher = new BCryptPasswordHasher("A", 8); + String hash = hasher.hash(password.toCharArray()); + assertThat(hash, startsWith("$2a$08")); + assertThat(hasher.check(password.toCharArray(), hash), is(true)); + assertThat(hasher.check(wrongPassword.toCharArray(), hash), is(false)); + + hasher = new BCryptPasswordHasher("B", 10); + hash = hasher.hash(password.toCharArray()); + assertThat(hash, startsWith("$2b$10")); + assertThat(hasher.check(password.toCharArray(), hash), is(true)); + assertThat(hasher.check(wrongPassword.toCharArray(), hash), is(false)); + + hasher = new BCryptPasswordHasher("Y", 13); + hash = hasher.hash(password.toCharArray()); + assertThat(hash, startsWith("$2y$13")); + assertThat(hasher.check(password.toCharArray(), hash), is(true)); + assertThat(hasher.check(wrongPassword.toCharArray(), hash), is(false)); } - @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())); - } } diff --git a/src/test/java/org/opensearch/security/hasher/PBKDF2PasswordHasherTests.java b/src/test/java/org/opensearch/security/hasher/PBKDF2PasswordHasherTests.java new file mode 100644 index 0000000000..6d54173a10 --- /dev/null +++ b/src/test/java/org/opensearch/security/hasher/PBKDF2PasswordHasherTests.java @@ -0,0 +1,60 @@ +/* + * 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 org.junit.Before; +import org.junit.Test; + +import org.opensearch.security.support.ConfigConstants; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +public class PBKDF2PasswordHasherTests extends AbstractPasswordHasherTests { + + @Before + public void setup() { + passwordHasher = new PBKDF2PasswordHasher( + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT, + ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT + ); + } + + @Test + public void shouldGenerateValidHashesFromParameters() { + PasswordHasher hasher = new PBKDF2PasswordHasher("SHA1", 150000, 128); + String hash = hasher.hash(password.toCharArray()); + assertThat(hasher.check(password.toCharArray(), hash), is(true)); + assertThat(hasher.check(wrongPassword.toCharArray(), hash), is(false)); + + hasher = new PBKDF2PasswordHasher("SHA224", 100000, 224); + hash = hasher.hash(password.toCharArray()); + assertThat(hasher.check(password.toCharArray(), hash), is(true)); + assertThat(hasher.check(wrongPassword.toCharArray(), hash), is(false)); + + hasher = new PBKDF2PasswordHasher("SHA256", 75000, 256); + hash = hasher.hash(password.toCharArray()); + assertThat(hasher.check(password.toCharArray(), hash), is(true)); + assertThat(hasher.check(wrongPassword.toCharArray(), hash), is(false)); + + hasher = new PBKDF2PasswordHasher("SHA384", 50000, 384); + hash = hasher.hash(password.toCharArray()); + assertThat(hasher.check(password.toCharArray(), hash), is(true)); + assertThat(hasher.check(wrongPassword.toCharArray(), hash), is(false)); + + hasher = new PBKDF2PasswordHasher("SHA512", 10000, 512); + hash = hasher.hash(password.toCharArray()); + assertThat(hasher.check(password.toCharArray(), hash), is(true)); + assertThat(hasher.check(wrongPassword.toCharArray(), hash), is(false)); + } +} diff --git a/src/test/java/org/opensearch/security/hasher/PasswordHasherFactoryTests.java b/src/test/java/org/opensearch/security/hasher/PasswordHasherFactoryTests.java new file mode 100644 index 0000000000..81e250438e --- /dev/null +++ b/src/test/java/org/opensearch/security/hasher/PasswordHasherFactoryTests.java @@ -0,0 +1,186 @@ +/* + * 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 org.junit.Test; + +import org.opensearch.common.settings.Settings; +import org.opensearch.security.support.ConfigConstants; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThrows; + +public class PasswordHasherFactoryTests { + + @Test + public void shouldReturnBCryptByDefault() { + final Settings settings = Settings.EMPTY; + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); + assertThat(passwordHasher instanceof BCryptPasswordHasher, is(true)); + } + + @Test + public void shouldReturnBCryptWhenBCryptSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT) + .build(); + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); + assertThat(passwordHasher instanceof BCryptPasswordHasher, is(true)); + } + + @Test + public void shouldReturnBCryptWhenBCryptWithValidMinorVersionSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR, "B") + .build(); + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); + assertThat(passwordHasher instanceof BCryptPasswordHasher, is(true)); + } + + @Test + public void shouldReturnBCryptWhenBCryptWithValidLogRoundsSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS, 8) + .build(); + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); + assertThat(passwordHasher instanceof BCryptPasswordHasher, is(true)); + } + + @Test + public void shouldReturnExceptionWhenInvalidBCryptMinorVersionSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR, "X") + .build(); + assertThrows(IllegalArgumentException.class, () -> { PasswordHasherFactory.createPasswordHasher(settings); }); + } + + @Test + public void shouldReturnExceptionWhenInvalidBCryptRoundsSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR, "3") + .build(); + assertThrows(IllegalArgumentException.class, () -> { PasswordHasherFactory.createPasswordHasher(settings); }); + + final Settings settings2 = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.BCRYPT) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR, "32") + .build(); + assertThrows(IllegalArgumentException.class, () -> { PasswordHasherFactory.createPasswordHasher(settings2); }); + + } + + @Test + public void shouldReturnPBKDF2WhenPBKDF2Specified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .build(); + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); + assertThat(passwordHasher instanceof PBKDF2PasswordHasher, is(true)); + } + + @Test + public void shouldReturnPBKDF2WhenPBKDF2WithValidFunctionSpecified() { + final Settings settingsSHA1 = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, "SHA1") + .build(); + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settingsSHA1); + assertThat(passwordHasher instanceof PBKDF2PasswordHasher, is(true)); + + final Settings settingsSHA224 = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, "SHA224") + .build(); + passwordHasher = PasswordHasherFactory.createPasswordHasher(settingsSHA224); + assertThat(passwordHasher instanceof PBKDF2PasswordHasher, is(true)); + + final Settings settingsSHA256 = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, "SHA256") + .build(); + passwordHasher = PasswordHasherFactory.createPasswordHasher(settingsSHA256); + assertThat(passwordHasher instanceof PBKDF2PasswordHasher, is(true)); + + final Settings settingsSHA384 = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, "SHA384") + .build(); + passwordHasher = PasswordHasherFactory.createPasswordHasher(settingsSHA384); + assertThat(passwordHasher instanceof PBKDF2PasswordHasher, is(true)); + + final Settings settingsSHA512 = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, "SHA512") + .build(); + passwordHasher = PasswordHasherFactory.createPasswordHasher(settingsSHA512); + assertThat(passwordHasher instanceof PBKDF2PasswordHasher, is(true)); + } + + @Test + public void shouldReturnPBKDF2WhenPBKDF2WithValidIterationsSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS, 32000) + .build(); + + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); + assertThat(passwordHasher instanceof PBKDF2PasswordHasher, is(true)); + } + + @Test + public void shouldReturnPBKDF2WhenPBKDF2WithValidLengthSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH, 512) + .build(); + PasswordHasher passwordHasher = PasswordHasherFactory.createPasswordHasher(settings); + assertThat(passwordHasher instanceof PBKDF2PasswordHasher, is(true)); + } + + @Test + public void shouldReturnExceptionWhenInvalidPBKDF2FunctionSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_FUNCTION, "SHA1000") + .build(); + assertThrows(IllegalArgumentException.class, () -> { PasswordHasherFactory.createPasswordHasher(settings); }); + } + + @Test + public void shouldReturnExceptionWhenInvalidPBKDF2IterationsSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS, -100000) + .build(); + assertThrows(IllegalArgumentException.class, () -> { PasswordHasherFactory.createPasswordHasher(settings); }); + } + + @Test + public void shouldReturnExceptionWhenInvalidPBKDF2LengthSpecified() { + final Settings settings = Settings.builder() + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2) + .put(ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH, -100) + .build(); + assertThrows(IllegalArgumentException.class, () -> { PasswordHasherFactory.createPasswordHasher(settings); }); + } + + @Test + public void shouldReturnExceptionWhenInvalidHashingAlgorithmSpecified() { + final Settings settings = Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, "Invalid").build(); + assertThrows(IllegalArgumentException.class, () -> { PasswordHasherFactory.createPasswordHasher(settings); }); + } +} diff --git a/src/test/java/org/opensearch/security/tools/HasherTests.java b/src/test/java/org/opensearch/security/tools/HasherTests.java new file mode 100644 index 0000000000..9b6ab8028a --- /dev/null +++ b/src/test/java/org/opensearch/security/tools/HasherTests.java @@ -0,0 +1,145 @@ +/* + * 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.tools; + +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.io.PrintStream; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import com.password4j.CompressedPBKDF2Function; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class HasherTests { + private final ByteArrayOutputStream out = new ByteArrayOutputStream(); + private final PrintStream originalOut = System.out; + private final InputStream originalIn = System.in; + + @Before + public void setOutputStreams() { + System.setOut(new PrintStream(out)); + } + + @After + public void restoreStreams() { + System.setOut(originalOut); + System.setIn(originalIn); + } + + @Test + public void testWithDefaultArguments() { + Hasher.main(new String[] { "-p", "password" }); + assertTrue("should return a valid BCrypt hash with the default BCrypt configuration", out.toString().startsWith("$2y$12")); + } + + @Test + public void testWithBCryptRoundsArgument() { + Hasher.main(new String[] { "-p", "password", "-a", "BCrypt", "-r", "5" }); + assertTrue("should return a valid BCrypt hash with the correct value for \"rounds\"", out.toString().startsWith("$2y$05")); + out.reset(); + + Hasher.main(new String[] { "-p", "password", "-a", "BCrypt", "-r", "5" }); + assertTrue("should return a valid BCrypt hash with the correct value for \"rounds\"", out.toString().startsWith("$2y$05")); + } + + @Test + public void testWithBCryptMinorArgument() { + Hasher.main(new String[] { "-p", "password", "-a", "BCrypt", "-min", "A" }); + assertTrue("should return a valid BCrypt hash with the correct value for \"minor\"", out.toString().startsWith("$2a$12")); + out.reset(); + + Hasher.main(new String[] { "-p", "password", "-a", "BCrypt", "-min", "Y" }); + assertTrue("should return a valid BCrypt hash with the correct value for \"minor\"", out.toString().startsWith("$2y$12")); + out.reset(); + + Hasher.main(new String[] { "-p", "password", "-a", "BCrypt", "-min", "B" }); + assertTrue("should return a valid BCrypt hash with the correct value for \"minor\"", out.toString().startsWith("$2b$12")); + out.reset(); + } + + @Test + public void testWithBCryptAllArguments() { + Hasher.main(new String[] { "-p", "password", "-a", "BCrypt", "-min", "A", "-r", "5" }); + assertTrue("should return a valid BCrypt hash with the correct configuration", out.toString().startsWith("$2a$05")); + } + + @Test + public void testWithPBKDF2DefaultArguments() { + Hasher.main(new String[] { "-p", "password", "-a", "PBKDF2" }); + CompressedPBKDF2Function pbkdf2Function = CompressedPBKDF2Function.getInstanceFromHash(out.toString()); + assertEquals("should return a valid PBKDF2 hash with the correct value for \"function\"", pbkdf2Function.getAlgorithm(), "SHA256"); + assertEquals("should return a valid PBKDF2 hash with the default value for \"iterations\"", pbkdf2Function.getIterations(), 600000); + assertEquals("should return a valid PBKDF2 hash with the default value for \"length\"", pbkdf2Function.getLength(), 256); + } + + @Test + public void testWithPBKDF2FunctionArgument() { + Hasher.main(new String[] { "-p", "password", "-a", "PBKDF2", "-f", "SHA512" }); + CompressedPBKDF2Function pbkdf2Function = CompressedPBKDF2Function.getInstanceFromHash(out.toString()); + assertEquals("should return a valid PBKDF2 hash with the correct value for \"function\"", pbkdf2Function.getAlgorithm(), "SHA512"); + assertEquals("should return a valid PBKDF2 hash with the default value for \"iterations\"", pbkdf2Function.getIterations(), 600000); + assertEquals("should return a valid PBKDF2 hash with the default value for \"length\"", pbkdf2Function.getLength(), 256); + out.reset(); + + Hasher.main(new String[] { "-p", "password", "-a", "PBKDF2", "-f", "SHA384" }); + pbkdf2Function = CompressedPBKDF2Function.getInstanceFromHash(out.toString()); + assertEquals("should return a valid PBKDF2 hash with the correct value for \"function\"", pbkdf2Function.getAlgorithm(), "SHA384"); + assertEquals("should return a valid PBKDF2 hash with the default value for \"iterations\"", pbkdf2Function.getIterations(), 600000); + assertEquals("should return a valid PBKDF2 hash with the default value for \"length\"", pbkdf2Function.getLength(), 256); + } + + @Test + public void testWithPBKDF2IterationsArgument() { + Hasher.main(new String[] { "-p", "password", "-a", "PBKDF2", "-i", "100000" }); + CompressedPBKDF2Function pbkdf2Function = CompressedPBKDF2Function.getInstanceFromHash(out.toString()); + assertEquals("should return a valid PBKDF2 hash with the correct value for \"function\"", pbkdf2Function.getAlgorithm(), "SHA256"); + assertEquals("should return a valid PBKDF2 hash with the default value for \"iterations\"", pbkdf2Function.getIterations(), 100000); + assertEquals("should return a valid PBKDF2 hash with the default value for \"length\"", pbkdf2Function.getLength(), 256); + out.reset(); + + Hasher.main(new String[] { "-p", "password", "-a", "PBKDF2", "-i", "200000" }); + pbkdf2Function = CompressedPBKDF2Function.getInstanceFromHash(out.toString()); + assertEquals("should return a valid PBKDF2 hash with the correct value for \"function\"", pbkdf2Function.getAlgorithm(), "SHA256"); + assertEquals("should return a valid PBKDF2 hash with the default value for \"iterations\"", pbkdf2Function.getIterations(), 200000); + assertEquals("should return a valid PBKDF2 hash with the default value for \"length\"", pbkdf2Function.getLength(), 256); + } + + @Test + public void testWithPBKDF2LengthArgument() { + Hasher.main(new String[] { "-p", "password", "-a", "PBKDF2", "-l", "400" }); + CompressedPBKDF2Function pbkdf2Function = CompressedPBKDF2Function.getInstanceFromHash(out.toString()); + assertEquals("should return a valid PBKDF2 hash with the correct value for \"function\"", pbkdf2Function.getAlgorithm(), "SHA256"); + assertEquals("should return a valid PBKDF2 hash with the default value for \"iterations\"", pbkdf2Function.getIterations(), 600000); + assertEquals("should return a valid PBKDF2 hash with the default value for \"length\"", pbkdf2Function.getLength(), 400); + out.reset(); + + Hasher.main(new String[] { "-p", "password", "-a", "PBKDF2", "-l", "300" }); + pbkdf2Function = CompressedPBKDF2Function.getInstanceFromHash(out.toString()); + assertEquals("should return a valid PBKDF2 hash with the correct value for \"function\"", pbkdf2Function.getAlgorithm(), "SHA256"); + assertEquals("should return a valid PBKDF2 hash with the default value for \"iterations\"", pbkdf2Function.getIterations(), 600000); + assertEquals("should return a valid PBKDF2 hash with the default value for \"length\"", pbkdf2Function.getLength(), 300); + } + + @Test + public void testWithPBKDF2AllArguments() { + Hasher.main(new String[] { "-p", "password", "-a", "PBKDF2", "-l", "250", "-i", "150000", "-f", "SHA384" }); + CompressedPBKDF2Function pbkdf2Function = CompressedPBKDF2Function.getInstanceFromHash(out.toString()); + assertEquals("should return a valid PBKDF2 hash with the correct value for \"function\"", pbkdf2Function.getAlgorithm(), "SHA384"); + assertEquals("should return a valid PBKDF2 hash with the default value for \"iterations\"", pbkdf2Function.getIterations(), 150000); + assertEquals("should return a valid PBKDF2 hash with the default value for \"length\"", pbkdf2Function.getLength(), 250); + } +} diff --git a/src/test/resources/auditlog/internal_users_pbkdf2.yml b/src/test/resources/auditlog/internal_users_pbkdf2.yml new file mode 100644 index 0000000000..6f95dd107d --- /dev/null +++ b/src/test/resources/auditlog/internal_users_pbkdf2.yml @@ -0,0 +1,46 @@ +--- +_meta: + type: "internalusers" + config_version: 2 +admin: + hash: "$3$1331439861760512$wBFrJJIAokWuJxlO6BQPLashXgznvR4tRmXk3aEy9SpHWrb9kFjPPLByZZzMLBNQFjhepgbngYh7RfMh8TrPLw==$vqGlzGsxqGf9TgfxhORjdoqRFB3npvBd9B0GAtBMg9mD2zBbSTohRYlOxUL7UQLma66zZdD67c4RNE9BKelabw==" + reserved: false + hidden: false + backend_roles: [] + attributes: {} + description: "Hashed with PBKDF2 with standard configuration" +user1: + hash: "$1$4294967296128$VmnDMbQ4wLiFUq178RKvj+EYfdb3Q26qCiDcJDoCxpYnKpyuG0JhTC2wpUkMUveV5RmBFzldKQkdqZEfE0XAgg==$9u3aMWc6VP2oGkXei7UaXA==" + reserved: false + hidden: false + backend_roles: [] + attributes: {} + description: "Hashed with PBKDF2 with: SHA1 function | 1000 iterations | 128 length" +user2: + hash: "$2$214748364800224$eQgqv2RI6yo95yeVnM5sfwUCwxHo6re0w+wpx6ZqZtHQV+dzlyP6YFitjNG2mlaTkg0pR56xArQaAgapdVcBQQ==$tGHWhoc83cd5nZ7QIZKPORjW/N5jklhYhRgXpw==" + reserved: false + hidden: false + backend_roles: [] + attributes: {} + description: "Hashed with PBKDF2 with: SHA224 function | 50000 iterations | 224 length" +user3: + hash: "$3$322122547200256$5b3wEAsMc05EZFxfncCUfZRERgvbwlBhYXd5vVR14kNJtmhXSpYMzydRZxO9096IPTkc47doH4hIdKX6LTguLg==$oQQvAtUyOC6cwdAYi5WeIM7rGUN9l3IdJ9y2RNxZCWo=" + reserved: false + hidden: false + backend_roles: [] + attributes: {} + description: "Hashed with PBKDF2 with: SHA256 function | 75000 iterations | 256 length" +user4: + hash: "$4$429496729600384$+SNSgbZD67a1bd92iuEiHCq5pvvrCx3HrNIf5hbGIJdxgXegpWilpB6vUGvYigegAUzZqE9iIsL4pSJztUNJYw==$lTxZ7tax6dBQ0r4qPJpc8d7YuoTBiUujY9HJeAZvARXMjIgvnJwa6FeYugttOKc0" + reserved: false + hidden: false + backend_roles: [] + attributes: {} + description: "Hashed with PBKDF2 with: SHA384 function | 100000 iterations | 384 length" +user5: + hash: "$5$644245094400512$HQe/MOv/NAlgodNhqTmjqj5jGxBwG5xuRaxKwn7r4nlUba1kj/CYnpdFaXGvVeRxt2NLm8fbekS6NYonv358Ew==$1sDx+0tMbtGzU6jlQg/Dyt30Yxuy5RdNmP9B1EzMTxYWi8k1xg2gXLy7w1XbetEC8UD/lpyXJPlaoxXpsaADyA==" + reserved: false + hidden: false + backend_roles: [] + attributes: {} + description: "Hashed with PBKDF2 with: SHA512 function | 150000 iterations | 512 length"