From 40bf58e2a035dd497a19ed4f87b3741e18806301 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Fri, 29 Jun 2018 10:25:45 +0300 Subject: [PATCH] Fix RealmInteg test failures As part of the changes in #31234,the password verification logic determines the algorithm used for hashing the password from the format of the stored password hash itself. Thus, it is generally possible to validate a password even if it's associated stored hash was not created with the same algorithm than the one currently set in the settings. At the same time, we introduced a check for incoming client change password requests to make sure that the request's password is hashed with the same algorithm that is configured to be used in the node settings. In the spirit of randomizing the algorithms used, the {@code SecurityClient} used in the {@code NativeRealmIntegTests} and {@code ReservedRealmIntegTests} would send all requests dealing with user passwords by randomly selecting a hashing algorithm each time. This meant that some change password requests were using a different password hashing algorithm than the one used for the node and the request would fail. This commit changes this behavior in the two aforementioned Integ tests to use the same password hashing algorithm for the node and the clients, no matter what the request is. Resolves #31670 --- .../authc/esnative/NativeRealmIntegTests.java | 69 ++++++++++--------- .../esnative/ReservedRealmIntegTests.java | 21 +++++- 2 files changed, 56 insertions(+), 34 deletions(-) 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 81c85c1a5f828..be663e77a468c 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 @@ -35,6 +35,7 @@ import org.elasticsearch.xpack.core.security.action.user.ChangePasswordResponse; import org.elasticsearch.xpack.core.security.action.user.DeleteUserResponse; import org.elasticsearch.xpack.core.security.action.user.GetUsersResponse; +import org.elasticsearch.xpack.core.security.authc.support.Hasher; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.permission.Role; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; @@ -72,22 +73,24 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { private static boolean anonymousEnabled; + private static Hasher hasher; private boolean roleExists; @BeforeClass public static void init() { anonymousEnabled = randomBoolean(); + hasher = getFastStoredHashAlgoForTests(); } @Override public Settings nodeSettings(int nodeOrdinal) { + Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)) + .put("xpack.security.authc.password_hashing.algorithm", hasher.name()); if (anonymousEnabled) { - return Settings.builder().put(super.nodeSettings(nodeOrdinal)) - .put(AnonymousUser.ROLES_SETTING.getKey(), "native_anonymous") - .build(); + builder.put(AnonymousUser.ROLES_SETTING.getKey(), "native_anonymous"); } - return super.nodeSettings(nodeOrdinal); + return builder.build(); } @Before @@ -111,7 +114,7 @@ public void setupAnonymousRoleIfNecessary() throws Exception { public void testDeletingNonexistingUserAndRole() throws Exception { SecurityClient c = securityClient(); // first create the index so it exists - c.preparePutUser("joe", "s3kirt".toCharArray(), getFastStoredHashAlgoForTests(), "role1", "user").get(); + c.preparePutUser("joe", "s3kirt".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(); @@ -131,7 +134,7 @@ public void testAddAndGetUser() throws Exception { final List existingUsers = Arrays.asList(c.prepareGetUsers().get().users()); final int existing = existingUsers.size(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3kirt".toCharArray(), getFastStoredHashAlgoForTests(), "role1", "user").get(); + c.preparePutUser("joe", "s3kirt".toCharArray(), hasher, "role1", "user").get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_INDEX_NAME); logger.info("--> retrieving user"); @@ -142,8 +145,8 @@ public void testAddAndGetUser() throws Exception { assertArrayEquals(joe.roles(), new String[]{"role1", "user"}); logger.info("--> adding two more users"); - c.preparePutUser("joe2", "s3kirt2".toCharArray(), getFastStoredHashAlgoForTests(), "role2", "user").get(); - c.preparePutUser("joe3", "s3kirt3".toCharArray(), getFastStoredHashAlgoForTests(), "role3", "user").get(); + c.preparePutUser("joe2", "s3kirt2".toCharArray(), hasher, "role2", "user").get(); + c.preparePutUser("joe3", "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); @@ -237,7 +240,7 @@ public void testAddUserAndRoleThenAuth() throws Exception { new BytesArray("{\"match_all\": {}}")) .get(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), "test_role").get(); + c.preparePutUser("joe", "s3krit".toCharArray(), hasher, "test_role").get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_INDEX_NAME); logger.info("--> retrieving user"); @@ -258,7 +261,7 @@ public void testAddUserAndRoleThenAuth() throws Exception { public void testUpdatingUserAndAuthentication() throws Exception { SecurityClient c = securityClient(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), SecuritySettingsSource.TEST_ROLE).get(); + c.preparePutUser("joe", "s3krit".toCharArray(), hasher, SecuritySettingsSource.TEST_ROLE).get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_INDEX_NAME); logger.info("--> retrieving user"); @@ -275,7 +278,7 @@ public void testUpdatingUserAndAuthentication() throws Exception { assertEquals(1L, searchResp.getHits().getTotalHits()); - c.preparePutUser("joe", "s3krit2".toCharArray(), getFastStoredHashAlgoForTests(), SecuritySettingsSource.TEST_ROLE).get(); + c.preparePutUser("joe", "s3krit2".toCharArray(), hasher, SecuritySettingsSource.TEST_ROLE).get(); try { client().filterWithHeader(Collections.singletonMap("Authorization", token)).prepareSearch("idx").get(); @@ -293,7 +296,7 @@ public void testUpdatingUserAndAuthentication() throws Exception { public void testCreateDeleteAuthenticate() { SecurityClient c = securityClient(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), + c.preparePutUser("joe", "s3krit".toCharArray(), hasher, SecuritySettingsSource.TEST_ROLE).get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_INDEX_NAME); @@ -332,7 +335,7 @@ public void testCreateAndUpdateRole() { new BytesArray("{\"match_all\": {}}")) .get(); logger.error("--> creating user"); - c.preparePutUser("joe", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), "test_role").get(); + c.preparePutUser("joe", "s3krit".toCharArray(), hasher, "test_role").get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_INDEX_NAME); @@ -381,7 +384,7 @@ public void testAuthenticateWithDeletedRole() { .addIndices(new String[]{"*"}, new String[]{"read"}, new String[]{"body", "title"}, null, new BytesArray("{\"match_all\": {}}")) .get(); - c.preparePutUser("joe", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), "test_role").get(); + c.preparePutUser("joe", "s3krit".toCharArray(), hasher, "test_role").get(); logger.error("--> waiting for .security index"); ensureGreen(SECURITY_INDEX_NAME); @@ -415,7 +418,7 @@ public void testPutUserWithoutPassword() { 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, getFastStoredHashAlgoForTests(), "admin_role").get(); + client.preparePutUser("joe", null, hasher, "admin_role").get(); fail("cannot create a user without a password"); } catch (IllegalArgumentException e) { assertThat(e.getMessage(), containsString("password must be specified")); @@ -425,7 +428,7 @@ public void testPutUserWithoutPassword() { // create joe with a password and verify the user works client.preparePutUser("joe", SecuritySettingsSourceField.TEST_PASSWORD.toCharArray(), - getFastStoredHashAlgoForTests(), "admin_role").get(); + 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() @@ -433,7 +436,7 @@ public void testPutUserWithoutPassword() { assertFalse(response.isTimedOut()); // modify joe without sending the password - client.preparePutUser("joe", null, getFastStoredHashAlgoForTests(), "read_role").fullName("Joe Smith").get(); + client.preparePutUser("joe", 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)); @@ -454,7 +457,7 @@ public void testPutUserWithoutPassword() { // update the user with password and admin role again String secondPassword = SecuritySettingsSourceField.TEST_PASSWORD + "2"; - client.preparePutUser("joe", secondPassword.toCharArray(), getFastStoredHashAlgoForTests(), "admin_role"). + client.preparePutUser("joe", secondPassword.toCharArray(), hasher, "admin_role"). fullName("Joe Smith").get(); getUsersResponse = client.prepareGetUsers("joe").get(); assertThat(getUsersResponse.hasUsers(), is(true)); @@ -483,7 +486,7 @@ public void testPutUserWithoutPassword() { public void testCannotCreateUserWithShortPassword() throws Exception { SecurityClient client = securityClient(); try { - client.preparePutUser("joe", randomAlphaOfLengthBetween(0, 5).toCharArray(), getFastStoredHashAlgoForTests(), + client.preparePutUser("joe", randomAlphaOfLengthBetween(0, 5).toCharArray(), hasher, "admin_role").get(); fail("cannot create a user without a password < 6 characters"); } catch (ValidationException v) { @@ -494,7 +497,7 @@ public void testCannotCreateUserWithShortPassword() throws Exception { public void testCannotCreateUserWithInvalidCharactersInName() throws Exception { SecurityClient client = securityClient(); ValidationException v = expectThrows(ValidationException.class, - () -> client.preparePutUser("fóóbár", "my-am@zing-password".toCharArray(), getFastStoredHashAlgoForTests(), + () -> client.preparePutUser("fóóbár", "my-am@zing-password".toCharArray(), hasher, "admin_role").get() ); assertThat(v.getMessage(), containsString("names must be")); @@ -505,7 +508,7 @@ public void testUsersAndRolesDoNotInterfereWithIndicesStats() throws Exception { SecurityClient client = securityClient(); if (randomBoolean()) { - client.preparePutUser("joe", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), + client.preparePutUser("joe", "s3krit".toCharArray(), hasher, SecuritySettingsSource.TEST_ROLE).get(); } else { client.preparePutRole("read_role") @@ -526,7 +529,7 @@ 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, getFastStoredHashAlgoForTests(), "admin").get()); + : null, hasher, "admin").get()); assertThat(exception.getMessage(), containsString("Username [" + username + "] is reserved")); exception = expectThrows(IllegalArgumentException.class, @@ -539,21 +542,21 @@ public void testOperationsOnReservedUsers() throws Exception { exception = expectThrows(IllegalArgumentException.class, () -> securityClient().prepareChangePassword(AnonymousUser.DEFAULT_ANONYMOUS_USERNAME, "foobar".toCharArray(), - getFastStoredHashAlgoForTests()).get()); + 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(), - getFastStoredHashAlgoForTests()).get()); + hasher).get()); assertThat(exception.getMessage(), containsString("Username [" + AnonymousUser.DEFAULT_ANONYMOUS_USERNAME + "] is reserved")); exception = expectThrows(IllegalArgumentException.class, - () -> securityClient().preparePutUser(SystemUser.NAME, "foobar".toCharArray(), getFastStoredHashAlgoForTests()).get()); + () -> securityClient().preparePutUser(SystemUser.NAME, "foobar".toCharArray(), hasher).get()); assertThat(exception.getMessage(), containsString("user [" + SystemUser.NAME + "] is internal")); exception = expectThrows(IllegalArgumentException.class, () -> securityClient().prepareChangePassword(SystemUser.NAME, "foobar".toCharArray(), - getFastStoredHashAlgoForTests()).get()); + hasher).get()); assertThat(exception.getMessage(), containsString("user [" + SystemUser.NAME + "] is internal")); exception = expectThrows(IllegalArgumentException.class, @@ -592,7 +595,7 @@ public void testOperationsOnReservedRoles() throws Exception { @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/31670") public void testCreateAndChangePassword() throws Exception { - securityClient().preparePutUser("joe", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), + securityClient().preparePutUser("joe", "s3krit".toCharArray(), hasher, SecuritySettingsSource.TEST_ROLE).get(); final String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); ClusterHealthResponse response = client().filterWithHeader(Collections.singletonMap("Authorization", token)) @@ -601,7 +604,7 @@ public void testCreateAndChangePassword() throws Exception { ChangePasswordResponse passwordResponse = securityClient( client().filterWithHeader(Collections.singletonMap("Authorization", token))) - .prepareChangePassword("joe", SecuritySettingsSourceField.TEST_PASSWORD.toCharArray(), getFastStoredHashAlgoForTests()).get(); + .prepareChangePassword("joe", SecuritySettingsSourceField.TEST_PASSWORD.toCharArray(), hasher).get(); assertThat(passwordResponse, notNullValue()); @@ -681,7 +684,7 @@ public void testRealmUsageStats() { final int numNativeUsers = scaledRandomIntBetween(1, 32); SecurityClient securityClient = new SecurityClient(client()); for (int i = 0; i < numNativeUsers; i++) { - securityClient.preparePutUser("joe" + i, "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), + securityClient.preparePutUser("joe" + i, "s3krit".toCharArray(), hasher, "superuser").get(); } @@ -702,7 +705,7 @@ public void testRealmUsageStats() { public void testSetEnabled() throws Exception { - securityClient().preparePutUser("joe", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), + securityClient().preparePutUser("joe", "s3krit".toCharArray(), hasher, SecuritySettingsSource.TEST_ROLE).get(); final String token = basicAuthHeaderValue("joe", new SecureString("s3krit".toCharArray())); ClusterHealthResponse response = client().filterWithHeader(Collections.singletonMap("Authorization", token)) @@ -727,7 +730,7 @@ public void testSetEnabled() throws Exception { public void testNegativeLookupsThenCreateRole() throws Exception { SecurityClient securityClient = new SecurityClient(client()); - securityClient.preparePutUser("joe", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), "unknown_role").get(); + securityClient.preparePutUser("joe", "s3krit".toCharArray(), hasher, "unknown_role").get(); final int negativeLookups = scaledRandomIntBetween(1, 10); for (int i = 0; i < negativeLookups; i++) { @@ -763,9 +766,9 @@ public void testNegativeLookupsThenCreateRole() throws Exception { * 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(), getFastStoredHashAlgoForTests(), SecuritySettingsSource + securityClient().preparePutUser("joe", "s3krit".toCharArray(), hasher, SecuritySettingsSource .TEST_ROLE).get(); - securityClient().preparePutUser("executor", "s3krit".toCharArray(), getFastStoredHashAlgoForTests(), "superuser").get(); + securityClient().preparePutUser("executor", "s3krit".toCharArray(), hasher, "superuser").get(); final String token = basicAuthHeaderValue("executor", new SecureString("s3krit".toCharArray())); final Client client = client().filterWithHeader(MapBuilder.newMapBuilder() .put("Authorization", token) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmIntegTests.java index ae1317fa85adc..5f46b69082095 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmIntegTests.java @@ -8,13 +8,16 @@ import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.NativeRealmIntegTestCase; import org.elasticsearch.xpack.core.security.action.user.ChangePasswordResponse; +import org.elasticsearch.xpack.core.security.authc.support.Hasher; import org.elasticsearch.xpack.core.security.client.SecurityClient; import org.elasticsearch.xpack.core.security.user.BeatsSystemUser; import org.elasticsearch.xpack.core.security.user.ElasticUser; import org.elasticsearch.xpack.core.security.user.KibanaUser; import org.elasticsearch.xpack.core.security.user.LogstashSystemUser; +import org.junit.BeforeClass; import java.util.Arrays; @@ -29,6 +32,22 @@ */ public class ReservedRealmIntegTests extends NativeRealmIntegTestCase { + private static Hasher hasher; + + @BeforeClass + public static void setHasher() { + hasher = getFastStoredHashAlgoForTests(); + } + + @Override + public Settings nodeSettings(int nodeOrdinal) { + Settings settings = Settings.builder() + .put(super.nodeSettings(nodeOrdinal)) + .put("xpack.security.authc.password_hashing.algorithm", hasher.name()) + .build(); + return settings; + } + public void testAuthenticate() { for (String username : Arrays.asList(ElasticUser.NAME, KibanaUser.NAME, LogstashSystemUser.NAME, BeatsSystemUser.NAME)) { ClusterHealthResponse response = client() @@ -77,7 +96,7 @@ public void testChangingPassword() { } ChangePasswordResponse response = securityClient() - .prepareChangePassword(username, Arrays.copyOf(newPassword, newPassword.length), getFastStoredHashAlgoForTests()) + .prepareChangePassword(username, Arrays.copyOf(newPassword, newPassword.length), hasher) .get(); assertThat(response, notNullValue());