honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Formal support for "password_hash" in Put User (#35242) 

For some time, the PutUser REST API has supported storing a pre-hashed
password for a user. The change adds validation and tests around that
feature so that it can be documented & officially supported.

It also prevents the request from containing both a "password" and a "password_hash".
This commit is contained in:
Tim Vernum 2018-11-14 16:46:31 +11:00 committed by GitHub
parent d8b1c23e1d
commit 231f6c1595
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 139 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/PutUserRequestBuilder.java
View File

@ -54,9 +54,10 @@ public class PutUserRequestBuilder extends ActionRequestBuilder<PutUserRequest,
if (password != null) {
Validation.Error error = Validation.Users.validatePassword(password);
if (error != null) {
ValidationException validationException = new ValidationException();
validationException.addValidationError(error.toString());
throw validationException;
throw validationException(error.toString());
}
if (request.passwordHash() != null) {
throw validationException("password_hash has already been set");
}
request.passwordHash(hasher.hash(new SecureString(password)));
} else {
@ -80,7 +81,15 @@ public class PutUserRequestBuilder extends ActionRequestBuilder<PutUserRequest,
return this;
}
public PutUserRequestBuilder passwordHash(char[] passwordHash) {
public PutUserRequestBuilder passwordHash(char[] passwordHash, Hasher configuredHasher) {
final Hasher resolvedHasher = Hasher.resolveFromHash(passwordHash);
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(passwordHash);
return this;
}
@ -120,7 +129,7 @@ public class PutUserRequestBuilder extends ActionRequestBuilder<PutUserRequest,
} else if (User.Fields.PASSWORD_HASH.match(currentFieldName, parser.getDeprecationHandler())) {
if (token == XContentParser.Token.VALUE_STRING) {
char[] passwordChars = parser.text().toCharArray();
passwordHash(passwordChars);
passwordHash(passwordChars, hasher);
} else {
throw new ElasticsearchParseException(
"expected field [{}] to be of type string, but found [{}] instead", currentFieldName, token);
@ -177,4 +186,9 @@ public class PutUserRequestBuilder extends ActionRequestBuilder<PutUserRequest,
}
}
private ValidationException validationException(String abc) {
ValidationException validationException = new ValidationException();
validationException.addValidationError(abc);
return validationException;
}
}

72
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/PutUserRequestBuilderTests.java
View File

@ -7,7 +7,11 @@ package org.elasticsearch.xpack.security.action.user;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.client.Client;
import org.elasticsearch.common.ValidationException;
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.PutUserRequest;
@ -16,9 +20,12 @@ 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.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.Mockito.mock;
@ -135,4 +142,69 @@ public class PutUserRequestBuilderTests extends ESTestCase {
builder.source("kibana4", new BytesArray(json.getBytes(StandardCharsets.UTF_8)), XContentType.JSON, Hasher.BCRYPT).request();
assertFalse(request.enabled());
}
public void testWithValidPasswordHash() throws IOException {
final Hasher hasher = Hasher.BCRYPT4; // this is the fastest hasher we officially support
final char[] hash = hasher.hash(new SecureString("secret".toCharArray()));
final String json = "{\n" +
" \"password_hash\": \"" + new String(hash) + "\"," +
" \"roles\": []\n" +
"}";
PutUserRequestBuilder requestBuilder = new PutUserRequestBuilder(mock(Client.class));
PutUserRequest request = requestBuilder.source("hash_user",
new BytesArray(json.getBytes(StandardCharsets.UTF_8)), XContentType.JSON, hasher).request();
assertThat(request.passwordHash(), equalTo(hash));
assertThat(request.username(), equalTo("hash_user"));
}
public void testWithMismatchedPasswordHash() throws IOException {
final Hasher systemHasher = Hasher.BCRYPT8;
final Hasher userHasher = Hasher.BCRYPT4; // this is the fastest hasher we officially support
final char[] hash = userHasher.hash(new SecureString("secret".toCharArray()));
final String json = "{\n" +
" \"password_hash\": \"" + new String(hash) + "\"," +
" \"roles\": []\n" +
"}";
PutUserRequestBuilder builder = new PutUserRequestBuilder(mock(Client.class));
final IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> {
builder.source("hash_user", new BytesArray(json.getBytes(StandardCharsets.UTF_8)), XContentType.JSON, systemHasher).request();
});
assertThat(ex.getMessage(), containsString(userHasher.name()));
assertThat(ex.getMessage(), containsString(systemHasher.name()));
}
public void testWithPasswordHashThatsNotReallyAHash() throws IOException {
final Hasher systemHasher = Hasher.PBKDF2;
final String json = "{\n" +
" \"password_hash\": \"not-a-hash\"," +
" \"roles\": []\n" +
"}";
PutUserRequestBuilder builder = new PutUserRequestBuilder(mock(Client.class));
final IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> {
builder.source("hash_user", new BytesArray(json.getBytes(StandardCharsets.UTF_8)), XContentType.JSON, systemHasher).request();
});
assertThat(ex.getMessage(), containsString(Hasher.NOOP.name()));
assertThat(ex.getMessage(), containsString(systemHasher.name()));
}
public void testWithBothPasswordAndHash() throws IOException {
final Hasher hasher = randomFrom(Hasher.BCRYPT4, Hasher.PBKDF2_1000);
final String password = randomAlphaOfLength(12);
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));
fields.put("roles", Collections.emptyList());
BytesReference json = BytesReference.bytes(XContentBuilder.builder(XContentType.JSON.xContent())
.map(shuffleMap(fields, Collections.emptySet())));
PutUserRequestBuilder builder = new PutUserRequestBuilder(mock(Client.class));
final IllegalArgumentException ex = expectThrows(ValidationException.class, () -> {
builder.source("hash_user", json, XContentType.JSON, hasher).request();
});
assertThat(ex.getMessage(), containsString("password_hash has already been set"));
}
}

48
x-pack/plugin/src/test/resources/rest-api-spec/test/users/10_basic.yml
View File

@ -13,6 +13,10 @@ teardown:
xpack.security.delete_user:
username: "joe"
ignore: 404
- do:
xpack.security.delete_user:
username: "bob"
ignore: 404
---
"Test put user api":
@ -101,3 +105,47 @@ teardown:
"key2" : "val2"
}
}
---
"Test put user with password hash":
# Mostly this chain of put_user , search index, set value is to work around the fact that the
# rest tests treat anything with a leading "$" as a stashed value, and bcrypt passwords start with "$"
# But it has the nice side effect of automatically adjusting to any changes in the default hasher for
# the ES cluster
- do:
xpack.security.put_user:
username: "bob"
body: >
{
"password" : "correct horse battery staple",
"roles" : [ ]
}
- do:
get:
index: .security
type: doc
id: user-bob
- set: { _source.password: "hash" }
- do:
xpack.security.put_user:
username: "joe"
body: >
{
"password_hash" : "$hash",
"roles" : [ "superuser" ]
}
# base64("joe:correct horse battery staple") = "am9lOmNvcnJlY3QgaG9yc2UgYmF0dGVyeSBzdGFwbGU="
- do:
headers:
Authorization: "Basic am9lOmNvcnJlY3QgaG9yc2UgYmF0dGVyeSBzdGFwbGU="
xpack.security.authenticate: {}
- match: { username: "joe" }
- do:
catch: unauthorized
headers:
Authorization: "Basic am9lOnMza3JpdA=="
xpack.security.authenticate: {}