Skip to content

Commit 7d0cb66

Browse files
Dan CecoiDan Cecoi
Dan Cecoi
authored and
Dan Cecoi
committed
Add support for PBKDF2, Scrypt, and Argon2 for password hashing (opensearch-project#4079)
Signed-off-by: Dan Cecoi <[email protected]>
1 parent 9a85f23 commit 7d0cb66

17 files changed

+378
-58
lines changed

build.gradle

+2
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,8 @@ dependencies {
581581
implementation 'org.ldaptive:ldaptive:1.2.3'
582582
implementation 'com.nimbusds:nimbus-jose-jwt:9.37.3'
583583

584+
implementation 'com.password4j:password4j:1.7.3'
585+
584586
//JWT
585587
implementation "io.jsonwebtoken:jjwt-api:${jjwt_version}"
586588
implementation "io.jsonwebtoken:jjwt-impl:${jjwt_version}"

plugin-security.policy

+3
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ grant {
7777
//Enable this permission to debug unauthorized de-serialization attempt
7878
//permission java.io.SerializablePermission "enableSubstitution";
7979

80+
81+
permission java.io.FilePermission "/psw4j.properties","read";
82+
8083
};
8184

8285
grant codeBase "${codebase.netty-common}" {

src/integrationTest/java/org/opensearch/security/api/AccountRestApiIntegrationTest.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,17 @@
1313
import org.junit.Test;
1414

1515
import org.opensearch.core.xcontent.ToXContentObject;
16+
import org.opensearch.security.hash.PasswordHash;
17+
import org.opensearch.security.hash.PasswordHashImpl;
1618
import org.opensearch.test.framework.TestSecurityConfig;
1719
import org.opensearch.test.framework.cluster.TestRestClient;
1820

21+
import java.nio.CharBuffer;
22+
1923
import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic;
2024
import static org.hamcrest.CoreMatchers.is;
2125
import static org.hamcrest.MatcherAssert.assertThat;
2226
import static org.hamcrest.Matchers.not;
23-
import static org.opensearch.security.dlic.rest.support.Utils.hash;
2427

2528
public class AccountRestApiIntegrationTest extends AbstractApiIntegrationTest {
2629

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

107110
private void verifyPasswordCanBeChanged() throws Exception {
108111
final var newPassword = randomAlphabetic(10);
112+
final PasswordHash passwordService = new PasswordHashImpl();
109113
withUser(
110114
TEST_USER,
111115
TEST_USER_PASSWORD,
112116
client -> ok(
113-
() -> client.putJson(accountPath(), changePasswordWithHashPayload(TEST_USER_PASSWORD, hash(newPassword.toCharArray())))
117+
() -> client.putJson(accountPath(), changePasswordWithHashPayload(TEST_USER_PASSWORD,
118+
passwordService.hash(CharBuffer.wrap(newPassword.toCharArray()))))
114119
)
115120
);
116121
withUser(

src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java

+11-10
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@
3131
import java.io.IOException;
3232
import java.io.UnsupportedEncodingException;
3333
import java.nio.ByteBuffer;
34-
import java.security.SecureRandom;
34+
import java.nio.CharBuffer;
35+
3536
import java.util.ArrayList;
3637
import java.util.Arrays;
3738
import java.util.Collections;
@@ -46,7 +47,6 @@
4647

4748
import org.apache.logging.log4j.LogManager;
4849
import org.apache.logging.log4j.Logger;
49-
import org.bouncycastle.crypto.generators.OpenBSDBCrypt;
5050

5151
import org.opensearch.action.admin.indices.create.CreateIndexRequest;
5252
import org.opensearch.action.index.IndexRequest;
@@ -57,6 +57,8 @@
5757
import org.opensearch.core.common.bytes.BytesReference;
5858
import org.opensearch.core.xcontent.ToXContentObject;
5959
import org.opensearch.core.xcontent.XContentBuilder;
60+
import org.opensearch.security.hash.PasswordHash;
61+
import org.opensearch.security.hash.PasswordHashImpl;
6062
import org.opensearch.security.securityconf.impl.CType;
6163
import org.opensearch.test.framework.cluster.OpenSearchClientProvider.UserCredentialsHolder;
6264

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

454-
xContentBuilder.field("hash", hash(password.toCharArray()));
456+
xContentBuilder.field("hash", hash(password));
455457

456458
Set<String> roleNames = getRoleNames();
457459

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

936-
static String hash(final char[] clearTextPassword) {
937-
final byte[] salt = new byte[16];
938-
new SecureRandom().nextBytes(salt);
939-
final String hash = OpenBSDBCrypt.generate((Objects.requireNonNull(clearTextPassword)), salt, 12);
940-
Arrays.fill(salt, (byte) 0);
941-
Arrays.fill(clearTextPassword, '\0');
942-
return hash;
938+
static String hash(final String clearTextPassword) {
939+
PasswordHash passwordService = new PasswordHashImpl();
940+
return passwordService.hash((
941+
Objects.requireNonNull(
942+
CharBuffer.wrap(clearTextPassword.toCharArray())
943+
)));
943944
}
944945

945946
private void writeEmptyConfigToIndex(Client client, CType configType) {

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

+134
Original file line numberDiff line numberDiff line change
@@ -1916,6 +1916,140 @@ public List<Setting<?>> getSettings() {
19161916
);
19171917
}
19181918

1919+
1920+
//todo: What is Property.Filtered?
1921+
//todo: Do we want these properties to be Final?
1922+
settings.add(
1923+
Setting.simpleString(
1924+
ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM,
1925+
ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM_DEFAULT,
1926+
Property.NodeScope,
1927+
Property.Filtered,
1928+
Property.Final
1929+
)
1930+
);
1931+
1932+
settings.add(Setting.intSetting(
1933+
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS,
1934+
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ITERATIONS_DEFAULT,
1935+
Property.NodeScope,
1936+
Property.Filtered,
1937+
Property.Final
1938+
));
1939+
1940+
settings.add(Setting.intSetting(
1941+
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH,
1942+
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_LENGTH_DEFAULT,
1943+
Property.NodeScope,
1944+
Property.Filtered,
1945+
Property.Final
1946+
));
1947+
1948+
settings.add(Setting.simpleString(
1949+
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM,
1950+
ConfigConstants.SECURITY_PASSWORD_HASHING_PBKDF2_ALGORITHM_DEFAULT,
1951+
Property.NodeScope,
1952+
Property.Filtered,
1953+
Property.Final
1954+
));
1955+
1956+
settings.add(Setting.intSetting(
1957+
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR,
1958+
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_WORK_FACTOR_DEFAULT,
1959+
Property.NodeScope,
1960+
Property.Filtered,
1961+
Property.Final
1962+
));
1963+
1964+
settings.add(Setting.intSetting(
1965+
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES,
1966+
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_RESOURCES_DEFAULT,
1967+
Property.NodeScope,
1968+
Property.Filtered,
1969+
Property.Final
1970+
));
1971+
1972+
settings.add(Setting.intSetting(
1973+
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION,
1974+
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_PARALLELIZATION_DEFAULT,
1975+
Property.NodeScope,
1976+
Property.Filtered,
1977+
Property.Final
1978+
));
1979+
1980+
settings.add(Setting.intSetting(
1981+
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH,
1982+
ConfigConstants.SECURITY_PASSWORD_HASHING_SCRYPT_DERIVED_KEY_LENGTH_DEFAULT,
1983+
Property.NodeScope,
1984+
Property.Filtered,
1985+
Property.Final
1986+
));
1987+
1988+
settings.add(Setting.intSetting(
1989+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_MEMORY,
1990+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_MEMORY_DEFAULT,
1991+
Property.NodeScope,
1992+
Property.Filtered,
1993+
Property.Final
1994+
));
1995+
1996+
settings.add(Setting.intSetting(
1997+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS,
1998+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_ITERATIONS_DEFAULT,
1999+
Property.NodeScope,
2000+
Property.Filtered,
2001+
Property.Final
2002+
));
2003+
2004+
settings.add(Setting.intSetting(
2005+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_LENGTH,
2006+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_LENGTH_DEFAULT,
2007+
Property.NodeScope,
2008+
Property.Filtered,
2009+
Property.Final
2010+
));
2011+
2012+
settings.add(Setting.intSetting(
2013+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_PARALLELISM,
2014+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_PARALLELISM_DEFAULT,
2015+
Property.NodeScope,
2016+
Property.Filtered,
2017+
Property.Final
2018+
));
2019+
2020+
settings.add(Setting.simpleString(
2021+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_TYPE,
2022+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_TYPE_DEFAULT,
2023+
Property.NodeScope,
2024+
Property.Filtered,
2025+
Property.Final
2026+
));
2027+
2028+
settings.add(Setting.intSetting(
2029+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_VERSION,
2030+
ConfigConstants.SECURITY_PASSWORD_HASHING_ARGON2_VERSION_DEFAULT,
2031+
Property.NodeScope,
2032+
Property.Filtered,
2033+
Property.Final
2034+
));
2035+
2036+
2037+
settings.add(Setting.intSetting(
2038+
ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS,
2039+
ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_ROUNDS_DEFAULT,
2040+
Property.NodeScope,
2041+
Property.Filtered,
2042+
Property.Final
2043+
));
2044+
2045+
settings.add(Setting.simpleString(
2046+
ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR,
2047+
ConfigConstants.SECURITY_PASSWORD_HASHING_BCRYPT_MINOR_DEFAULT,
2048+
Property.NodeScope,
2049+
Property.Filtered,
2050+
Property.Final
2051+
));
2052+
19192053
return settings;
19202054
}
19212055

src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

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

14+
import java.nio.CharBuffer;
1415
import java.util.List;
1516
import java.util.Map;
1617
import java.util.Set;
@@ -19,8 +20,6 @@
1920
import com.google.common.collect.ImmutableMap;
2021
import com.google.common.collect.ImmutableSet;
2122
import org.apache.commons.lang3.tuple.Triple;
22-
import org.bouncycastle.crypto.generators.OpenBSDBCrypt;
23-
2423
import org.opensearch.cluster.service.ClusterService;
2524
import org.opensearch.common.settings.Settings;
2625
import org.opensearch.core.common.Strings;
@@ -32,6 +31,8 @@
3231
import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
3332
import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType;
3433
import org.opensearch.security.dlic.rest.validation.ValidationResult;
34+
import org.opensearch.security.hash.PasswordHash;
35+
import org.opensearch.security.hash.PasswordHashImpl;
3536
import org.opensearch.security.securityconf.Hashed;
3637
import org.opensearch.security.securityconf.impl.CType;
3738
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
@@ -44,13 +45,15 @@
4445
import static org.opensearch.security.dlic.rest.api.Responses.ok;
4546
import static org.opensearch.security.dlic.rest.api.Responses.response;
4647
import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
47-
import static org.opensearch.security.dlic.rest.support.Utils.hash;
4848

4949
/**
5050
* Rest API action to fetch or update account details of the signed-in user.
5151
* Currently this action serves GET and PUT request for /_opendistro/_security/api/account endpoint
5252
*/
5353
public class AccountApiAction extends AbstractApiAction {
54+
55+
private final PasswordHash passwordService;
56+
5457
private static final List<Route> routes = addRoutesPrefix(
5558
ImmutableList.of(new Route(Method.GET, "/account"), new Route(Method.PUT, "/account"))
5659
);
@@ -62,6 +65,7 @@ public AccountApiAction(
6265
) {
6366
super(Endpoint.ACCOUNT, clusterService, threadPool, securityApiDependencies);
6467
this.requestHandlersBuilder.configureRequestHandlers(this::accountApiRequestHandlers);
68+
this.passwordService = new PasswordHashImpl(securityApiDependencies.settings());
6569
}
6670

6771
@Override
@@ -132,7 +136,7 @@ ValidationResult<SecurityConfiguration> validCurrentPassword(final SecurityConfi
132136
final var currentPassword = content.get("current_password").asText();
133137
final var internalUserEntry = (Hashed) securityConfiguration.configuration().getCEntry(username);
134138
final var currentHash = internalUserEntry.getHash();
135-
if (currentHash == null || !OpenBSDBCrypt.checkPassword(currentHash, currentPassword.toCharArray())) {
139+
if (currentHash == null || !passwordService.check(CharBuffer.wrap(currentPassword.toCharArray()), currentHash)) {
136140
return ValidationResult.error(RestStatus.BAD_REQUEST, badRequestMessage("Could not validate your current password."));
137141
}
138142
return ValidationResult.success(securityConfiguration);
@@ -148,7 +152,7 @@ ValidationResult<SecurityConfiguration> updatePassword(final SecurityConfigurati
148152
if (Strings.isNullOrEmpty(password)) {
149153
hash = securityJsonNode.get("hash").asString();
150154
} else {
151-
hash = hash(password.toCharArray());
155+
hash = this.passwordService.hash(CharBuffer.wrap(password.toCharArray()));
152156
}
153157
if (Strings.isNullOrEmpty(hash)) {
154158
return ValidationResult.error(

src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
package org.opensearch.security.dlic.rest.api;
1313

1414
import java.io.IOException;
15+
import java.nio.CharBuffer;
1516
import java.util.List;
1617
import java.util.Map;
1718

@@ -31,6 +32,8 @@
3132
import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
3233
import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType;
3334
import org.opensearch.security.dlic.rest.validation.ValidationResult;
35+
import org.opensearch.security.hash.PasswordHash;
36+
import org.opensearch.security.hash.PasswordHashImpl;
3437
import org.opensearch.security.securityconf.Hashed;
3538
import org.opensearch.security.securityconf.impl.CType;
3639
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
@@ -47,7 +50,6 @@
4750
import static org.opensearch.security.dlic.rest.api.Responses.payload;
4851
import static org.opensearch.security.dlic.rest.api.Responses.response;
4952
import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
50-
import static org.opensearch.security.dlic.rest.support.Utils.hash;
5153

5254
public class InternalUsersApiAction extends AbstractApiAction {
5355

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

62+
private final PasswordHash passwordService;
63+
6064
static final List<String> RESTRICTED_FROM_USERNAME = ImmutableList.of(
6165
":" // Not allowed in basic auth, see https://stackoverflow.com/a/33391003/533057
6266
);
@@ -92,6 +96,7 @@ public InternalUsersApiAction(
9296
super(Endpoint.INTERNALUSERS, clusterService, threadPool, securityApiDependencies);
9397
this.userService = userService;
9498
this.requestHandlersBuilder.configureRequestHandlers(this::internalUsersApiRequestHandlers);
99+
this.passwordService = new PasswordHashImpl(securityApiDependencies.settings());
95100
}
96101

97102
@Override
@@ -268,7 +273,7 @@ private ValidationResult<SecurityConfiguration> generateHashForPassword(final Se
268273
if (content.has("password")) {
269274
final var plainTextPassword = content.get("password").asText();
270275
content.remove("password");
271-
content.put("hash", hash(plainTextPassword.toCharArray()));
276+
content.put("hash", passwordService.hash(CharBuffer.wrap(plainTextPassword.toCharArray())));
272277
}
273278
return ValidationResult.success(securityConfiguration);
274279
}

0 commit comments

Comments
 (0)