From 059eb5510867eeee97c67ad42b6217687ff69c7c Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Fri, 21 Jun 2019 17:11:07 +1000 Subject: [PATCH] Use SecureString for password length validation (#43465) This replaces the use of char[] in the password length validation code, with the use of SecureString Although the use of char[] is not in itself problematic, using a SecureString encourages callers to think about the lifetime of the password object and to clear it after use. Backport of: #42884 --- .../user/ChangePasswordRequestBuilder.java | 2 +- .../action/user/PutUserRequestBuilder.java | 17 +++- .../core/security/client/SecurityClient.java | 5 + .../core/security/support/Validation.java | 9 +- .../security/support/ValidationTests.java | 7 +- .../esnative/tool/SetupPasswordTool.java | 2 +- .../security/authc/file/tool/UsersTool.java | 30 +++--- .../authc/esnative/NativeRealmIntegTests.java | 98 +++++++++---------- .../esnative/tool/SetupPasswordToolTests.java | 2 +- .../security/authz/SecurityScrollTests.java | 2 +- .../SecurityIndexManagerIntegTests.java | 4 +- 11 files changed, 96 insertions(+), 82 deletions(-) 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 d7538c2a556..632e5fb6b54 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 @@ -42,7 +42,7 @@ public class ChangePasswordRequestBuilder } public static char[] validateAndHashPassword(SecureString password, Hasher hasher) { - Validation.Error error = Validation.Users.validatePassword(password.getChars()); + Validation.Error error = Validation.Users.validatePassword(password); if (error != null) { ValidationException validationException = new ValidationException(); validationException.addValidationError(error.toString()); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/PutUserRequestBuilder.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/PutUserRequestBuilder.java index f667bff513b..bee938f6c02 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/PutUserRequestBuilder.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/PutUserRequestBuilder.java @@ -25,7 +25,6 @@ import org.elasticsearch.xpack.core.security.xcontent.XContentUtils; import java.io.IOException; import java.io.InputStream; -import java.util.Arrays; import java.util.Map; import java.util.Objects; @@ -50,7 +49,15 @@ public class PutUserRequestBuilder extends ActionRequestBuilder listener) { client.execute(PutUserAction.INSTANCE, request, listener); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Validation.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Validation.java index 55caed43411..5c2c0304291 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Validation.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Validation.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.security.support; +import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.xpack.core.security.authc.esnative.ClientReservedRealm; @@ -81,10 +82,10 @@ public final class Validation { return null; } - public static Error validatePassword(char[] password) { - return password.length >= MIN_PASSWD_LENGTH ? - null : - new Error("passwords must be at least [" + MIN_PASSWD_LENGTH + "] characters long"); + public static Error validatePassword(SecureString password) { + return password.length() >= MIN_PASSWD_LENGTH ? + null : + new Error("passwords must be at least [" + MIN_PASSWD_LENGTH + "] characters long"); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/ValidationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/ValidationTests.java index 458280677a4..b6226ec5c18 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/ValidationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/ValidationTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.security.support; +import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; @@ -59,12 +60,12 @@ public class ValidationTests extends ESTestCase { } public void testUsersValidatePassword() throws Exception { - String passwd = randomAlphaOfLength(randomIntBetween(0, 20)); + SecureString passwd = new SecureString(randomAlphaOfLength(randomIntBetween(0, 20)).toCharArray()); logger.info("{}[{}]", passwd, passwd.length()); if (passwd.length() >= 6) { - assertThat(Users.validatePassword(passwd.toCharArray()), nullValue()); + assertThat(Users.validatePassword(passwd), nullValue()); } else { - assertThat(Users.validatePassword(passwd.toCharArray()), notNullValue()); + assertThat(Users.validatePassword(passwd), notNullValue()); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java index 5926cdbc01c..866f3722e6e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java @@ -193,7 +193,7 @@ public class SetupPasswordTool extends LoggingAwareMultiCommand { // loop for two consecutive good passwords while (true) { SecureString password1 = new SecureString(terminal.readSecret("Enter password for [" + user + "]: ")); - Validation.Error err = Validation.Users.validatePassword(password1.getChars()); + Validation.Error err = Validation.Users.validatePassword(password1); if (err != null) { terminal.println(err.toString()); terminal.println("Try again."); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java index 6007ef5fd6d..6d51fc5df93 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.security.authc.file.tool; import joptsimple.OptionSet; import joptsimple.OptionSpec; - import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.LoggingAwareMultiCommand; @@ -111,7 +110,7 @@ public class UsersTool extends LoggingAwareMultiCommand { throw new UserException(ExitCodes.DATA_ERROR, "Invalid username [" + username + "]... " + validationError); } - char[] password = parsePassword(terminal, passwordOption.value(options)); + final char[] passwordHash = getPasswordHash(terminal, env, passwordOption.value(options)); String[] roles = parseRoles(terminal, env, rolesOption.value(options)); Path passwordFile = FileUserPasswdStore.resolveFile(env); @@ -125,9 +124,8 @@ public class UsersTool extends LoggingAwareMultiCommand { if (users.containsKey(username)) { throw new UserException(ExitCodes.CODE_ERROR, "User [" + username + "] already exists"); } - final Hasher hasher = Hasher.resolve(XPackSettings.PASSWORD_HASHING_ALGORITHM.get(env.settings())); users = new HashMap<>(users); // make modifiable - users.put(username, hasher.hash(new SecureString(password))); + users.put(username, passwordHash); FileUserPasswdStore.writeFile(users, passwordFile); if (roles.length > 0) { @@ -218,7 +216,7 @@ public class UsersTool extends LoggingAwareMultiCommand { protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { String username = parseUsername(arguments.values(options), env.settings()); - char[] password = parsePassword(terminal, passwordOption.value(options)); + char[] passwordHash = getPasswordHash(terminal, env, passwordOption.value(options)); Path file = FileUserPasswdStore.resolveFile(env); FileAttributesChecker attributesChecker = new FileAttributesChecker(file); @@ -229,9 +227,8 @@ public class UsersTool extends LoggingAwareMultiCommand { if (users.containsKey(username) == false) { throw new UserException(ExitCodes.NO_USER, "User [" + username + "] doesn't exist"); } - final Hasher hasher = Hasher.resolve(XPackSettings.PASSWORD_HASHING_ALGORITHM.get(env.settings())); users = new HashMap<>(users); // make modifiable - users.put(username, hasher.hash(new SecureString(password))); + users.put(username, passwordHash); FileUserPasswdStore.writeFile(users, file); attributesChecker.check(terminal); @@ -440,23 +437,32 @@ public class UsersTool extends LoggingAwareMultiCommand { return username; } + private static char[] getPasswordHash(Terminal terminal, Environment env, String cliPasswordValue) throws UserException { + final Hasher hasher = Hasher.resolve(XPackSettings.PASSWORD_HASHING_ALGORITHM.get(env.settings())); + final char[] passwordHash; + try (SecureString password = parsePassword(terminal, cliPasswordValue)) { + passwordHash = hasher.hash(password); + } + return passwordHash; + } + // pkg private for testing - static char[] parsePassword(Terminal terminal, String passwordStr) throws UserException { - char[] password; + static SecureString parsePassword(Terminal terminal, String passwordStr) throws UserException { + SecureString password; if (passwordStr != null) { - password = passwordStr.toCharArray(); + password = new SecureString(passwordStr.toCharArray()); Validation.Error validationError = Users.validatePassword(password); if (validationError != null) { throw new UserException(ExitCodes.DATA_ERROR, "Invalid password..." + validationError); } } else { - password = terminal.readSecret("Enter new password: "); + password = new SecureString(terminal.readSecret("Enter new password: ")); Validation.Error validationError = Users.validatePassword(password); if (validationError != null) { throw new UserException(ExitCodes.DATA_ERROR, "Invalid password..." + validationError); } char[] retyped = terminal.readSecret("Retype new password: "); - if (Arrays.equals(password, retyped) == false) { + if (Arrays.equals(password.getChars(), retyped) == false) { throw new UserException(ExitCodes.DATA_ERROR, "Password mismatch"); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java index 9e7371f95ed..5b12df0584f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java @@ -124,7 +124,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { public void testDeletingNonexistingUserAndRole() throws Exception { SecurityClient c = securityClient(); // first create the index so it exists - c.preparePutUser("joe", "s3kirt".toCharArray(), hasher, "role1", "user").get(); + c.preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, "role1", "user").get(); DeleteUserResponse resp = c.prepareDeleteUser("missing").get(); assertFalse("user shouldn't be found", resp.found()); DeleteRoleResponse resp2 = c.prepareDeleteRole("role").get(); @@ -144,7 +144,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { final List existingUsers = Arrays.asList(c.prepareGetUsers().get().users()); final int existing = existingUsers.size(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3kirt".toCharArray(), hasher, "role1", "user").get(); + c.preparePutUser("joe", new SecureString("s3kirt".toCharArray()), hasher, "role1", "user").get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_MAIN_ALIAS); logger.info("--> retrieving user"); @@ -155,8 +155,8 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { assertArrayEquals(joe.roles(), new String[]{"role1", "user"}); logger.info("--> adding two more users"); - c.preparePutUser("joe2", "s3kirt2".toCharArray(), hasher, "role2", "user").get(); - c.preparePutUser("joe3", "s3kirt3".toCharArray(), hasher, "role3", "user").get(); + c.preparePutUser("joe2", new SecureString("s3kirt2".toCharArray()), hasher, "role2", "user").get(); + c.preparePutUser("joe3", new SecureString("s3kirt3".toCharArray()), hasher, "role3", "user").get(); GetUsersResponse allUsersResp = c.prepareGetUsers().get(); assertTrue("users should exist", allUsersResp.hasUsers()); assertEquals("should be " + (3 + existing) + " users total", 3 + existing, allUsersResp.users().length); @@ -250,7 +250,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { new BytesArray("{\"match_all\": {}}"), randomBoolean()) .get(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3krit".toCharArray(), hasher, "test_role").get(); + c.preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, "test_role").get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_MAIN_ALIAS); logger.info("--> retrieving user"); @@ -262,7 +262,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { // Index a document with the default test user client().prepareIndex("idx", "doc", "1").setSource("body", "foo").setRefreshPolicy(IMMEDIATE).get(); - String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); + String token = basicAuthHeaderValue("joe", new SecureString("s3krit")); SearchResponse searchResp = client().filterWithHeader(Collections.singletonMap("Authorization", token)).prepareSearch("idx").get(); assertEquals(1L, searchResp.getHits().getTotalHits().value); @@ -271,7 +271,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { public void testUpdatingUserAndAuthentication() throws Exception { SecurityClient c = securityClient(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3krit".toCharArray(), hasher, SecuritySettingsSource.TEST_ROLE).get(); + c.preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, SecuritySettingsSource.TEST_ROLE).get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_MAIN_ALIAS); logger.info("--> retrieving user"); @@ -283,12 +283,12 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { ensureGreen("idx"); // Index a document with the default test user client().prepareIndex("idx", "doc", "1").setSource("body", "foo").setRefreshPolicy(IMMEDIATE).get(); - String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); + String token = basicAuthHeaderValue("joe", new SecureString("s3krit")); SearchResponse searchResp = client().filterWithHeader(Collections.singletonMap("Authorization", token)).prepareSearch("idx").get(); assertEquals(1L, searchResp.getHits().getTotalHits().value); - c.preparePutUser("joe", "s3krit2".toCharArray(), hasher, SecuritySettingsSource.TEST_ROLE).get(); + c.preparePutUser("joe", new SecureString("s3krit2".toCharArray()), hasher, SecuritySettingsSource.TEST_ROLE).get(); try { client().filterWithHeader(Collections.singletonMap("Authorization", token)).prepareSearch("idx").get(); @@ -298,7 +298,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { assertThat(e.status(), is(RestStatus.UNAUTHORIZED)); } - token = basicAuthHeaderValue("joe", new SecureString("s3krit2".toCharArray())); + token = basicAuthHeaderValue("joe", new SecureString("s3krit2")); searchResp = client().filterWithHeader(Collections.singletonMap("Authorization", token)).prepareSearch("idx").get(); assertEquals(1L, searchResp.getHits().getTotalHits().value); } @@ -306,7 +306,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { public void testCreateDeleteAuthenticate() { SecurityClient c = securityClient(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3krit".toCharArray(), hasher, + c.preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, SecuritySettingsSource.TEST_ROLE).get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_MAIN_ALIAS); @@ -319,7 +319,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { ensureGreen("idx"); // Index a document with the default test user client().prepareIndex("idx", "doc", "1").setSource("body", "foo").setRefreshPolicy(IMMEDIATE).get(); - String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); + String token = basicAuthHeaderValue("joe", new SecureString("s3krit")); SearchResponse searchResp = client().filterWithHeader(Collections.singletonMap("Authorization", token)).prepareSearch("idx").get(); assertEquals(1L, searchResp.getHits().getTotalHits().value); @@ -345,12 +345,12 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { new BytesArray("{\"match_all\": {}}"), randomBoolean()) .get(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3krit".toCharArray(), hasher, "test_role").get(); + c.preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, "test_role").get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_MAIN_ALIAS); if (authenticate) { - final String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); + final String token = basicAuthHeaderValue("joe", new SecureString("s3krit")); ClusterHealthResponse response = client().filterWithHeader(Collections.singletonMap("Authorization", token)).admin().cluster() .prepareHealth().get(); assertFalse(response.isTimedOut()); @@ -394,7 +394,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { .addIndices(new String[]{"*"}, new String[]{"create_index"}, null, null, null, true) .get(); logger.error("--> creating user"); - securityClient().preparePutUser("joe", "s3krit".toCharArray(), hasher, "test_role", "snapshot_user").get(); + securityClient().preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, "test_role", "snapshot_user").get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_MAIN_ALIAS); logger.info("--> creating repository"); @@ -404,7 +404,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { .put("location", randomRepoPath()) .put("compress", randomBoolean()) .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES))); - final String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); + final String token = basicAuthHeaderValue("joe", new SecureString("s3krit")); // joe can snapshot all indices, including '.security' SnapshotInfo snapshotInfo = client().filterWithHeader(Collections.singletonMap("Authorization", token)).admin().cluster() .prepareCreateSnapshot("test-repo", "test-snap-1") @@ -458,11 +458,11 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { .addIndices(new String[]{"*"}, new String[]{"read"}, new String[]{"body", "title"}, null, new BytesArray("{\"match_all\": {}}"), randomBoolean()) .get(); - c.preparePutUser("joe", "s3krit".toCharArray(), hasher, "test_role").get(); + c.preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, "test_role").get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_MAIN_ALIAS); - final String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); + final String token = basicAuthHeaderValue("joe", new SecureString("s3krit")); ClusterHealthResponse response = client().filterWithHeader(Collections.singletonMap("Authorization", token)).admin().cluster() .prepareHealth().get(); assertFalse(response.isTimedOut()); @@ -492,7 +492,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { assertThat(client.prepareGetUsers("joes").get().hasUsers(), is(false)); // check that putting a user without a password fails if the user doesn't exist try { - client.preparePutUser("joe", null, hasher, "admin_role").get(); + client.preparePutUser("joe", (SecureString) null, hasher, "admin_role").get(); fail("cannot create a user without a password"); } catch (IllegalArgumentException e) { assertThat(e.getMessage(), containsString("password must be specified")); @@ -501,8 +501,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { assertThat(client.prepareGetUsers("joes").get().hasUsers(), is(false)); // create joe with a password and verify the user works - client.preparePutUser("joe", SecuritySettingsSourceField.TEST_PASSWORD.toCharArray(), - hasher, "admin_role").get(); + client.preparePutUser("joe", SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING, hasher, "admin_role").get(); assertThat(client.prepareGetUsers("joe").get().hasUsers(), is(true)); final String token = basicAuthHeaderValue("joe", SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING); ClusterHealthResponse response = client().filterWithHeader(Collections.singletonMap("Authorization", token)).admin().cluster() @@ -510,7 +509,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { assertFalse(response.isTimedOut()); // modify joe without sending the password - client.preparePutUser("joe", null, hasher, "read_role").fullName("Joe Smith").get(); + client.preparePutUser("joe", (SecureString) null, hasher, "read_role").fullName("Joe Smith").get(); GetUsersResponse getUsersResponse = client.prepareGetUsers("joe").get(); assertThat(getUsersResponse.hasUsers(), is(true)); assertThat(getUsersResponse.users().length, is(1)); @@ -531,7 +530,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { // update the user with password and admin role again String secondPassword = SecuritySettingsSourceField.TEST_PASSWORD + "2"; - client.preparePutUser("joe", secondPassword.toCharArray(), hasher, "admin_role"). + client.preparePutUser("joe", new SecureString(secondPassword.toCharArray()), hasher, "admin_role"). fullName("Joe Smith").get(); getUsersResponse = client.prepareGetUsers("joe").get(); assertThat(getUsersResponse.hasUsers(), is(true)); @@ -560,8 +559,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { public void testCannotCreateUserWithShortPassword() throws Exception { SecurityClient client = securityClient(); try { - client.preparePutUser("joe", randomAlphaOfLengthBetween(0, 5).toCharArray(), hasher, - "admin_role").get(); + client.preparePutUser("joe", new SecureString(randomAlphaOfLengthBetween(0, 5).toCharArray()), hasher, "admin_role").get(); fail("cannot create a user without a password < 6 characters"); } catch (IllegalArgumentException v) { assertThat(v.getMessage().contains("password"), is(true)); @@ -571,8 +569,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { public void testCannotCreateUserWithInvalidCharactersInName() throws Exception { SecurityClient client = securityClient(); IllegalArgumentException v = expectThrows(IllegalArgumentException.class, - () -> client.preparePutUser("fóóbár", "my-am@zing-password".toCharArray(), hasher, - "admin_role").get() + () -> client.preparePutUser("fóóbár", new SecureString("my-am@zing-password".toCharArray()), hasher, "admin_role").get() ); assertThat(v.getMessage(), containsString("names must be")); } @@ -582,7 +579,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { SecurityClient client = securityClient(); if (randomBoolean()) { - client.preparePutUser("joe", "s3krit".toCharArray(), hasher, + client.preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, SecuritySettingsSource.TEST_ROLE).get(); } else { client.preparePutRole("read_role") @@ -602,8 +599,8 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { public void testOperationsOnReservedUsers() throws Exception { final String username = randomFrom(ElasticUser.NAME, KibanaUser.NAME); IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, - () -> securityClient().preparePutUser(username, randomBoolean() ? SecuritySettingsSourceField.TEST_PASSWORD.toCharArray() - : null, hasher, "admin").get()); + () -> securityClient().preparePutUser(username, + randomBoolean() ? SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING : null, hasher, "admin").get()); assertThat(exception.getMessage(), containsString("user [" + username + "] is reserved")); exception = expectThrows(IllegalArgumentException.class, @@ -614,27 +611,25 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { () -> securityClient().prepareDeleteUser(AnonymousUser.DEFAULT_ANONYMOUS_USERNAME).get()); assertThat(exception.getMessage(), containsString("user [" + AnonymousUser.DEFAULT_ANONYMOUS_USERNAME + "] is anonymous")); + final char[] foobar = "foobar".toCharArray(); exception = expectThrows(IllegalArgumentException.class, - () -> securityClient().prepareChangePassword(AnonymousUser.DEFAULT_ANONYMOUS_USERNAME, "foobar".toCharArray(), - hasher).get()); + () -> securityClient().prepareChangePassword(AnonymousUser.DEFAULT_ANONYMOUS_USERNAME, foobar, hasher).get()); assertThat(exception.getMessage(), containsString("user [" + AnonymousUser.DEFAULT_ANONYMOUS_USERNAME + "] is anonymous")); exception = expectThrows(IllegalArgumentException.class, - () -> securityClient().preparePutUser(AnonymousUser.DEFAULT_ANONYMOUS_USERNAME, "foobar".toCharArray(), - hasher).get()); + () -> securityClient().preparePutUser(AnonymousUser.DEFAULT_ANONYMOUS_USERNAME, new SecureString(foobar), hasher).get()); assertThat(exception.getMessage(), containsString("user [" + AnonymousUser.DEFAULT_ANONYMOUS_USERNAME + "] is anonymous")); exception = expectThrows(IllegalArgumentException.class, - () -> securityClient().preparePutUser(SystemUser.NAME, "foobar".toCharArray(), hasher).get()); + () -> securityClient().preparePutUser(SystemUser.NAME, new SecureString(foobar), hasher).get()); assertThat(exception.getMessage(), containsString("user [" + SystemUser.NAME + "] is internal")); exception = expectThrows(IllegalArgumentException.class, - () -> securityClient().prepareChangePassword(SystemUser.NAME, "foobar".toCharArray(), - hasher).get()); + () -> securityClient().prepareChangePassword(SystemUser.NAME, foobar, hasher).get()); assertThat(exception.getMessage(), containsString("user [" + SystemUser.NAME + "] is internal")); exception = expectThrows(IllegalArgumentException.class, - () -> securityClient().prepareDeleteUser(SystemUser.NAME).get()); + () -> securityClient().prepareDeleteUser(SystemUser.NAME).get()); assertThat(exception.getMessage(), containsString("user [" + SystemUser.NAME + "] is internal")); // get should work @@ -671,9 +666,9 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { } public void testCreateAndChangePassword() throws Exception { - securityClient().preparePutUser("joe", "s3krit".toCharArray(), hasher, + securityClient().preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, SecuritySettingsSource.TEST_ROLE).get(); - final String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); + final String token = basicAuthHeaderValue("joe", new SecureString("s3krit")); ClusterHealthResponse response = client().filterWithHeader(Collections.singletonMap("Authorization", token)) .admin().cluster().prepareHealth().get(); assertThat(response.isTimedOut(), is(false)); @@ -760,8 +755,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { final int numNativeUsers = scaledRandomIntBetween(1, 32); SecurityClient securityClient = new SecurityClient(client()); for (int i = 0; i < numNativeUsers; i++) { - securityClient.preparePutUser("joe" + i, "s3krit".toCharArray(), hasher, - "superuser").get(); + securityClient.preparePutUser("joe" + i, new SecureString("s3krit".toCharArray()), hasher, "superuser").get(); } XPackUsageResponse response = new XPackUsageRequestBuilder(client()).get(); @@ -780,10 +774,9 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { } public void testSetEnabled() throws Exception { - - securityClient().preparePutUser("joe", "s3krit".toCharArray(), hasher, + securityClient().preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, SecuritySettingsSource.TEST_ROLE).get(); - final String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); + final String token = basicAuthHeaderValue("joe", new SecureString("s3krit")); ClusterHealthResponse response = client().filterWithHeader(Collections.singletonMap("Authorization", token)) .admin().cluster().prepareHealth().get(); assertThat(response.isTimedOut(), is(false)); @@ -806,20 +799,20 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { public void testNegativeLookupsThenCreateRole() throws Exception { SecurityClient securityClient = new SecurityClient(client()); - securityClient.preparePutUser("joe", "s3krit".toCharArray(), hasher, "unknown_role").get(); + securityClient.preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, "unknown_role").get(); final int negativeLookups = scaledRandomIntBetween(1, 10); for (int i = 0; i < negativeLookups; i++) { if (anonymousEnabled && roleExists) { ClusterHealthResponse response = client() .filterWithHeader(Collections.singletonMap("Authorization", - basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())))) + basicAuthHeaderValue("joe", new SecureString("s3krit")))) .admin().cluster().prepareHealth().get(); assertNoTimeout(response); } else { ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, () -> client() .filterWithHeader(Collections.singletonMap("Authorization", - basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())))) + basicAuthHeaderValue("joe", new SecureString("s3krit")))) .admin().cluster().prepareHealth().get()); assertThat(e.status(), is(RestStatus.FORBIDDEN)); } @@ -828,7 +821,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { securityClient.preparePutRole("unknown_role").cluster("all").get(); ClusterHealthResponse response = client() .filterWithHeader(Collections.singletonMap("Authorization", - basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())))) + basicAuthHeaderValue("joe", new SecureString("s3krit")))) .admin().cluster().prepareHealth().get(); assertNoTimeout(response); } @@ -842,10 +835,9 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { * the loader returned a null value, while the other caller(s) would get a null value unexpectedly */ public void testConcurrentRunAs() throws Exception { - securityClient().preparePutUser("joe", "s3krit".toCharArray(), hasher, SecuritySettingsSource - .TEST_ROLE).get(); - securityClient().preparePutUser("executor", "s3krit".toCharArray(), hasher, "superuser").get(); - final String token = basicAuthHeaderValue("executor", new SecureString("s3krit".toCharArray())); + securityClient().preparePutUser("joe", new SecureString("s3krit".toCharArray()), hasher, SecuritySettingsSource.TEST_ROLE).get(); + securityClient().preparePutUser("executor", new SecureString("s3krit".toCharArray()), hasher, "superuser").get(); + final String token = basicAuthHeaderValue("executor", new SecureString("s3krit")); final Client client = client().filterWithHeader(MapBuilder.newMapBuilder() .put("Authorization", token) .put("es-security-runas-user", "joe") diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java index 793e20888a5..e93950739a1 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java @@ -416,7 +416,7 @@ public class SetupPasswordToolTests extends CommandTestCase { while (failCount-- > 0) { String password1 = randomAlphaOfLength(randomIntBetween(3, 10)); terminal.addSecretInput(password1); - Validation.Error err = Validation.Users.validatePassword(password1.toCharArray()); + Validation.Error err = Validation.Users.validatePassword(new SecureString(password1.toCharArray())); if (err == null) { // passes strength validation, fail by mismatch terminal.addSecretInput(password1 + "typo"); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/SecurityScrollTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/SecurityScrollTests.java index f507edf9787..91fca881982 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/SecurityScrollTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/SecurityScrollTests.java @@ -32,7 +32,7 @@ public class SecurityScrollTests extends SecurityIntegTestCase { securityClient().preparePutRole("scrollable") .addIndices(new String[] { randomAlphaOfLengthBetween(4, 12) }, new String[] { "read" }, null, null, null, randomBoolean()) .get(); - securityClient().preparePutUser("other", SecuritySettingsSourceField.TEST_PASSWORD.toCharArray(), getFastStoredHashAlgoForTests(), + securityClient().preparePutUser("other", SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING, getFastStoredHashAlgoForTests(), "scrollable") .get(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerIntegTests.java index 55cd659509b..10a796f489c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerIntegTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.security.support; import org.elasticsearch.action.ActionFuture; import org.elasticsearch.action.support.PlainActionFuture; +import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.test.SecurityIntegTestCase; import org.elasticsearch.xpack.core.security.action.user.PutUserRequest; @@ -45,9 +46,10 @@ public class SecurityIndexManagerIntegTests extends SecurityIntegTestCase { @Override protected void doRun() throws Exception { final List requests = new ArrayList<>(numRequests); + final SecureString password = new SecureString("password".toCharArray()); for (int i = 0; i < numRequests; i++) { requests.add(securityClient() - .preparePutUser("user" + userNumber.getAndIncrement(), "password".toCharArray(), + .preparePutUser("user" + userNumber.getAndIncrement(), password, getFastStoredHashAlgoForTests(), randomAlphaOfLengthBetween(1, 16)) .request());