Skip to content

Commit

Permalink
Fix RealmInteg test failures
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jkakavas committed Jun 29, 2018
1 parent b7b413e commit 40bf58e
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -131,7 +134,7 @@ public void testAddAndGetUser() throws Exception {
final List<User> 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");
Expand All @@ -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);
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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"));
Expand All @@ -425,15 +428,15 @@ 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()
.prepareHealth().get();
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));
Expand All @@ -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));
Expand Down Expand Up @@ -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) {
Expand All @@ -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"));
Expand All @@ -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")
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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))
Expand All @@ -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());


Expand Down Expand Up @@ -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();
}

Expand All @@ -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))
Expand All @@ -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++) {
Expand Down Expand Up @@ -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.<String, String>newMapBuilder()
.put("Authorization", token)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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()
Expand Down Expand Up @@ -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());

Expand Down

0 comments on commit 40bf58e

Please sign in to comment.