Skip to content

Commit

Permalink
[7.x] Support password_hash for Change Password API (#75500) (#76275)
Browse files Browse the repository at this point in the history
This change adds support for a password_hash field in the request
body of the Change Password API. Clients can use this to pre-hash
the password when making a change password request.
  • Loading branch information
elasticsearchmachine authored Aug 10, 2021
1 parent 0cad5fb commit 7b08bfa
Show file tree
Hide file tree
Showing 6 changed files with 214 additions and 7 deletions.
19 changes: 17 additions & 2 deletions x-pack/docs/en/rest-api/security/change-password.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,23 @@ For more information about the native realm, see
[[security-api-change-password-request-body]]
==== {api-request-body-title}

`password`::
(Required, string) The new password value.
`password` ::
(string) The new password value. Passwords must be at least 6 characters long.
+
One of `password` or `password_hash` is required.

`password_hash` ::
(string) A _hash_ of the new password value. This must be produced using the
same hashing algorithm as has been configured for password storage. For more
details, see the explanation of the
`xpack.security.authc.password_hashing.algorithm` setting in
<<hashing-settings>>.
+
Using this parameter allows the client to pre-hash the password for
performance and/or confidentiality reasons.
+
The `password` parameter and the `password_hash` parameter cannot be
used in the same request.


[[security-api-change-password-example]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ public ChangePasswordRequestBuilder username(String username) {
public static char[] validateAndHashPassword(SecureString password, Hasher hasher) {
Validation.Error error = Validation.Users.validatePassword(password);
if (error != null) {
ValidationException validationException = new ValidationException();
validationException.addValidationError(error.toString());
throw validationException;
throw validationException(error.toString());
}
return hasher.hash(password);
}
Expand All @@ -59,11 +57,30 @@ public static char[] validateAndHashPassword(SecureString password, Hasher hashe
public ChangePasswordRequestBuilder password(char[] password, Hasher hasher) {
try (SecureString secureString = new SecureString(password)) {
char[] hash = validateAndHashPassword(secureString, hasher);
if (request.passwordHash() != null) {
throw validationException("password_hash has already been set");
}
request.passwordHash(hash);
}
return this;
}

/**
* Sets the password hash.
*/
public ChangePasswordRequestBuilder passwordHash(char[] passwordHashChars, Hasher configuredHasher) {
final Hasher resolvedHasher = Hasher.resolveFromHash(passwordHashChars);
if (resolvedHasher.equals(configuredHasher) == false) {
throw new IllegalArgumentException("Provided password hash uses [" + resolvedHasher
+ "] but the configured hashing algorithm is [" + configuredHasher + "]");
}
if (request.passwordHash() != null) {
throw validationException("password_hash has already been set");
}
request.passwordHash(passwordHashChars);
return this;
}

/**
* Populate the change password request from the source in the provided content type
*/
Expand All @@ -90,6 +107,14 @@ public ChangePasswordRequestBuilder source(BytesReference source, XContentType x
throw new ElasticsearchParseException(
"expected field [{}] to be of type string, but found [{}] instead", currentFieldName, token);
}
} else if (User.Fields.PASSWORD_HASH.match(currentFieldName, parser.getDeprecationHandler())) {
if (token == XContentParser.Token.VALUE_STRING) {
char[] passwordHashChars = parser.text().toCharArray();
passwordHash(passwordHashChars, hasher);
} else {
throw new ElasticsearchParseException(
"expected field [{}] to be of type string, but found [{}] instead", currentFieldName, token);
}
} else {
throw new ElasticsearchParseException("failed to parse change password request. unexpected field [{}]",
currentFieldName);
Expand All @@ -98,4 +123,10 @@ public ChangePasswordRequestBuilder source(BytesReference source, XContentType x
}
return this;
}

private static ValidationException validationException(String message) {
ValidationException validationException = new ValidationException();
validationException.addValidationError(message);
return validationException;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ public void changePassword(final ChangePasswordRequest request, final ActionList
new ActionListener<UpdateResponse>() {
@Override
public void onResponse(UpdateResponse updateResponse) {
assert updateResponse.getResult() == DocWriteResponse.Result.UPDATED;
assert updateResponse.getResult() == DocWriteResponse.Result.UPDATED
|| updateResponse.getResult() == DocWriteResponse.Result.NOOP;
clearRealmCache(request.username(), listener, null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand Down Expand Up @@ -89,7 +90,8 @@ public RestResponse buildResponse(ActionResponse.Empty changePasswordResponse,
});
}

private static final Set<String> FILTERED_FIELDS = Collections.singleton("password");
private static final Set<String> FILTERED_FIELDS = Collections.unmodifiableSet(
Sets.newHashSet("password", "password_hash"));

@Override
public Set<String> getFilteredFields() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.security.action.user;

import org.elasticsearch.client.Client;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.security.action.user.ChangePasswordRequest;
import org.elasticsearch.xpack.core.security.action.user.ChangePasswordRequestBuilder;
import org.elasticsearch.xpack.core.security.authc.support.Hasher;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.LinkedHashMap;

import static org.elasticsearch.test.SecurityIntegTestCase.getFastStoredHashAlgoForTests;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Mockito.mock;

public class ChangePasswordRequestBuilderTests extends ESTestCase {

public void testWithCleartextPassword() throws IOException {
final Hasher hasher = getFastStoredHashAlgoForTests();
final String json = "{\n" +
" \"password\": \"superlongpassword\"" +
"}";
ChangePasswordRequestBuilder builder = new ChangePasswordRequestBuilder(mock(Client.class));
ChangePasswordRequest request = builder.source(
new BytesArray(json.getBytes(StandardCharsets.UTF_8)), XContentType.JSON, hasher).request();
assertThat(hasher.verify(new SecureString("superlongpassword".toCharArray()), request.passwordHash()), equalTo(true));
}

public void testWithHashedPassword() throws IOException {
final Hasher hasher = getFastStoredHashAlgoForTests();
final char[] hash = hasher.hash(new SecureString("superlongpassword".toCharArray()));
final String json = "{\n" +
" \"password_hash\": \"" + new String(hash) + "\"" +
"}";
ChangePasswordRequestBuilder builder = new ChangePasswordRequestBuilder(mock(Client.class));
ChangePasswordRequest request = builder.source(
new BytesArray(json.getBytes(StandardCharsets.UTF_8)), XContentType.JSON, hasher).request();
assertThat(request.passwordHash(), equalTo(hash));
}

public void testWithHashedPasswordWithWrongAlgo() {
final Hasher systemHasher = getFastStoredHashAlgoForTests();
Hasher userHasher = getFastStoredHashAlgoForTests();
while (userHasher.name().equals(systemHasher.name())){
userHasher = getFastStoredHashAlgoForTests();
}
final char[] hash = userHasher.hash(new SecureString("superlongpassword".toCharArray()));
final String json = "{\n" +
" \"password_hash\": \"" + new String(hash) + "\"" +
"}";
ChangePasswordRequestBuilder builder = new ChangePasswordRequestBuilder(mock(Client.class));
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
builder.source(new BytesArray(json.getBytes(StandardCharsets.UTF_8)), XContentType.JSON, systemHasher).request();
});
assertThat(e.getMessage(), containsString(userHasher.name()));
assertThat(e.getMessage(), containsString(systemHasher.name()));
}

public void testWithHashedPasswordNotHash() {
final Hasher systemHasher = getFastStoredHashAlgoForTests();
final char[] hash = randomAlphaOfLength(20).toCharArray();
final String json = "{\n" +
" \"password_hash\": \"" + new String(hash) + "\"" +
"}";
ChangePasswordRequestBuilder builder = new ChangePasswordRequestBuilder(mock(Client.class));
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
builder.source(new BytesArray(json.getBytes(StandardCharsets.UTF_8)), XContentType.JSON, systemHasher).request();
});
assertThat(e.getMessage(), containsString(Hasher.NOOP.name()));
assertThat(e.getMessage(), containsString(systemHasher.name()));
}

public void testWithPasswordAndHash() throws IOException {
final Hasher hasher = getFastStoredHashAlgoForTests();
final String password = randomAlphaOfLength(14);
final char[] hash = hasher.hash(new SecureString(password.toCharArray()));
final LinkedHashMap<String, Object> fields = new LinkedHashMap<>();
fields.put("password", password);
fields.put("password_hash", new String(hash));
BytesReference json = BytesReference.bytes(XContentBuilder.builder(XContentType.JSON.xContent())
.map(shuffleMap(fields, Collections.emptySet())));

ChangePasswordRequestBuilder builder = new ChangePasswordRequestBuilder(mock(Client.class));
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
builder.source(json, XContentType.JSON, hasher).request();
});
assertThat(e.getMessage(), containsString("password_hash has already been set"));

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,59 @@ teardown:
cluster.health: {}
- match: { timed_out: false }

---
"Test changing users password with prehashed password":
- skip:
features: ["catch_unauthorized", "warnings"]
- do:
headers:
Authorization: "Basic am9lOnMza3JpdC1wYXNzd29yZA=="
cluster.health: {}
- match: { timed_out: false }

# We need this so that we can get hold of the password hash of the user without conditionals or multiple tests as the default hashing
# algorithm is different in FIPS mode. Additionally, for bcrypt, the stored hash string starts with a "$" and it would otherwise be
# interpreted as a stashed value, if we tried to use it as the value of "password_hash".
- do:
warnings:
- "this request accesses system indices: [.security-7], but in a future major version, direct access to system indices will be prevented by default"
get:
index: .security
id: user-joe
- set: { _source.password: "hash" }

# change password
- do:
security.change_password:
username: "joe"
body: >
{
"password" : "s3krit-password2"
}
# login with new credentials
- do:
headers:
Authorization: "Basic am9lOnMza3JpdC1wYXNzd29yZDI="
cluster.health: {}
- match: { timed_out: false }

# We change the password to the original one using the hash
- do:
security.change_password:
username: "joe"
body: >
{
"password_hash" : "$hash"
}
# login with original credentials
- do:
headers:
Authorization: "Basic am9lOnMza3JpdC1wYXNzd29yZA=="
cluster.health: {}
- match: { timed_out: false }

---
"Test user changing their own password":
- skip:
Expand Down

0 comments on commit 7b08bfa

Please sign in to comment.