From 68eb4d981e42bd0f300579f13fb045ec11bb7165 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Wed, 12 Oct 2016 11:42:02 -0400 Subject: [PATCH] security: wildcards for superusers includes the security index The superuser role is the only user assignable role that grants access to the .security index, but when resolving wildcards the index was not getting resolved. The resolution of indices and aliases explicitly excludes the .security index for users that are not the internal user without checking if the user has the superuser role. This commit adds a check in for the superuser role. Original commit: elastic/x-pack-elasticsearch@02ee0a87408c646b1495097a0bf478339dc3a39b --- .../security/authz/AuthorizationService.java | 2 +- .../authz/AuthorizationServiceTests.java | 75 +++++++++++++------ 2 files changed, 54 insertions(+), 23 deletions(-) diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index 5d6116b1595..1dc602e9d52 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -138,7 +138,7 @@ public class AuthorizationService extends AbstractComponent { } } - if (XPackUser.is(user) == false) { + if (XPackUser.is(user) == false && Arrays.binarySearch(user.roles(), SuperuserRole.NAME) < 0) { // we should filter out the .security index from wildcards if (indicesAndAliases.remove(SecurityTemplateService.SECURITY_INDEX_NAME)) { logger.debug("removed [{}] from user [{}] list of authorized indices", diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index 356cd4192b5..6161ddbc354 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -56,6 +56,7 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.action.search.SearchTransportService; +import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportRequest; @@ -77,10 +78,12 @@ import org.elasticsearch.xpack.security.user.XPackUser; import org.junit.Before; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException; import static org.elasticsearch.test.SecurityTestsUtils.assertAuthorizationException; +import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; @@ -543,6 +546,11 @@ public class AuthorizationServiceTests extends ESTestCase { request = new ClusterHealthRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "foo", "bar"); authorizationService.authorize(createAuthentication(user), ClusterHealthAction.NAME, request); verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request); + + SearchRequest searchRequest = new SearchRequest("_all"); + IndexNotFoundException e = expectThrows(IndexNotFoundException.class, + () -> authorizationService.authorize(createAuthentication(user), SearchAction.NAME, searchRequest)); + assertThat(e.getMessage(), containsString("no such index")); } public void testGrantedNonXPackUserCanExecuteMonitoringOperationsAgainstSecurityIndex() { @@ -589,29 +597,29 @@ public class AuthorizationServiceTests extends ESTestCase { .numberOfShards(1).numberOfReplicas(0).build(), true) .build()); - List> requests = new ArrayList<>(); - requests.add(new Tuple<>(DeleteAction.NAME, new DeleteRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); - requests.add(new Tuple<>(UpdateAction.NAME, new UpdateRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); - requests.add(new Tuple<>(IndexAction.NAME, new IndexRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); - requests.add(new Tuple<>(SearchAction.NAME, new SearchRequest(SecurityTemplateService.SECURITY_INDEX_NAME))); - requests.add(new Tuple<>(TermVectorsAction.NAME, - new TermVectorsRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); - requests.add(new Tuple<>(GetAction.NAME, new GetRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); - requests.add(new Tuple<>(TermVectorsAction.NAME, - new TermVectorsRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); - requests.add(new Tuple<>(IndicesAliasesAction.NAME, new IndicesAliasesRequest() - .addAliasAction(AliasActions.add().alias("security_alias").index(SecurityTemplateService.SECURITY_INDEX_NAME)))); - requests.add(new Tuple<>(ClusterHealthAction.NAME, new ClusterHealthRequest(SecurityTemplateService.SECURITY_INDEX_NAME))); - requests.add(new Tuple<>(ClusterHealthAction.NAME, - new ClusterHealthRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "foo", "bar"))); + for (User user : Arrays.asList(XPackUser.INSTANCE, superuser)) { + List> requests = new ArrayList<>(); + requests.add(new Tuple<>(DeleteAction.NAME, new DeleteRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); + requests.add(new Tuple<>(UpdateAction.NAME, new UpdateRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); + requests.add(new Tuple<>(IndexAction.NAME, new IndexRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); + requests.add(new Tuple<>(SearchAction.NAME, new SearchRequest(SecurityTemplateService.SECURITY_INDEX_NAME))); + requests.add(new Tuple<>(TermVectorsAction.NAME, + new TermVectorsRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); + requests.add(new Tuple<>(GetAction.NAME, new GetRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); + requests.add(new Tuple<>(TermVectorsAction.NAME, + new TermVectorsRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "type", "id"))); + requests.add(new Tuple<>(IndicesAliasesAction.NAME, new IndicesAliasesRequest() + .addAliasAction(AliasActions.add().alias("security_alias").index(SecurityTemplateService.SECURITY_INDEX_NAME)))); + requests.add(new Tuple<>(ClusterHealthAction.NAME, new ClusterHealthRequest(SecurityTemplateService.SECURITY_INDEX_NAME))); + requests.add(new Tuple<>(ClusterHealthAction.NAME, + new ClusterHealthRequest(SecurityTemplateService.SECURITY_INDEX_NAME, "foo", "bar"))); - for (Tuple requestTuple : requests) { - String action = requestTuple.v1(); - TransportRequest request = requestTuple.v2(); - authorizationService.authorize(createAuthentication(XPackUser.INSTANCE), action, request); - verify(auditTrail).accessGranted(XPackUser.INSTANCE, action, request); - authorizationService.authorize(createAuthentication(superuser), action, request); - verify(auditTrail).accessGranted(superuser, action, request); + for (Tuple requestTuple : requests) { + String action = requestTuple.v1(); + TransportRequest request = requestTuple.v2(); + authorizationService.authorize(createAuthentication(user), action, request); + verify(auditTrail).accessGranted(user, action, request); + } } } @@ -641,6 +649,29 @@ public class AuthorizationServiceTests extends ESTestCase { authorizationService.authorize(createAuthentication(userWithNoRoles), IndicesExistsAction.NAME, new IndicesExistsRequest("a")); } + public void testXPackUserAndSuperusersCanExecuteOperationAgainstSecurityIndexWithWildcard() { + final User superuser = new User("custom_admin", SuperuserRole.NAME); + when(rolesStore.role(SuperuserRole.NAME)).thenReturn(Role.builder(SuperuserRole.DESCRIPTOR).build()); + ClusterState state = mock(ClusterState.class); + when(clusterService.state()).thenReturn(state); + when(state.metaData()).thenReturn(MetaData.builder() + .put(new IndexMetaData.Builder(SecurityTemplateService.SECURITY_INDEX_NAME) + .settings(Settings.builder().put("index.version.created", Version.CURRENT).build()) + .numberOfShards(1).numberOfReplicas(0).build(), true) + .build()); + + String action = SearchAction.NAME; + SearchRequest request = new SearchRequest("_all"); + authorizationService.authorize(createAuthentication(XPackUser.INSTANCE), action, request); + verify(auditTrail).accessGranted(XPackUser.INSTANCE, action, request); + assertThat(request.indices(), arrayContaining(".security")); + + request = new SearchRequest("_all"); + authorizationService.authorize(createAuthentication(superuser), action, request); + verify(auditTrail).accessGranted(superuser, action, request); + assertThat(request.indices(), arrayContaining(".security")); + } + private 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);