Skip to content

Commit

Permalink
Add support for PBKDF2, Scrypt, and Argon2 for password hashing (open…
Browse files Browse the repository at this point in the history
…search-project#4079)

Signed-off-by: Dan Cecoi <[email protected]>
  • Loading branch information
Dan Cecoi authored and Dan Cecoi committed Apr 22, 2024
1 parent 9a85f23 commit 7d0cb66
Show file tree
Hide file tree
Showing 17 changed files with 378 additions and 58 deletions.
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,8 @@ dependencies {
implementation 'org.ldaptive:ldaptive:1.2.3'
implementation 'com.nimbusds:nimbus-jose-jwt:9.37.3'

implementation 'com.password4j:password4j:1.7.3'

//JWT
implementation "io.jsonwebtoken:jjwt-api:${jjwt_version}"
implementation "io.jsonwebtoken:jjwt-impl:${jjwt_version}"
Expand Down
3 changes: 3 additions & 0 deletions plugin-security.policy
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ grant {
//Enable this permission to debug unauthorized de-serialization attempt
//permission java.io.SerializablePermission "enableSubstitution";


permission java.io.FilePermission "/psw4j.properties","read";

};

grant codeBase "${codebase.netty-common}" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,17 @@
import org.junit.Test;

import org.opensearch.core.xcontent.ToXContentObject;
import org.opensearch.security.hash.PasswordHash;
import org.opensearch.security.hash.PasswordHashImpl;
import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.cluster.TestRestClient;

import java.nio.CharBuffer;

import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.not;
import static org.opensearch.security.dlic.rest.support.Utils.hash;

public class AccountRestApiIntegrationTest extends AbstractApiIntegrationTest {

Expand Down Expand Up @@ -106,11 +109,13 @@ private void verifyWrongPayload(final TestRestClient client) throws Exception {

private void verifyPasswordCanBeChanged() throws Exception {
final var newPassword = randomAlphabetic(10);
final PasswordHash passwordService = new PasswordHashImpl();
withUser(
TEST_USER,
TEST_USER_PASSWORD,
client -> ok(
() -> client.putJson(accountPath(), changePasswordWithHashPayload(TEST_USER_PASSWORD, hash(newPassword.toCharArray())))
() -> client.putJson(accountPath(), changePasswordWithHashPayload(TEST_USER_PASSWORD,
passwordService.hash(CharBuffer.wrap(newPassword.toCharArray()))))
)
);
withUser(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.nio.ByteBuffer;
import java.security.SecureRandom;
import java.nio.CharBuffer;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -46,7 +47,6 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.bouncycastle.crypto.generators.OpenBSDBCrypt;

import org.opensearch.action.admin.indices.create.CreateIndexRequest;
import org.opensearch.action.index.IndexRequest;
Expand All @@ -57,6 +57,8 @@
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.xcontent.ToXContentObject;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.security.hash.PasswordHash;
import org.opensearch.security.hash.PasswordHashImpl;
import org.opensearch.security.securityconf.impl.CType;
import org.opensearch.test.framework.cluster.OpenSearchClientProvider.UserCredentialsHolder;

Expand Down Expand Up @@ -451,7 +453,7 @@ public Object getAttribute(String attributeName) {
public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException {
xContentBuilder.startObject();

xContentBuilder.field("hash", hash(password.toCharArray()));
xContentBuilder.field("hash", hash(password));

Set<String> roleNames = getRoleNames();

Expand Down Expand Up @@ -933,13 +935,12 @@ public void updateInternalUsersConfiguration(Client client, List<User> users) {
updateConfigInIndex(client, CType.INTERNALUSERS, userMap);
}

static String hash(final char[] clearTextPassword) {
final byte[] salt = new byte[16];
new SecureRandom().nextBytes(salt);
final String hash = OpenBSDBCrypt.generate((Objects.requireNonNull(clearTextPassword)), salt, 12);
Arrays.fill(salt, (byte) 0);
Arrays.fill(clearTextPassword, '\0');
return hash;
static String hash(final String clearTextPassword) {
PasswordHash passwordService = new PasswordHashImpl();
return passwordService.hash((
Objects.requireNonNull(
CharBuffer.wrap(clearTextPassword.toCharArray())
)));
}

private void writeEmptyConfigToIndex(Client client, CType configType) {
Expand Down
134 changes: 134 additions & 0 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -1916,6 +1916,140 @@ public List<Setting<?>> getSettings() {
);
}


//todo: What is Property.Filtered?
//todo: Do we want these properties to be Final?
settings.add(
Setting.simpleString(
ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM,
ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
)
);

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS,
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH,
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.simpleString(
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM,
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR,
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES,
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION,
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH,
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_MEMORY,
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_MEMORY_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS,
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_LENGTH,
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_LENGTH_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_PARALLELISM,
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_PARALLELISM_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.simpleString(
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_TYPE,
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_TYPE_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_VERSION,
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_VERSION_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));


settings.add(Setting.intSetting(
ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS,
ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

settings.add(Setting.simpleString(
ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR,
ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR_DEFAULT,
Property.NodeScope,
Property.Filtered,
Property.Final
));

return settings;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

package org.opensearch.security.dlic.rest.api;

import java.nio.CharBuffer;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand All @@ -19,8 +20,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import org.apache.commons.lang3.tuple.Triple;
import org.bouncycastle.crypto.generators.OpenBSDBCrypt;

import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.common.Strings;
Expand All @@ -32,6 +31,8 @@
import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType;
import org.opensearch.security.dlic.rest.validation.ValidationResult;
import org.opensearch.security.hash.PasswordHash;
import org.opensearch.security.hash.PasswordHashImpl;
import org.opensearch.security.securityconf.Hashed;
import org.opensearch.security.securityconf.impl.CType;
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
Expand All @@ -44,13 +45,15 @@
import static org.opensearch.security.dlic.rest.api.Responses.ok;
import static org.opensearch.security.dlic.rest.api.Responses.response;
import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
import static org.opensearch.security.dlic.rest.support.Utils.hash;

/**
* Rest API action to fetch or update account details of the signed-in user.
* Currently this action serves GET and PUT request for /_opendistro/_security/api/account endpoint
*/
public class AccountApiAction extends AbstractApiAction {

private final PasswordHash passwordService;

private static final List<Route> routes = addRoutesPrefix(
ImmutableList.of(new Route(Method.GET, "/account"), new Route(Method.PUT, "/account"))
);
Expand All @@ -62,6 +65,7 @@ public AccountApiAction(
) {
super(Endpoint.ACCOUNT, clusterService, threadPool, securityApiDependencies);
this.requestHandlersBuilder.configureRequestHandlers(this::accountApiRequestHandlers);
this.passwordService = new PasswordHashImpl(securityApiDependencies.settings());
}

@Override
Expand Down Expand Up @@ -132,7 +136,7 @@ ValidationResult<SecurityConfiguration> validCurrentPassword(final SecurityConfi
final var currentPassword = content.get("current_password").asText();
final var internalUserEntry = (Hashed) securityConfiguration.configuration().getCEntry(username);
final var currentHash = internalUserEntry.getHash();
if (currentHash == null || !OpenBSDBCrypt.checkPassword(currentHash, currentPassword.toCharArray())) {
if (currentHash == null || !passwordService.check(CharBuffer.wrap(currentPassword.toCharArray()), currentHash)) {
return ValidationResult.error(RestStatus.BAD_REQUEST, badRequestMessage("Could not validate your current password."));
}
return ValidationResult.success(securityConfiguration);
Expand All @@ -148,7 +152,7 @@ ValidationResult<SecurityConfiguration> updatePassword(final SecurityConfigurati
if (Strings.isNullOrEmpty(password)) {
hash = securityJsonNode.get("hash").asString();
} else {
hash = hash(password.toCharArray());
hash = this.passwordService.hash(CharBuffer.wrap(password.toCharArray()));
}
if (Strings.isNullOrEmpty(hash)) {
return ValidationResult.error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
package org.opensearch.security.dlic.rest.api;

import java.io.IOException;
import java.nio.CharBuffer;
import java.util.List;
import java.util.Map;

Expand All @@ -31,6 +32,8 @@
import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType;
import org.opensearch.security.dlic.rest.validation.ValidationResult;
import org.opensearch.security.hash.PasswordHash;
import org.opensearch.security.hash.PasswordHashImpl;
import org.opensearch.security.securityconf.Hashed;
import org.opensearch.security.securityconf.impl.CType;
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
Expand All @@ -47,7 +50,6 @@
import static org.opensearch.security.dlic.rest.api.Responses.payload;
import static org.opensearch.security.dlic.rest.api.Responses.response;
import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
import static org.opensearch.security.dlic.rest.support.Utils.hash;

public class InternalUsersApiAction extends AbstractApiAction {

Expand All @@ -57,6 +59,8 @@ protected void consumeParameters(final RestRequest request) {
request.param("filterBy");
}

private final PasswordHash passwordService;

static final List<String> RESTRICTED_FROM_USERNAME = ImmutableList.of(
":" // Not allowed in basic auth, see https://stackoverflow.com/a/33391003/533057
);
Expand Down Expand Up @@ -92,6 +96,7 @@ public InternalUsersApiAction(
super(Endpoint.INTERNALUSERS, clusterService, threadPool, securityApiDependencies);
this.userService = userService;
this.requestHandlersBuilder.configureRequestHandlers(this::internalUsersApiRequestHandlers);
this.passwordService = new PasswordHashImpl(securityApiDependencies.settings());
}

@Override
Expand Down Expand Up @@ -268,7 +273,7 @@ private ValidationResult<SecurityConfiguration> generateHashForPassword(final Se
if (content.has("password")) {
final var plainTextPassword = content.get("password").asText();
content.remove("password");
content.put("hash", hash(plainTextPassword.toCharArray()));
content.put("hash", passwordService.hash(CharBuffer.wrap(plainTextPassword.toCharArray())));
}
return ValidationResult.success(securityConfiguration);
}
Expand Down
Loading

0 comments on commit 7d0cb66

Please sign in to comment.