Skip to content

Commit

Permalink
Security: fix dynamic mapping updates with aliases (#30787)
Browse files Browse the repository at this point in the history
This commit fixes an issue with dynamic mapping updates when an index
operation is performed against an alias and when the user only has
permissions to the alias. Dynamic mapping updates resolve the concrete
index early to prevent issues so the information about the alias that
the triggering operation was being executed against is lost. When
security is enabled and a user only has privileges to the alias, this
dynamic mapping update would be rejected as it is executing against the
concrete index and not the alias. In order to handle this situation,
the security code needs to look at the concrete index and the
authorized indices of the user; if the concrete index is not authorized
the code will attempt to find an alias that the user has permissions to
update the mappings of.

Closes #30597
  • Loading branch information
jaymode committed May 24, 2018
1 parent 44c6882 commit d1c0d41
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 10 deletions.
Expand Up @@ -14,11 +14,14 @@
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.cluster.metadata.AliasMetaData;
import org.elasticsearch.cluster.metadata.AliasOrIndex;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Settings;
Expand All @@ -35,14 +38,15 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.SortedMap;
import java.util.concurrent.CopyOnWriteArraySet;
import java.util.stream.Collectors;

import static org.elasticsearch.xpack.core.security.authz.IndicesAndAliasesResolverField.NO_INDEX_PLACEHOLDER;

public class IndicesAndAliasesResolver {
class IndicesAndAliasesResolver {

//`*,-*` what we replace indices with if we need Elasticsearch to return empty responses without throwing exception
private static final String[] NO_INDICES_ARRAY = new String[] { "*", "-*" };
Expand All @@ -51,7 +55,7 @@ public class IndicesAndAliasesResolver {
private final IndexNameExpressionResolver nameExpressionResolver;
private final RemoteClusterResolver remoteClusterResolver;

public IndicesAndAliasesResolver(Settings settings, ClusterService clusterService) {
IndicesAndAliasesResolver(Settings settings, ClusterService clusterService) {
this.nameExpressionResolver = new IndexNameExpressionResolver(settings);
this.remoteClusterResolver = new RemoteClusterResolver(settings, clusterService.getClusterSettings());
}
Expand Down Expand Up @@ -85,7 +89,7 @@ public IndicesAndAliasesResolver(Settings settings, ClusterService clusterServic
* Otherwise, <em>N</em> will be added to the <em>local</em> index list.
*/

public ResolvedIndices resolve(TransportRequest request, MetaData metaData, AuthorizedIndices authorizedIndices) {
ResolvedIndices resolve(TransportRequest request, MetaData metaData, AuthorizedIndices authorizedIndices) {
if (request instanceof IndicesAliasesRequest) {
ResolvedIndices.Builder resolvedIndicesBuilder = new ResolvedIndices.Builder();
IndicesAliasesRequest indicesAliasesRequest = (IndicesAliasesRequest) request;
Expand Down Expand Up @@ -116,7 +120,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
*/
assert indicesRequest.indices() == null || indicesRequest.indices().length == 0
: "indices are: " + Arrays.toString(indicesRequest.indices()); // Arrays.toString() can handle null values - all good
resolvedIndicesBuilder.addLocal(((PutMappingRequest) indicesRequest).getConcreteIndex().getName());
resolvedIndicesBuilder.addLocal(getPutMappingIndexOrAlias((PutMappingRequest) indicesRequest, authorizedIndices, metaData));
} else if (indicesRequest instanceof IndicesRequest.Replaceable) {
IndicesRequest.Replaceable replaceable = (IndicesRequest.Replaceable) indicesRequest;
final boolean replaceWildcards = indicesRequest.indicesOptions().expandWildcardsOpen()
Expand Down Expand Up @@ -213,7 +217,48 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
return resolvedIndicesBuilder.build();
}

public static boolean allowsRemoteIndices(IndicesRequest request) {
/**
* Special handling of the value to authorize for a put mapping request. Dynamic put mapping
* requests use a concrete index, but we allow permissions to be defined on aliases so if the
* request's concrete index is not in the list of authorized indices, then we need to look to
* see if this can be authorized against an alias
*/
static String getPutMappingIndexOrAlias(PutMappingRequest request, AuthorizedIndices authorizedIndices, MetaData metaData) {
final String concreteIndexName = request.getConcreteIndex().getName();
final List<String> authorizedIndicesList = authorizedIndices.get();

// validate that the concrete index exists, otherwise there is no remapping that we could do
final AliasOrIndex aliasOrIndex = metaData.getAliasAndIndexLookup().get(concreteIndexName);
final String resolvedAliasOrIndex;
if (aliasOrIndex == null) {
resolvedAliasOrIndex = concreteIndexName;
} else if (aliasOrIndex.isAlias()) {
throw new IllegalStateException("concrete index [" + concreteIndexName + "] is an alias but should not be");
} else if (authorizedIndicesList.contains(concreteIndexName)) {
// user is authorized to put mappings for this index
resolvedAliasOrIndex = concreteIndexName;
} else {
// the user is not authorized to put mappings for this index, but could have been
// authorized for a write using an alias that triggered a dynamic mapping update
ImmutableOpenMap<String, List<AliasMetaData>> foundAliases =
metaData.findAliases(Strings.EMPTY_ARRAY, new String[] { concreteIndexName });
List<AliasMetaData> aliasMetaData = foundAliases.get(concreteIndexName);
if (aliasMetaData != null) {
Optional<String> foundAlias = aliasMetaData.stream()
.map(AliasMetaData::alias)
.filter(authorizedIndicesList::contains)
.filter(aliasName -> metaData.getAliasAndIndexLookup().get(aliasName).getIndices().size() == 1)
.findFirst();
resolvedAliasOrIndex = foundAlias.orElse(concreteIndexName);
} else {
resolvedAliasOrIndex = concreteIndexName;
}
}

return resolvedAliasOrIndex;
}

static boolean allowsRemoteIndices(IndicesRequest request) {
return request instanceof SearchRequest || request instanceof FieldCapabilitiesRequest
|| request instanceof GraphExploreRequest;
}
Expand Down
Expand Up @@ -39,10 +39,12 @@
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.search.internal.ShardSearchTransportRequest;
import org.elasticsearch.test.ESTestCase;
Expand Down Expand Up @@ -149,7 +151,10 @@ public void setup() {
new IndicesPrivileges[] { IndicesPrivileges.builder().indices(authorizedIndices).privileges("all").build() }, null));
roleMap.put("dash", new RoleDescriptor("dash", null,
new IndicesPrivileges[] { IndicesPrivileges.builder().indices(dashIndices).privileges("all").build() }, null));
roleMap.put("test", new RoleDescriptor("role", new String[] { "monitor" }, null, null));
roleMap.put("test", new RoleDescriptor("test", new String[] { "monitor" }, null, null));
roleMap.put("alias_read_write", new RoleDescriptor("alias_read_write", null,
new IndicesPrivileges[] { IndicesPrivileges.builder().indices("barbaz", "foofoobar").privileges("read", "write").build() },
null));
roleMap.put(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName(), ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR);
final FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY);
doAnswer((i) -> {
Expand Down Expand Up @@ -651,7 +656,7 @@ public void testResolveWildcardsIndicesAliasesRequestNoMatchingIndices() {
request.addAliasAction(AliasActions.add().alias("alias2").index("bar*"));
request.addAliasAction(AliasActions.add().alias("alias3").index("non_matching_*"));
//if a single operation contains wildcards and ends up being resolved to no indices, it makes the whole request fail
expectThrows(IndexNotFoundException.class,
expectThrows(IndexNotFoundException.class,
() -> resolveIndices(request, buildAuthorizedIndices(user, IndicesAliasesAction.NAME)));
}

Expand Down Expand Up @@ -1180,10 +1185,10 @@ public void testIndicesExists() {
assertNoIndices(request, resolveIndices(request,
buildAuthorizedIndices(userNoIndices, IndicesExistsAction.NAME)));
}

{
IndicesExistsRequest request = new IndicesExistsRequest("does_not_exist");

assertNoIndices(request, resolveIndices(request,
buildAuthorizedIndices(user, IndicesExistsAction.NAME)));
}
Expand Down Expand Up @@ -1228,7 +1233,7 @@ public void testNonXPackUserAccessingSecurityIndex() {
List<String> indices = resolveIndices(request, authorizedIndices).getLocal();
assertThat(indices, not(hasItem(SecurityIndexManager.SECURITY_INDEX_NAME)));
}

{
IndicesAliasesRequest aliasesRequest = new IndicesAliasesRequest();
aliasesRequest.addAliasAction(AliasActions.add().alias("security_alias1").index("*"));
Expand Down Expand Up @@ -1317,6 +1322,21 @@ public void testAliasDateMathExpressionNotSupported() {
assertThat(request.aliases(), arrayContainingInAnyOrder("<datetime-{now/M}>"));
}

public void testDynamicPutMappingRequestFromAlias() {
PutMappingRequest request = new PutMappingRequest(Strings.EMPTY_ARRAY).setConcreteIndex(new Index("foofoo", UUIDs.base64UUID()));
User user = new User("alias-writer", "alias_read_write");
AuthorizedIndices authorizedIndices = buildAuthorizedIndices(user, PutMappingAction.NAME);

String putMappingIndexOrAlias = IndicesAndAliasesResolver.getPutMappingIndexOrAlias(request, authorizedIndices, metaData);
assertEquals("barbaz", putMappingIndexOrAlias);

// multiple indices map to an alias so we can only return the concrete index
final String index = randomFrom("foo", "foobar");
request = new PutMappingRequest(Strings.EMPTY_ARRAY).setConcreteIndex(new Index(index, UUIDs.base64UUID()));
putMappingIndexOrAlias = IndicesAndAliasesResolver.getPutMappingIndexOrAlias(request, authorizedIndices, metaData);
assertEquals(index, putMappingIndexOrAlias);
}

// TODO with the removal of DeleteByQuery is there another way to test resolving a write action?


Expand Down
@@ -0,0 +1,90 @@
---
setup:
- skip:
features: headers

- do:
cluster.health:
wait_for_status: yellow

- do:
xpack.security.put_role:
name: "alias_write_role"
body: >
{
"indices": [
{ "names": ["write_alias"], "privileges": ["write"] }
]
}
- do:
xpack.security.put_user:
username: "test_user"
body: >
{
"password" : "x-pack-test-password",
"roles" : [ "alias_write_role" ],
"full_name" : "user with privileges to write via alias"
}
- do:
indices.create:
index: write_index_1
body:
settings:
index:
number_of_shards: 1
number_of_replicas: 0

- do:
indices.put_alias:
index: write_index_1
name: write_alias

---
teardown:
- do:
xpack.security.delete_user:
username: "test_user"
ignore: 404

- do:
xpack.security.delete_role:
name: "alias_write_role"
ignore: 404

- do:
indices.delete_alias:
index: "write_index_1"
name: [ "write_alias" ]
ignore: 404

- do:
indices.delete:
index: [ "write_index_1" ]
ignore: 404

---
"Test indexing documents into an alias with dynamic mappings":

- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
create:
id: 1
index: write_alias
type: doc
body: >
{
"name" : "doc1"
}
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
create:
id: 2
index: write_alias
type: doc
body: >
{
"name2" : "doc2"
}

0 comments on commit d1c0d41

Please sign in to comment.