diff --git a/x-pack/docs/en/rest-api/security/change-password.asciidoc b/x-pack/docs/en/rest-api/security/change-password.asciidoc index bb2e9bf449ecd..c035661cdd707 100644 --- a/x-pack/docs/en/rest-api/security/change-password.asciidoc +++ b/x-pack/docs/en/rest-api/security/change-password.asciidoc @@ -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 +<>. ++ +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]] diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/ChangePasswordRequestBuilder.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/ChangePasswordRequestBuilder.java index 8ea9ec705b10c..5eff333d070f7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/ChangePasswordRequestBuilder.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/ChangePasswordRequestBuilder.java @@ -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); } @@ -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 */ @@ -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); @@ -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; + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStore.java index 0d9c68f32d62b..2e0f5b192dd04 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStore.java @@ -243,7 +243,8 @@ public void changePassword(final ChangePasswordRequest request, final ActionList new ActionListener() { @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); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestChangePasswordAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestChangePasswordAction.java index abc71a8b6dfdc..7312257e4ce53 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestChangePasswordAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestChangePasswordAction.java @@ -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; @@ -88,7 +87,7 @@ public RestResponse buildResponse(ActionResponse.Empty response, XContentBuilder }); } - private static final Set FILTERED_FIELDS = Collections.singleton("password"); + private static final Set FILTERED_FIELDS = Set.of("password", "password_hash"); @Override public Set getFilteredFields() { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/ChangePasswordRequestBuilderTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/ChangePasswordRequestBuilderTests.java new file mode 100644 index 0000000000000..79a2540d475e8 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/ChangePasswordRequestBuilderTests.java @@ -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 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")); + + } +} diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/change_password/10_basic.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/change_password/10_basic.yml index 022ec669d670f..8f416bb029015 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/change_password/10_basic.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/change_password/10_basic.yml @@ -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: