Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support password_hash for Change Password API #75500

Merged
merged 9 commits into from
Aug 10, 2021
Merged
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 @@ -243,7 +243,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 @@ -27,7 +27,6 @@
import org.elasticsearch.xpack.security.rest.action.SecurityBaseRestHandler;

import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Set;

Expand Down Expand Up @@ -88,7 +87,7 @@ public RestResponse buildResponse(ActionResponse.Empty response, XContentBuilder
});
}

private static final Set<String> FILTERED_FIELDS = Collections.singleton("password");
private static final Set<String> FILTERED_FIELDS = Set.of("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"
}
jkakavas marked this conversation as resolved.
Show resolved Hide resolved

# login with original credentials
- do:
headers:
Authorization: "Basic am9lOnMza3JpdC1wYXNzd29yZA=="
cluster.health: {}
- match: { timed_out: false }

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