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

HLRC: Add ability to put user with a password hash #35844

Merged
merged 6 commits into from
Nov 27, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,46 @@ public final class PutUserRequest implements Validatable, ToXContentObject {

private final User user;
private final @Nullable char[] password;
private final @Nullable char[] passwordHash;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for my understanding, is there a reason why we do not use SecureString for password/passwordHash in HLRC?
I guess if users use SecureString they will get a warning in IDE if the resource is not closed properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use SecureString anywhere in the HLRC.
I think it's conversation worth having (or maybe it already happened, but I can't find it), but I'd prefer it not be buried in this PR.

private final boolean enabled;
private final RefreshPolicy refreshPolicy;

/**
* Create or update a user in the native realm, with the user's new or updated password specified in plaintext.
* @param user the user to be created or updated
* @param password the password of the user. The password array is not modified by this class.
* It is the responsibility of the caller to clear the password after receiving
* a response.
* @param enabled true if the user is enabled and allowed to access elasticsearch
* @param refreshPolicy the refresh policy for the request.
*/
public static PutUserRequest withPassword(User user, char[] password, boolean enabled, RefreshPolicy refreshPolicy) {
return new PutUserRequest(user, password, null, enabled, refreshPolicy);
}

/**
* Create or update a user in the native realm, with the user's new or updated password specified as a cryptographic hash.
* @param user the user to be created or updated
* @param passwordHash the hash of the password of the user. It must be in the correct format for the password hashing algorithm in
* use on this elasticsearch cluster. The array is not modified by this class.
* It is the responsibility of the caller to clear the hash after receiving a response.
* @param enabled true if the user is enabled and allowed to access elasticsearch
* @param refreshPolicy the refresh policy for the request.
*/
public static PutUserRequest withPasswordHash(User user, char[] passwordHash, boolean enabled, RefreshPolicy refreshPolicy) {
return new PutUserRequest(user, null, passwordHash, enabled, refreshPolicy);
}

/**
* Update an existing user in the native realm without modifying their password.
* @param user the user to be created or updated
* @param enabled true if the user is enabled and allowed to access elasticsearch
* @param refreshPolicy the refresh policy for the request.
*/
public static PutUserRequest updateUser(User user, boolean enabled, RefreshPolicy refreshPolicy) {
return new PutUserRequest(user, null, null, enabled, refreshPolicy);
}

/**
* Creates a new request that is used to create or update a user in the native realm.
*
Expand All @@ -51,10 +88,33 @@ public final class PutUserRequest implements Validatable, ToXContentObject {
* a response.
* @param enabled true if the user is enabled and allowed to access elasticsearch
* @param refreshPolicy the refresh policy for the request.
* @deprecated Use {@link #withPassword(User, char[], boolean, RefreshPolicy)} or
* {@link #updateUser(User, boolean, RefreshPolicy)} instead.
*/
@Deprecated
public PutUserRequest(User user, @Nullable char[] password, boolean enabled, @Nullable RefreshPolicy refreshPolicy) {
this(user, password, null, enabled, refreshPolicy);
}

/**
* Creates a new request that is used to create or update a user in the native realm.
* @param user the user to be created or updated
* @param password the password of the user. The password array is not modified by this class.
* It is the responsibility of the caller to clear the password after receiving
* a response.
* @param passwordHash the hash of the password. Only one of "password" or "passwordHash" may be populated.
* The other parameter must be {@code null}.
* @param enabled true if the user is enabled and allowed to access elasticsearch
* @param refreshPolicy the refresh policy for the request.
*/
private PutUserRequest(User user, @Nullable char[] password, @Nullable char[] passwordHash, boolean enabled,
RefreshPolicy refreshPolicy) {
this.user = Objects.requireNonNull(user, "user is required, cannot be null");
if (password != null && passwordHash != null) {
throw new IllegalArgumentException("cannot specify both password and passwordHash");
}
this.password = password;
this.passwordHash = passwordHash;
this.enabled = enabled;
this.refreshPolicy = refreshPolicy == null ? RefreshPolicy.getDefault() : refreshPolicy;
}
Expand Down Expand Up @@ -82,6 +142,7 @@ public boolean equals(Object o) {
final PutUserRequest that = (PutUserRequest) o;
return Objects.equals(user, that.user)
&& Arrays.equals(password, that.password)
&& Arrays.equals(passwordHash, that.passwordHash)
&& enabled == that.enabled
&& refreshPolicy == that.refreshPolicy;
}
Expand All @@ -90,6 +151,7 @@ public boolean equals(Object o) {
public int hashCode() {
int result = Objects.hash(user, enabled, refreshPolicy);
result = 31 * result + Arrays.hashCode(password);
result = 31 * result + Arrays.hashCode(passwordHash);
return result;
}

Expand All @@ -108,12 +170,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.startObject();
builder.field("username", user.getUsername());
if (password != null) {
byte[] charBytes = CharArrays.toUtf8Bytes(password);
try {
builder.field("password").utf8Value(charBytes, 0, charBytes.length);
} finally {
Arrays.fill(charBytes, (byte) 0);
}
charField(builder, "password", password);
}
if (passwordHash != null) {
charField(builder, "password_hash", passwordHash);
}
builder.field("roles", user.getRoles());
if (user.getFullName() != null) {
Expand All @@ -126,4 +186,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field("enabled", enabled);
return builder.endObject();
}

private void charField(XContentBuilder builder, String fieldName, char[] chars) throws IOException {
byte[] charBytes = CharArrays.toUtf8Bytes(chars);
try {
builder.field(fieldName).utf8Value(charBytes, 0, charBytes.length);
} finally {
Arrays.fill(charBytes, (byte) 0);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
import org.apache.http.nio.entity.NStringEntity;
import org.elasticsearch.ElasticsearchStatusException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.LatchedActionListener;
import org.elasticsearch.action.support.PlainActionFuture;
Expand Down Expand Up @@ -75,7 +76,11 @@
import org.elasticsearch.rest.RestStatus;
import org.hamcrest.Matchers;

import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
import java.io.IOException;
import java.security.SecureRandom;
import java.util.Base64;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -88,6 +93,7 @@

import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.emptyIterable;
Expand All @@ -103,10 +109,13 @@ public void testPutUser() throws Exception {
RestHighLevelClient client = highLevelClient();

{
//tag::put-user-execute
//tag::put-user-password-request
char[] password = new char[]{'p', 'a', 's', 's', 'w', 'o', 'r', 'd'};
User user = new User("example", Collections.singletonList("superuser"));
PutUserRequest request = new PutUserRequest(user, password, true, RefreshPolicy.NONE);
PutUserRequest request = PutUserRequest.withPassword(user, password, true, RefreshPolicy.NONE);
//end::put-user-password-request

//tag::put-user-execute
PutUserResponse response = client.security().putUser(request, RequestOptions.DEFAULT);
//end::put-user-execute

Expand All @@ -116,11 +125,37 @@ public void testPutUser() throws Exception {

assertTrue(isCreated);
}

{
byte[] salt = new byte[32];
SecureRandom.getInstanceStrong().nextBytes(salt);
char[] password = new char[]{'p', 'a', 's', 's', 'w', 'o', 'r', 'd'};
User user2 = new User("example2", Collections.singletonList("superuser"));
PutUserRequest request = new PutUserRequest(user2, password, true, RefreshPolicy.NONE);
User user = new User("example2", Collections.singletonList("superuser"));

//tag::put-user-hash-request
SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance("PBKDF2withHMACSHA512");
PBEKeySpec keySpec = new PBEKeySpec(password, salt, 10000, 256);
final byte[] pbkdfEncoded = secretKeyFactory.generateSecret(keySpec).getEncoded();
char[] passwordHash = ("{PBKDF2}10000$" + Base64.getEncoder().encodeToString(salt)
+ "$" + Base64.getEncoder().encodeToString(pbkdfEncoded)).toCharArray();

PutUserRequest request = PutUserRequest.withPasswordHash(user, passwordHash, true, RefreshPolicy.NONE);
//end::put-user-hash-request

try {
client.security().putUser(request, RequestOptions.DEFAULT);
} catch (ElasticsearchStatusException e) {
// This is expected to fail as the server will not be using PBKDF2, but that's easiest hasher to support
// in a standard JVM without introducing additional libraries.
assertThat(e.getDetailedMessage(), containsString("PBKDF2"));
}
}

{
User user = new User("example", Arrays.asList("superuser", "another-role"));
//tag::put-user-update-request
PutUserRequest request = PutUserRequest.updateUser(user, true, RefreshPolicy.NONE);
//end::put-user-update-request

// tag::put-user-execute-listener
ActionListener<PutUserResponse> listener = new ActionListener<PutUserResponse>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.client.security;

import org.elasticsearch.client.security.user.User;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.test.ESTestCase;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;

public class PutUserRequestTests extends ESTestCase {

public void testBuildRequestWithPassword() throws Exception {
final User user = new User("hawkeye", Arrays.asList("kibana_user", "avengers"),
Collections.singletonMap("status", "active"), "Clinton Barton", null);
final char[] password = "f@rmb0y".toCharArray();
final PutUserRequest request = PutUserRequest.withPassword(user, password, true, RefreshPolicy.IMMEDIATE);
String json = Strings.toString(request);
final Map<String, Object> requestAsMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), json, false);
assertThat(requestAsMap.get("username"), is("hawkeye"));
assertThat(requestAsMap.get("roles"), instanceOf(List.class));
assertThat((List<?>) requestAsMap.get("roles"), containsInAnyOrder("kibana_user", "avengers"));
assertThat(requestAsMap.get("password"), is("f@rmb0y"));
assertThat(requestAsMap.containsKey("password_hash"), is(false));
assertThat(requestAsMap.get("full_name"), is("Clinton Barton"));
assertThat(requestAsMap.containsKey("email"), is(false));
assertThat(requestAsMap.get("enabled"), is(true));
assertThat(requestAsMap.get("metadata"), instanceOf(Map.class));
final Map<?, ?> metadata = (Map<?, ?>) requestAsMap.get("metadata");
assertThat(metadata.size(), is(1));
assertThat(metadata.get("status"), is("active"));
}

public void testBuildRequestWithPasswordHash() throws Exception {
final User user = new User("hawkeye", Arrays.asList("kibana_user", "avengers"),
Collections.singletonMap("status", "active"), "Clinton Barton", null);
final char[] passwordHash = "$2a$04$iu1G4x3ZKVDNi6egZIjkFuIPja6elQXiBF1LdRVauV4TGog6FYOpi".toCharArray();
final PutUserRequest request = PutUserRequest.withPasswordHash(user, passwordHash, true, RefreshPolicy.IMMEDIATE);
String json = Strings.toString(request);
final Map<String, Object> requestAsMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), json, false);
assertThat(requestAsMap.get("username"), is("hawkeye"));
assertThat(requestAsMap.get("roles"), instanceOf(List.class));
assertThat((List<?>) requestAsMap.get("roles"), containsInAnyOrder("kibana_user", "avengers"));
assertThat(requestAsMap.get("password_hash"), is("$2a$04$iu1G4x3ZKVDNi6egZIjkFuIPja6elQXiBF1LdRVauV4TGog6FYOpi"));
assertThat(requestAsMap.containsKey("password"), is(false));
assertThat(requestAsMap.get("full_name"), is("Clinton Barton"));
assertThat(requestAsMap.containsKey("email"), is(false));
assertThat(requestAsMap.get("enabled"), is(true));
assertThat(requestAsMap.get("metadata"), instanceOf(Map.class));
final Map<?, ?> metadata = (Map<?, ?>) requestAsMap.get("metadata");
assertThat(metadata.size(), is(1));
assertThat(metadata.get("status"), is("active"));
}

public void testBuildRequestForUpdateOnly() throws Exception {
final User user = new User("hawkeye", Arrays.asList("kibana_user", "avengers"),
Collections.singletonMap("status", "active"), "Clinton Barton", null);
final char[] passwordHash = "$2a$04$iu1G4x3ZKVDNi6egZIjkFuIPja6elQXiBF1LdRVauV4TGog6FYOpi".toCharArray();
final PutUserRequest request = PutUserRequest.updateUser(user, true, RefreshPolicy.IMMEDIATE);
String json = Strings.toString(request);
final Map<String, Object> requestAsMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), json, false);
assertThat(requestAsMap.get("username"), is("hawkeye"));
assertThat(requestAsMap.get("roles"), instanceOf(List.class));
assertThat((List<?>) requestAsMap.get("roles"), containsInAnyOrder("kibana_user", "avengers"));
assertThat(requestAsMap.containsKey("password"), is(false));
assertThat(requestAsMap.containsKey("password_hash"), is(false));
assertThat(requestAsMap.get("full_name"), is("Clinton Barton"));
assertThat(requestAsMap.containsKey("email"), is(false));
assertThat(requestAsMap.get("enabled"), is(true));
assertThat(requestAsMap.get("metadata"), instanceOf(Map.class));
final Map<?, ?> metadata = (Map<?, ?>) requestAsMap.get("metadata");
assertThat(metadata.size(), is(1));
assertThat(metadata.get("status"), is("active"));
}

}
Loading