security: exclude the anonymous role from the xpack user

The calls made by the native users and roles store use the internal xpack user to make the request
and this user has a built-in role that has a single instance. A bug was introduced when fixing the logic
for applying the anonymous role to all users in elastic/elasticsearch#3716. The anonymous role was now being added to
the xpack user, even though the additional role would have no effect as this user is a superuser.

When the anonymous role is applied to the xpack user and exists as a native role or doesn't exist
at all, we run into a deadlock since we wind up querying for the role as a user that also has the
anonymous role.

This change special cases the XPackUser when getting the collection of roles so that the only role
applied to this user is the superuser role.

Closes elastic/elasticsearch#3822

Original commit: elastic/x-pack-elasticsearch@e3093904f1
This commit is contained in:
Jay Modi 2016-10-20 08:11:01 -04:00 committed by GitHub
parent 388bfd761d
commit 05886cdf9f
5 changed files with 109 additions and 10 deletions

View File

@ -61,7 +61,7 @@ public class InternalClient extends FilterClient {
try {
Authentication authentication = new Authentication(XPackUser.INSTANCE,
new Authentication.RealmRef("__attach", "__attach", nodeName), null);
authentication.writeToContextIfMissing(threadPool().getThreadContext(), cryptoService, signUserHeader);
authentication.writeToContext(threadPool().getThreadContext(), cryptoService, signUserHeader);
} catch (IOException ioe) {
throw new ElasticsearchException("failed to attach internal user to request", ioe);
}

View File

@ -97,7 +97,7 @@ public class Authentication {
return authentication;
}
public void writeToContextIfMissing(ThreadContext context, CryptoService cryptoService, boolean sign)
void writeToContextIfMissing(ThreadContext context, CryptoService cryptoService, boolean sign)
throws IOException, IllegalArgumentException {
if (context.getTransient(AUTHENTICATION_KEY) != null) {
if (context.getHeader(AUTHENTICATION_KEY) == null) {
@ -113,7 +113,11 @@ public class Authentication {
}
}
void writeToContext(ThreadContext ctx, CryptoService cryptoService, boolean sign)
/**
* Writes the authentication to the context. There must not be an existing authentication in the context and if there is an
* {@link IllegalStateException} will be thrown
*/
public void writeToContext(ThreadContext ctx, CryptoService cryptoService, boolean sign)
throws IOException, IllegalArgumentException {
ensureContextDoesNotContainAuthentication(ctx);
String header = encode();

View File

@ -126,6 +126,7 @@ public class AuthorizationService extends AbstractComponent {
// get the roles of the authenticated user, which may be different than the effective
Collection<Role> roles = roles(authentication.getUser());
GlobalPermission permission = permission(roles);
final boolean isRunAs = authentication.getUser() != authentication.getRunAsUser();
// permission can be empty as it might be that the user's role is unknown
if (permission.isEmpty()) {
@ -270,7 +271,8 @@ public class AuthorizationService extends AbstractComponent {
}
}
private GlobalPermission permission(Collection<Role> roles) {
// pkg-private for testing
GlobalPermission permission(Collection<Role> roles) {
GlobalPermission.Compound.Builder rolesBuilder = GlobalPermission.Compound.builder();
for (Role role : roles) {
rolesBuilder.add(role);
@ -279,6 +281,19 @@ public class AuthorizationService extends AbstractComponent {
}
Collection<Role> roles(User user) {
// we need to special case the internal users in this method, if we apply the anonymous roles to every user including these system
// user accounts then we run into the chance of a deadlock because then we need to get a role that we may be trying to get as the
// internal user. The SystemUser is special cased as it has special privileges to execute internal actions and should never be
// passed into this method. The XPackUser has the Superuser role and we can simply return that
if (SystemUser.is(user)) {
throw new IllegalArgumentException("the user [" + user.principal() + "] is the system user and we should never try to get its" +
" roles");
}
if (XPackUser.is(user)) {
assert XPackUser.INSTANCE.roles().length == 1 && SuperuserRole.NAME.equals(XPackUser.INSTANCE.roles()[0]);
return Collections.singleton(SuperuserRole.INSTANCE);
}
Set<String> roleNames = new HashSet<>();
Collections.addAll(roleNames, user.roles());
if (isAnonymousEnabled && anonymousUser.equals(user) == false) {

View File

@ -40,6 +40,7 @@ import org.elasticsearch.xpack.security.user.SystemUser;
import org.elasticsearch.xpack.security.user.User;
import org.elasticsearch.test.NativeRealmIntegTestCase;
import org.elasticsearch.test.SecuritySettingsSource;
import org.junit.Before;
import org.junit.BeforeClass;
import java.util.ArrayList;
@ -63,6 +64,8 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
private static boolean anonymousEnabled;
private boolean roleExists;
@BeforeClass
public static void init() {
anonymousEnabled = randomBoolean();
@ -72,12 +75,30 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
public Settings nodeSettings(int nodeOrdinal) {
if (anonymousEnabled) {
return Settings.builder().put(super.nodeSettings(nodeOrdinal))
.put(AnonymousUser.ROLES_SETTING.getKey(), SecuritySettingsSource.DEFAULT_ROLE)
.put(AnonymousUser.ROLES_SETTING.getKey(), "native_anonymous")
.build();
}
return super.nodeSettings(nodeOrdinal);
}
@Before
public void setupAnonymousRoleIfNecessary() throws Exception {
roleExists = anonymousEnabled && randomBoolean();
if (anonymousEnabled) {
if (roleExists) {
logger.info("anonymous is enabled. creating [native_anonymous] role");
PutRoleResponse response = securityClient()
.preparePutRole("native_anonymous")
.cluster("ALL")
.addIndices(new String[]{"*"}, new String[]{"ALL"}, new FieldPermissions(), null)
.get();
assertTrue(response.isCreated());
} else {
logger.info("anonymous is enabled, but configured with a missing role");
}
}
}
public void testDeletingNonexistingUserAndRole() throws Exception {
SecurityClient c = securityClient();
DeleteUserResponse resp = c.prepareDeleteUser("joe").get();
@ -317,7 +338,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
.addIndices(new String[]{"*"}, new String[]{"read"},
new FieldPermissions(new String[]{"body", "title"}, null), new BytesArray("{\"match_all\": {}}"))
.get();
if (anonymousEnabled) {
if (anonymousEnabled && roleExists) {
assertNoTimeout(client()
.filterWithHeader(Collections.singletonMap("Authorization", token)).admin().cluster().prepareHealth().get());
} else {
@ -361,7 +382,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
.prepareHealth().get();
assertFalse(response.isTimedOut());
c.prepareDeleteRole("test_role").get();
if (anonymousEnabled) {
if (anonymousEnabled && roleExists) {
assertNoTimeout(
client().filterWithHeader(Collections.singletonMap("Authorization", token)).admin().cluster().prepareHealth().get());
} else {
@ -412,7 +433,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
assertThat(joe.fullName(), is("Joe Smith"));
// test that role change took effect if anonymous is disabled as anonymous grants monitoring permissions...
if (anonymousEnabled) {
if (anonymousEnabled && roleExists) {
assertNoTimeout(
client().filterWithHeader(Collections.singletonMap("Authorization", token)).admin().cluster().prepareHealth().get());
} else {
@ -575,12 +596,12 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase {
public void testRolesUsageStats() throws Exception {
NativeRolesStore rolesStore = internalCluster().getInstance(NativeRolesStore.class);
long roles = anonymousEnabled && roleExists ? 1L: 0L;
Map<String, Object> usage = rolesStore.usageStats();
assertThat(usage.get("size"), is(0L));
assertEquals(roles, usage.get("size"));
assertThat(usage.get("fls"), is(false));
assertThat(usage.get("dls"), is(false));
long roles = 0;
final boolean fls = randomBoolean();
final boolean dls = randomBoolean();
SecurityClient client = new SecurityClient(client());

View File

@ -77,6 +77,8 @@ import org.elasticsearch.xpack.security.authc.Authentication.RealmRef;
import org.elasticsearch.xpack.security.authc.DefaultAuthenticationFailureHandler;
import org.elasticsearch.xpack.security.authz.accesscontrol.IndicesAccessControl;
import org.elasticsearch.xpack.security.authz.permission.DefaultRole;
import org.elasticsearch.xpack.security.authz.accesscontrol.IndicesAccessControl.IndexAccessControl;
import org.elasticsearch.xpack.security.authz.permission.GlobalPermission;
import org.elasticsearch.xpack.security.authz.permission.Role;
import org.elasticsearch.xpack.security.authz.permission.SuperuserRole;
import org.elasticsearch.xpack.security.authz.privilege.ClusterPrivilege;
@ -94,11 +96,14 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException;
import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationException;
import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationExceptionRunAs;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.either;
import static org.hamcrest.Matchers.equalTo;
@ -108,6 +113,7 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
public class AuthorizationServiceTests extends ESTestCase {
@ -780,6 +786,59 @@ public class AuthorizationServiceTests extends ESTestCase {
}
}
public void testDoesNotUseRolesStoreForXPackUser() {
Collection<Role> roles = authorizationService.roles(XPackUser.INSTANCE);
assertThat(roles, contains(SuperuserRole.INSTANCE));
verifyZeroInteractions(rolesStore);
}
public void testPermissionIncludesAnonymousUserPermissions() {
Settings settings = Settings.builder().put(AnonymousUser.ROLES_SETTING.getKey(), "a_all").build();
final AnonymousUser anonymousUser = new AnonymousUser(settings);
authorizationService = new AuthorizationService(settings, rolesStore, clusterService, auditTrail,
new DefaultAuthenticationFailureHandler(), threadPool, anonymousUser);
final boolean roleExists = randomBoolean();
final Role anonymousRole = Role.builder("a_all").add(IndexPrivilege.ALL, "a").build();
if (roleExists) {
when(rolesStore.role("a_all")).thenReturn(anonymousRole);
}
final MetaData metaData = MetaData.builder()
.put(new IndexMetaData.Builder("a")
.settings(Settings.builder().put("index.version.created", Version.CURRENT).build())
.numberOfShards(1).numberOfReplicas(0).build(), true)
.build();
User user = new User("no_roles");
final Collection<Role> roles = authorizationService.roles(user);
GlobalPermission globalPermission = authorizationService.permission(roles);
verify(rolesStore).role("a_all");
if (roleExists) {
assertThat(roles, containsInAnyOrder(anonymousRole, DefaultRole.INSTANCE));
assertFalse(globalPermission.isEmpty());
// by default all users have a DefaultRole that grants cluster actions like change password
assertFalse(globalPermission.cluster().isEmpty());
assertFalse(globalPermission.indices().isEmpty());
Map<String, IndexAccessControl> authzMap =
globalPermission.indices().authorize(SearchAction.NAME, Collections.singleton("a"), metaData);
assertTrue(authzMap.containsKey("a"));
assertTrue(authzMap.get("a").isGranted());
assertFalse(authzMap.get("a").getFieldPermissions().hasFieldLevelSecurity());
assertNull(authzMap.get("a").getQueries());
} else {
assertThat(roles, contains(DefaultRole.INSTANCE));
assertFalse(globalPermission.isEmpty());
// by default all users have a DefaultRole that grants cluster actions like change password
assertFalse(globalPermission.cluster().isEmpty());
assertTrue(globalPermission.indices().isEmpty());
}
}
public void testGetRolesForSystemUserThrowsException() {
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> authorizationService.roles(SystemUser.INSTANCE));
assertEquals("the user [_system] is the system user and we should never try to get its roles", iae.getMessage());
}
private static Authentication createAuthentication(User user) {
RealmRef lookedUpBy = user.runAs() == null ? null : new RealmRef("looked", "up", "by");
return new Authentication(user, new RealmRef("test", "test", "foo"), lookedUpBy);