diff --git a/x-pack/plugin/core/src/main/java/module-info.java b/x-pack/plugin/core/src/main/java/module-info.java index ca5f8406fc97c..88dc179fed592 100644 --- a/x-pack/plugin/core/src/main/java/module-info.java +++ b/x-pack/plugin/core/src/main/java/module-info.java @@ -22,6 +22,7 @@ requires unboundid.ldapsdk; requires org.elasticsearch.tdigest; requires org.elasticsearch.xcore.templates; + requires org.elasticsearch.logging; exports org.elasticsearch.index.engine.frozen; exports org.elasticsearch.license; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/HasPrivilegesRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/HasPrivilegesRequest.java index 56485ed6b1565..d4d5dd5217477 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/HasPrivilegesRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/HasPrivilegesRequest.java @@ -8,13 +8,18 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.support.IndexComponentSelector; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.core.Tuple; import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.ApplicationResourcePrivileges; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; /** * A request for checking a user's privileges @@ -84,7 +89,50 @@ public ApplicationResourcePrivileges[] applicationPrivileges() { } public void indexPrivileges(IndicesPrivileges... privileges) { - this.indexPrivileges = privileges; + IndicesPrivileges[] newPrivileges = new IndicesPrivileges[privileges.length]; + for (int i = 0; i < privileges.length; i++) { + IndicesPrivileges currentPriv = privileges[i]; + IndicesPrivileges.Builder builder = IndicesPrivileges.builder(privileges[i]); + builder.indices((String[]) null); + List updatedIndexPatterns = new ArrayList<>(); + for (String indexPatternRequested : currentPriv.getIndices()) { + Tuple split = IndexNameExpressionResolver.splitSelectorExpression(indexPatternRequested); + String indexNameNoSelector = split.v1(); + String selectorAsString = split.v2(); + if (selectorAsString == null) { + assert indexPatternRequested.equals(indexNameNoSelector); + updatedIndexPatterns.add(indexNameNoSelector); // add as-is, no selector + } else { + IndexComponentSelector selector = IndexComponentSelector.getByKey(selectorAsString); + switch (selector) { + case DATA: + updatedIndexPatterns.add(indexNameNoSelector); // strip the selector + break; + case FAILURES: + updatedIndexPatterns.add(indexPatternRequested); // add as-is, keep selector in name + break; + case ALL_APPLICABLE: + updatedIndexPatterns.add(indexNameNoSelector); // add with no selector for data + updatedIndexPatterns.add( + IndexNameExpressionResolver.combineSelector(indexNameNoSelector, IndexComponentSelector.FAILURES) + ); // add with failure selector + break; + default: + throw new IllegalArgumentException( + "Unknown index component selector [" + + selectorAsString + + "], available options are: " + + IndexComponentSelector.values() + ); + + } + } + builder.indices(updatedIndexPatterns); + newPrivileges[i] = builder.build(); + } + } + + this.indexPrivileges = newPrivileges; } public void clusterPrivileges(String... privileges) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 9f5aaa8562a88..36af37f627933 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -1371,6 +1371,10 @@ public static Builder builder() { return new Builder(); } + public static Builder builder(IndicesPrivileges copyFrom) { + return new Builder(copyFrom); + } + public String[] getIndices() { return this.indices; } @@ -1553,6 +1557,15 @@ public static class Builder { private Builder() {} + private Builder(IndicesPrivileges copyFrom) { + indicesPrivileges.indices = copyFrom.indices; + indicesPrivileges.privileges = copyFrom.privileges; + indicesPrivileges.grantedFields = copyFrom.grantedFields; + indicesPrivileges.deniedFields = copyFrom.deniedFields; + indicesPrivileges.query = copyFrom.query; + indicesPrivileges.allowRestrictedIndices = copyFrom.allowRestrictedIndices; + } + public Builder indices(String... indices) { indicesPrivileges.indices = indices; return this; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java index b91db5ca34366..f4af7c78f15b1 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java @@ -25,6 +25,8 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.Index; +import org.elasticsearch.logging.LogManager; +import org.elasticsearch.logging.Logger; import org.elasticsearch.xpack.core.security.authz.RestrictedIndices; import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; @@ -38,6 +40,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -47,6 +50,7 @@ import java.util.function.Supplier; import static java.util.Collections.unmodifiableMap; +import static org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission.Group.maybeAddFailureExclusions; /** * A permission that is based on privileges for index related actions executed @@ -55,6 +59,7 @@ public final class IndicesPermission { private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(IndicesPermission.class); + private static final Logger logger = LogManager.getLogger(IndicesPermission.class); public static final IndicesPermission NONE = new IndicesPermission(new RestrictedIndices(Automatons.EMPTY), Group.EMPTY_ARRAY); @@ -152,7 +157,7 @@ private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String if (group.allowRestrictedIndices) { restrictedIndices.addAll(Arrays.asList(group.indices())); } else { - ordinaryIndices.addAll(Arrays.asList(group.indices())); + ordinaryIndices.addAll(Arrays.asList(maybeAddFailureExclusions(group.indices()))); } } else if (isMappingUpdateAction && containsPrivilegeThatGrantsMappingUpdatesForBwc(group)) { // special BWC case for certain privileges: allow put mapping on indices and aliases (but not on data streams), even if @@ -176,6 +181,7 @@ private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String public static class IsResourceAuthorizedPredicate implements BiPredicate { private final BiPredicate biPredicate; + private final StringMatcher resourceNameMatcher; // public for tests public IsResourceAuthorizedPredicate(StringMatcher resourceNameMatcher, StringMatcher additionalNonDatastreamNameMatcher) { @@ -183,11 +189,13 @@ public IsResourceAuthorizedPredicate(StringMatcher resourceNameMatcher, StringMa assert indexAbstraction == null || name.equals(indexAbstraction.getName()); return resourceNameMatcher.test(name) || (isPartOfDatastream(indexAbstraction) == false && additionalNonDatastreamNameMatcher.test(name)); - }); + }, resourceNameMatcher); + } - private IsResourceAuthorizedPredicate(BiPredicate biPredicate) { + private IsResourceAuthorizedPredicate(BiPredicate biPredicate, StringMatcher resourceNameMatcher) { this.biPredicate = biPredicate; + this.resourceNameMatcher = resourceNameMatcher; } /** @@ -197,7 +205,7 @@ private IsResourceAuthorizedPredicate(BiPredicate biPr */ @Override public final IsResourceAuthorizedPredicate and(BiPredicate other) { - return new IsResourceAuthorizedPredicate(this.biPredicate.and(other)); + return new IsResourceAuthorizedPredicate(this.biPredicate.and(other), this.resourceNameMatcher); } /** @@ -209,6 +217,17 @@ public final boolean test(IndexAbstraction indexAbstraction) { return test(indexAbstraction.getName(), indexAbstraction); } + /** + * Similar to {@link #test(IndexAbstraction)} but for the failures component of a data stream. Adds ::failures to name of the + * index abstraction to test if ::failures are allowed + */ + public final boolean testDataStreamForFailureAccess(IndexAbstraction indexAbstraction) { + assert indexAbstraction != null && indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM; + return resourceNameMatcher.test( + IndexNameExpressionResolver.combineSelector(indexAbstraction.getName(), IndexComponentSelector.FAILURES) + ); + } + /** * Verifies if access is authorized to the resource with the given {@param name}. * The {@param indexAbstraction}, which is the resource to be accessed, must be supplied if the resource exists or be {@code null} @@ -370,18 +389,18 @@ private static class IndexResource { private final IndexAbstraction indexAbstraction; private IndexResource(String name, @Nullable IndexAbstraction abstraction, @Nullable IndexComponentSelector selector) { - assert name != null : "Resource name cannot be null"; - assert abstraction == null || abstraction.getName().equals(name) - : "Index abstraction has unexpected name [" + abstraction.getName() + "] vs [" + name + "]"; - assert abstraction == null - || selector == null - || IndexComponentSelector.FAILURES.equals(selector) == false - || abstraction.isDataStreamRelated() - : "Invalid index component selector [" - + selector.getKey() - + "] applied to abstraction of type [" - + abstraction.getType() - + "]"; + // assert name != null : "Resource name cannot be null"; + // assert abstraction == null || abstraction.getName().equals(name) + // : "Index abstraction has unexpected name [" + abstraction.getName() + "] vs [" + name + "]"; + // assert abstraction == null + // || selector == null + // || IndexComponentSelector.FAILURES.equals(selector) == false + // || abstraction.isDataStreamRelated() + // : "Invalid index component selector [" + // + selector.getKey() + // + "] applied to abstraction of type [" + // + abstraction.getType() + // + "]"; this.name = name; this.indexAbstraction = abstraction; this.selector = selector; @@ -411,7 +430,11 @@ public boolean isPartOfDataStream() { public boolean checkIndex(Group group) { final DataStream ds = indexAbstraction == null ? null : indexAbstraction.getParentDataStream(); if (ds != null) { - if (group.checkIndex(ds.getName())) { + boolean isFailureStoreIndex = ds.isFailureStoreIndex(indexAbstraction.getName()); + if (isFailureStoreIndex + && group.checkIndex(IndexNameExpressionResolver.combineSelector(ds.getName(), IndexComponentSelector.FAILURES))) { + return true; + } else if (isFailureStoreIndex == false && group.checkIndex(ds.getName())) { return true; } } @@ -500,13 +523,18 @@ public IndicesAccessControl authorize( int totalResourceCount = 0; Map lookup = metadata.getIndicesLookup(); for (String indexOrAlias : requestedIndicesOrAliases) { - // Remove any selectors from abstraction name. Discard them for this check as we do not have access control for them (yet) Tuple expressionAndSelector = IndexNameExpressionResolver.splitSelectorExpression(indexOrAlias); - indexOrAlias = expressionAndSelector.v1(); IndexComponentSelector selector = expressionAndSelector.v2() == null ? null : IndexComponentSelector.getByKey(expressionAndSelector.v2()); - final IndexResource resource = new IndexResource(indexOrAlias, lookup.get(indexOrAlias), selector); + // If the request has name::data, upstream code will remove ::data from the name + // If the request has name::* upstream code will convert to name::failures and name (without ::data) + assert selector != IndexComponentSelector.DATA : "Data selector is not allowed in this context"; + assert selector != IndexComponentSelector.ALL_APPLICABLE : "All selector is not allowed in this context"; + assert selector == null || selector == IndexComponentSelector.FAILURES + : "Only the failures selector " + "or none is not allowed in this context"; + // look up the IndexAbstraction by the name without the selector, but leave the (::failures) selector for authorization + final IndexResource resource = new IndexResource(indexOrAlias, lookup.get(expressionAndSelector.v1()), selector); resources.put(resource.name, resource); totalResourceCount += resource.size(lookup); } @@ -810,28 +838,131 @@ public Group( @Nullable Set query, boolean allowRestrictedIndices, RestrictedIndices restrictedIndices, - String... indices + final String... indices ) { assert indices.length != 0; this.privilege = privilege; this.actionMatcher = privilege.predicate(); this.indices = indices; + // TODO: [Jake] how to support selectors for hasPrivileges ? (are reg-ex's just broken for hasPrivilege index checks ?) + // TODO: [Jake] ensure that only ::failure selectors can be added the role (i.e. error on name::* or name::data) + // TODO: [Jake] ensure that no selectors can be added to remote_indices (or gate usage with a feature flag, or just test) + String[] patternsReWritten = maybeAddFailureExclusions(indices); this.allowRestrictedIndices = allowRestrictedIndices; ConcurrentHashMap indexNameAutomatonMemo = new ConcurrentHashMap<>(1); if (allowRestrictedIndices) { - this.indexNameMatcher = StringMatcher.of(indices); - this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent(indices, k -> Automatons.patterns(indices)); + this.indexNameMatcher = StringMatcher.of(patternsReWritten); + this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent( + patternsReWritten, + k -> Automatons.patterns(patternsReWritten) + ); } else { - this.indexNameMatcher = StringMatcher.of(indices).and(name -> restrictedIndices.isRestricted(name) == false); + this.indexNameMatcher = StringMatcher.of(patternsReWritten).and(name -> restrictedIndices.isRestricted(name) == false); this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent( - indices, - k -> Automatons.minusAndMinimize(Automatons.patterns(indices), restrictedIndices.getAutomaton()) + patternsReWritten, + k -> Automatons.minusAndMinimize(Automatons.patterns(patternsReWritten), restrictedIndices.getAutomaton()) ); } this.fieldPermissions = Objects.requireNonNull(fieldPermissions); this.query = query; } + // TODO: [Jake] ensure this javadoc is still correct before merging (some minor details are wrong, but the gist is correct) + /** + * This method looks for any index patterns in this group that have all the following characteristics: + *
    + *
  • Index pattern has a trailing wildcard, i.e., {@code name*}
    • + *
  • Index pattern is a regular expression, i.e. {@code /name.*fooba[r]+/}
    • + *
  • Index pattern is not {@code "*"}.
    • + *
+ * + * If all of these conditions are met, then the pattern is transformed into a regular expression to exclude failures. + * For example: + *
    + *
  • {@code name*} becomes {@code /(name.*)&~(name.*::failures)/}
    • + *
  • {@code /name.*fooba[r]+/} becomes {@code /(name.*fooba[r]+)&~(name.*fooba[r]+::failures)/}
    • + *
  • {@code na*e} remains {@code na*e} (Lucene regular expressions are always begin/end anchored)
    • + *
+ * + * Only the {@code ::failures} selector on non-regular expressions is allowed in the role definition + * (ensured by create-time validation). + * + * @param indexPatterns the index patterns for this group that have been resolved to only contain the + * {@code ::failures} selector or no selector at all + * @return a {@code String[]} of the transformed and/or non-transformed index patterns for this group + * that will be used for authorization purposes + */ + static String[] maybeAddFailureExclusions(final String[] indexPatterns) { + // TODO: [Jake] use trace logging ! + logger.error(() -> String.format(Locale.ROOT, "original indices: %s", Arrays.toString(indexPatterns))); + String[] indexPatternsWithExclusions = new String[indexPatterns.length]; + for (int i = 0; i < indexPatterns.length; i++) { + assert indexPatterns[i].endsWith("::data") == false : "Data selector is not allowed in this context"; + assert indexPatterns[i].endsWith("::*") == false : "All selector is not allowed in this context"; + if (indexPatterns[i].equals("*") == false + && (indexPatterns[i].endsWith("*") || Automatons.isLuceneRegex(indexPatterns[i]))) { + indexPatternsWithExclusions[i] = convertToExcludeFailures(indexPatterns[i]); + } else { + indexPatternsWithExclusions[i] = indexPatterns[i]; + } + } + logger.error(() -> String.format(Locale.ROOT, "after failure exclusions: %s", Arrays.toString(indexPatternsWithExclusions))); + return indexPatternsWithExclusions; + } + + static String convertToExcludeFailures(String indexPattern) { + assert indexPattern != "*" : "* is a special case and should never exclude failures"; + assert indexPattern.endsWith("*") || Automatons.isLuceneRegex(indexPattern) + : "Only patterns with a trailing wildcard " + "or regular expressions should explicitly exclude failures"; + StringBuilder sb = new StringBuilder(); + if (indexPattern.endsWith("*")) { + String inny = globToRegex(indexPattern); + return sb.append("/(").append(inny).append(")&~(").append(inny).append("::failures)/").toString(); + } else if (Automatons.isLuceneRegex(indexPattern)) { + String inny = indexPattern.substring(1, indexPattern.length() - 1); + return sb.append("/(").append(inny).append(")&~((").append(inny).append(")::failures)/").toString(); + } else { + throw new IllegalArgumentException("Unexpected index pattern: " + indexPattern); // should never happen + } + } + + private static String globToRegex(String glob) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < glob.length(); i++) { + char c = glob.charAt(i); + switch (c) { + case '*': + sb.append(".*"); + break; + case '?': + sb.append('.'); + break; + case '.': + case '(': + case ')': + case '[': + case ']': + case '{': + case '}': + case '\\': + case '\"': + case '|': + case '+': + case '#': + case '@': + case '<': + case '>': + case '~': + sb.append('\\').append(c); + break; + default: + sb.append(c); + break; + } + } + return sb.toString(); + } + public IndexPrivilege privilege() { return privilege; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java index 52a3650159a08..7e12b7e5f2a47 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java @@ -335,6 +335,7 @@ private static Predicate predicate(Automaton automaton, final String toS } else if (automaton == EMPTY) { return Predicates.never(); } + automaton = Operations.determinize(automaton, maxDeterminizedStates); CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton); return new Predicate() { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermissionTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermissionTests.java new file mode 100644 index 0000000000000..a5182d732e931 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermissionTests.java @@ -0,0 +1,35 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.core.security.authz.permission; + +import org.elasticsearch.test.ESTestCase; +import org.hamcrest.Matchers; + +public class IndicesPermissionTests extends ESTestCase { + + public void testConvertToExcludeFailures() { + + // trailing wildcard + assertThat(IndicesPermission.Group.convertToExcludeFailures("foo*"), Matchers.equalTo("/(foo.*)&~(foo.*::failures)/")); + + // regular expression + assertThat(IndicesPermission.Group.convertToExcludeFailures("/foo.*/"), Matchers.equalTo("/(foo.*)&~((foo.*)::failures)/")); + assertThat(IndicesPermission.Group.convertToExcludeFailures("/a|b/"), Matchers.equalTo("/(a|b)&~((a|b)::failures)/")); + // TODO: [Jake] ensure the double parenthesis is functionally correct. + assertThat(IndicesPermission.Group.convertToExcludeFailures("/(a|b)/"), Matchers.equalTo("/((a|b))&~(((a|b))::failures)/")); + + } + + public void testMaybeAddFailureExclusions() { + String[] indices = new String[] { "foo*", "/foo.*/" }; + String[] result = IndicesPermission.Group.maybeAddFailureExclusions(indices); + // System.out.println(java.util.Arrays.toString(result)); + assertThat(result[0], Matchers.equalTo("/(foo.*)&~(foo.*::failures)/")); + assertThat(result[1], Matchers.equalTo("/(foo.*)&~((foo.*)::failures)/")); + } +} diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/FailureStoreSecurityRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/FailureStoreSecurityRestIT.java new file mode 100644 index 0000000000000..3d636657125bd --- /dev/null +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/FailureStoreSecurityRestIT.java @@ -0,0 +1,174 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.security; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.core.Strings; +import org.elasticsearch.search.SearchHit; +import org.elasticsearch.search.SearchResponseUtils; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; + +public class FailureStoreSecurityRestIT extends SecurityOnTrialLicenseRestTestCase { + + private static final String USER = "user"; + private static final SecureString PASSWORD = new SecureString("elastic-password"); + + public void testFailureStoreAccess() throws IOException { + String failureStoreAccessRole = "failure_store_access"; + createUser(USER, PASSWORD, List.of(failureStoreAccessRole)); + + upsertRole(Strings.format(""" + { + "description": "Role with failure store access", + "cluster": ["all"], + "indices": [{"names": ["test*::failures"], "privileges": ["read"]}] + }"""), failureStoreAccessRole); + + createTemplates(); + List ids = populateDataStreamWithBulkRequest(); + assertThat(ids.size(), equalTo(2)); + assertThat(ids, hasItem("1")); + String successDocId = "1"; + String failedDocId = ids.stream().filter(id -> false == id.equals(successDocId)).findFirst().get(); + + // user with access to failures index + assertContainsDocIds(performRequestAsUser1(new Request("GET", "/test1::failures/_search")), failedDocId); + assertContainsDocIds(performRequestAsUser1(new Request("GET", "/test*::failures/_search")), failedDocId); + assertContainsDocIds(performRequestAsUser1(new Request("GET", "/*1::failures/_search")), failedDocId); + assertContainsDocIds(performRequestAsUser1(new Request("GET", "/*::failures/_search")), failedDocId); + assertContainsDocIds(performRequestAsUser1(new Request("GET", "/.fs*/_search")), failedDocId); + + expectThrows404(() -> performRequestAsUser1(new Request("GET", "/test12::failures/_search"))); + expectThrows404(() -> performRequestAsUser1(new Request("GET", "/test2::failures/_search"))); + + // user with access to everything + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/test1::failures/_search")), failedDocId); + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/test*::failures/_search")), failedDocId); + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/*1::failures/_search")), failedDocId); + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/*::failures/_search")), failedDocId); + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/.fs*/_search")), failedDocId); + + expectThrows404(() -> adminClient().performRequest(new Request("GET", "/test12::failures/_search"))); + expectThrows404(() -> adminClient().performRequest(new Request("GET", "/test2::failures/_search"))); + } + + private static void expectThrows404(ThrowingRunnable get) { + var ex = expectThrows(ResponseException.class, get); + assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(404)); + } + + @SuppressWarnings("unchecked") + private static void assertContainsDocIds(Response response, String... docIds) throws IOException { + assertOK(response); + final SearchResponse searchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response)); + try { + SearchHit[] hits = searchResponse.getHits().getHits(); + assertThat(hits.length, equalTo(docIds.length)); + List actualDocIds = Arrays.stream(hits).map(SearchHit::getId).toList(); + assertThat(actualDocIds, containsInAnyOrder(docIds)); + } finally { + searchResponse.decRef(); + } + } + + private static void assert404(Response response) { + assertThat(response.getStatusLine().getStatusCode(), equalTo(404)); + } + + private static void assert403(Response response) { + assertThat(response.getStatusLine().getStatusCode(), equalTo(403)); + } + + private void createTemplates() throws IOException { + var componentTemplateRequest = new Request("PUT", "/_component_template/component1"); + componentTemplateRequest.setJsonEntity(""" + { + "template": { + "mappings": { + "properties": { + "@timestamp": { + "type": "date" + }, + "age": { + "type": "integer" + }, + "email": { + "type": "keyword" + }, + "name": { + "type": "text" + } + } + }, + "data_stream_options": { + "failure_store": { + "enabled": true + } + } + } + } + """); + assertOK(adminClient().performRequest(componentTemplateRequest)); + + var indexTemplateRequest = new Request("PUT", "/_index_template/template1"); + indexTemplateRequest.setJsonEntity(""" + { + "index_patterns": ["test*"], + "data_stream": {}, + "priority": 500, + "composed_of": ["component1"] + } + """); + assertOK(adminClient().performRequest(indexTemplateRequest)); + } + + @SuppressWarnings("unchecked") + private List populateDataStreamWithBulkRequest() throws IOException { + var bulkRequest = new Request("POST", "/_bulk?refresh=true"); + bulkRequest.setJsonEntity(""" + { "create" : { "_index" : "test1", "_id" : "1" } } + { "@timestamp": 1, "age" : 1, "name" : "jack", "email" : "jack@example.com" } + { "create" : { "_index" : "test1", "_id" : "2" } } + { "@timestamp": 2, "age" : "this should be an int", "name" : "jack", "email" : "jack@example.com" } + """); + Response response = adminClient().performRequest(bulkRequest); + assertOK(response); + // we need this dance because the ID for the failed document is random, **not** 2 + Map stringObjectMap = responseAsMap(response); + List items = (List) stringObjectMap.get("items"); + List ids = new ArrayList<>(); + for (Object item : items) { + Map itemMap = (Map) item; + Map create = (Map) itemMap.get("create"); + assertThat(create.get("status"), equalTo(201)); + ids.add((String) create.get("_id")); + } + return ids; + } + + private Response performRequestAsUser1(Request request) throws IOException { + request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", basicAuthHeaderValue(USER, PASSWORD)).build()); + var response = client().performRequest(request); + return response; + } + +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java index b9270b6035680..4c0e52c8098ac 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java @@ -875,16 +875,25 @@ static AuthorizedIndices resolveAuthorizedIndicesFromRole( // TODO: can this be done smarter? I think there are usually more indices/aliases in the cluster then indices defined a roles? if (includeDataStreams) { for (IndexAbstraction indexAbstraction : lookup.values()) { - if (predicate.test(indexAbstraction)) { + // the index abstraction here is from cluster state which will never have the ::failure data selector + // the first check is to see if the index/alias/data stream with no selector is authorized + // the second check is to see if the data stream with the ::failures appended to the name is authorized + boolean authorizedForDataAccess = predicate.test(indexAbstraction); + boolean authorizedForFailureAccess = indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM + && predicate.testDataStreamForFailureAccess(indexAbstraction); + if (authorizedForDataAccess || authorizedForFailureAccess) { indicesAndAliases.add(indexAbstraction.getName()); + // add data stream and its backing indices for any authorized data streams if (indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM) { - // add data stream and its backing indices for any authorized data streams - for (Index index : indexAbstraction.getIndices()) { - indicesAndAliases.add(index.getName()); + if (authorizedForDataAccess) { + for (Index index : indexAbstraction.getIndices()) { + indicesAndAliases.add(index.getName()); + } } - // TODO: We need to limit if a data stream's failure indices should return here. - for (Index index : ((DataStream) indexAbstraction).getFailureIndices()) { - indicesAndAliases.add(index.getName()); + if (authorizedForFailureAccess) { + for (Index index : ((DataStream) indexAbstraction).getFailureIndices()) { + indicesAndAliases.add(index.getName()); + } } } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java index f7dc725c3f07d..7383d7b2a73b8 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java @@ -333,7 +333,11 @@ public void setup() { new RoleDescriptor( "data_stream_test2", null, - new IndicesPrivileges[] { IndicesPrivileges.builder().indices(otherDataStreamName + "*").privileges("all").build() }, + new IndicesPrivileges[] { + IndicesPrivileges.builder() + .indices(otherDataStreamName + "*", otherDataStreamName + "*::failures") + .privileges("all") + .build() }, null ) ); @@ -342,7 +346,7 @@ public void setup() { new RoleDescriptor( "data_stream_test3", null, - new IndicesPrivileges[] { IndicesPrivileges.builder().indices("logs*").privileges("all").build() }, + new IndicesPrivileges[] { IndicesPrivileges.builder().indices("logs*", "logs*::failures").privileges("all").build() }, null ) ); @@ -2413,7 +2417,7 @@ public void testBackingIndicesAreVisibleWhenIncludedByRequestWithWildcard() { public void testBackingIndicesAreNotVisibleWhenNotIncludedByRequestWithoutWildcard() { final User user = new User("data-stream-tester2", "data_stream_test2"); String dataStreamName = "logs-foobar"; - GetAliasesRequest request = new GetAliasesRequest(TEST_REQUEST_TIMEOUT, dataStreamName); + GetAliasesRequest request = new GetAliasesRequest(TEST_REQUEST_TIMEOUT, dataStreamName, dataStreamName + "::failures"); assertThat(request, instanceOf(IndicesRequest.Replaceable.class)); assertThat(request.includeDataStreams(), is(true)); @@ -2680,6 +2684,6 @@ private void assertSameValues(List indices, String[] expectedIndices) { } private boolean runFailureStore() { - return DataStream.isFailureStoreFeatureFlagEnabled() && randomBoolean(); + return DataStream.isFailureStoreFeatureFlagEnabled() && Boolean.TRUE;// && randomBoolean(); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java index 4488c28750dc0..f0136e34b234c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java @@ -344,7 +344,9 @@ public void testErrorMessageIfIndexPatternIsTooComplex() { indices.toArray(Strings.EMPTY_ARRAY) ) ); - assertThat(e.getMessage(), containsString(indices.get(0))); + // trailing wildcards will be converted to regular expressions to avoid matching ::failure indices so this assert needs to + // convert `*` to `.`*` + assertThat(e.getMessage(), containsString(indices.get(0).replace("*", ".*"))); assertThat(e.getMessage(), containsString("too complex to evaluate")); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index ed173d8e2b127..76f401c9a221c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -492,7 +492,11 @@ public void testErrorForInvalidIndexNameRegex() { getRoleForRoleNames(compositeRolesStore, Set.of("_mock_role"), future); final ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, future::actionGet); - assertThat(e.getMessage(), containsString("The set of patterns [/~(([.]|ilm-history-).*/] is invalid")); + // TODO: [Jake] should we figure out how to avoid exposing the written pattern to the user? + assertThat( + e.getMessage(), + containsString("The set of patterns [/(~(([.]|ilm-history-).*)&~((~(([.]|ilm-history-).*)::failures)/] is invalid") + ); assertThat(e.getCause().getClass(), is(IllegalArgumentException.class)); }