Skip to content

POC support read_failures #122007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 54 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
1335ba6
WIP failure store security
n1v0lg Jan 30, 2025
b68affc
Authz
n1v0lg Jan 30, 2025
1bbce4f
Fix
n1v0lg Jan 30, 2025
552309b
Merge branch 'main' into failure-store-security-poc
n1v0lg Jan 30, 2025
d8aa0f9
Fix compilation
n1v0lg Jan 30, 2025
9e97332
Fix wildcard handling
n1v0lg Jan 30, 2025
4d9cf2f
WIP failure store check
n1v0lg Jan 31, 2025
95af75e
Moar
n1v0lg Jan 31, 2025
39ff7a7
Merge branch 'main' into failure-store-security-poc
n1v0lg Jan 31, 2025
61ae8ac
merge
n1v0lg Jan 31, 2025
3202566
Try handling wildcards
n1v0lg Jan 31, 2025
90aca93
Handle restricted indices
n1v0lg Jan 31, 2025
de81400
TODOs and REST IT
n1v0lg Jan 31, 2025
fbc86e2
Handle concrete failure index access with wildcards
n1v0lg Jan 31, 2025
ecbe06c
Add test
n1v0lg Jan 31, 2025
24567ad
Merge branch 'main' into failure-store-security-poc
n1v0lg Feb 3, 2025
9275a69
Handle wildcard and clean up
n1v0lg Feb 3, 2025
4f51d2b
More
n1v0lg Feb 3, 2025
33d91e8
Moar
n1v0lg Feb 3, 2025
cd9fd3b
Merge branch 'main' into failure-store-security-poc
n1v0lg Feb 3, 2025
2e4f374
Simplify
n1v0lg Feb 3, 2025
c194cd3
More coverage
n1v0lg Feb 3, 2025
51be855
Test concrete index access
n1v0lg Feb 3, 2025
754e615
More coverage
n1v0lg Feb 3, 2025
9b8b405
Tweak
n1v0lg Feb 3, 2025
ccd8a5d
Nit
n1v0lg Feb 3, 2025
707a9a9
Merge branch 'main' into failure-store-security-poc
n1v0lg Feb 3, 2025
91456e8
Rework all the things
n1v0lg Feb 3, 2025
e6de849
Merge branch 'failure-store-security-poc' of github.com:n1v0lg/elasti…
n1v0lg Feb 3, 2025
5103c66
More
n1v0lg Feb 3, 2025
6ac6a49
Fix test
n1v0lg Feb 3, 2025
45de638
Fix test
n1v0lg Feb 4, 2025
f0374b3
Merge branch 'main' into failure-store-security-poc
n1v0lg Feb 4, 2025
f99ae79
Rm sout
n1v0lg Feb 4, 2025
c8a7450
Try to simplify
n1v0lg Feb 4, 2025
9fbac06
TODO
n1v0lg Feb 4, 2025
038d66e
Rename
n1v0lg Feb 5, 2025
ed85b19
Merge branch 'main' into failure-store-security-poc
n1v0lg Feb 6, 2025
e7f84d9
Maybe HasPrivileges
n1v0lg Feb 6, 2025
132c6b0
Merge branch 'main' into failure-store-security-poc
n1v0lg Feb 6, 2025
be293d6
Support selectors
n1v0lg Feb 6, 2025
2f49f05
Fix
n1v0lg Feb 6, 2025
b78a4d4
Support read_failures
n1v0lg Feb 7, 2025
da7fb70
Undo all the things
n1v0lg Feb 7, 2025
2a36936
Undo
n1v0lg Feb 7, 2025
47af8c3
More
n1v0lg Feb 7, 2025
b790c94
Moar
n1v0lg Feb 7, 2025
de6054e
WIP fix unit tests
n1v0lg Feb 7, 2025
7961094
Moar
n1v0lg Feb 7, 2025
6de6b6d
Comment out
n1v0lg Feb 7, 2025
7412064
Test write
n1v0lg Feb 7, 2025
678256c
More
n1v0lg Feb 7, 2025
0be2246
Fix more tests
n1v0lg Feb 7, 2025
53ec1a3
Concrete index access
n1v0lg Feb 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ A successful call returns an object with "cluster", "index", and "remote_cluster
"none",
"read",
"read_cross_cluster",
"read_failures",
"view_index_metadata",
"write"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ default boolean isDataStreamRelated() {
return false;
}

default boolean isFailureIndexOfDataStream() {
return false;
}

/**
* An index abstraction type.
*/
Expand Down Expand Up @@ -193,6 +197,11 @@ public boolean isSystem() {
return isSystem;
}

@Override
public boolean isFailureIndexOfDataStream() {
return getParentDataStream() != null && getParentDataStream().isFailureStoreIndex(getName());
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Predicate;
import java.util.function.BiPredicate;
import java.util.function.Supplier;

public class IndexAbstractionResolver {
Expand All @@ -37,7 +37,7 @@ public List<String> resolveIndexAbstractions(
IndicesOptions indicesOptions,
Metadata metadata,
Supplier<Set<String>> allAuthorizedAndAvailable,
Predicate<String> isAuthorized,
BiPredicate<String, String> isAuthorized,
boolean includeDataStreams
) {
List<String> finalIndices = new ArrayList<>();
Expand Down Expand Up @@ -70,7 +70,8 @@ public List<String> resolveIndexAbstractions(
if (indicesOptions.expandWildcardExpressions() && Regex.isSimpleMatchPattern(indexAbstraction)) {
wildcardSeen = true;
Set<String> resolvedIndices = new HashSet<>();
for (String authorizedIndex : allAuthorizedAndAvailable.get()) {
Set<String> authorizedIndices = allAuthorizedAndAvailable.get();
for (String authorizedIndex : authorizedIndices) {
if (Regex.simpleMatch(indexAbstraction, authorizedIndex)
&& isIndexVisible(
indexAbstraction,
Expand Down Expand Up @@ -103,7 +104,7 @@ && isIndexVisible(
resolveSelectorsAndCombine(indexAbstraction, selectorString, indicesOptions, resolvedIndices, metadata);
if (minus) {
finalIndices.removeAll(resolvedIndices);
} else if (indicesOptions.ignoreUnavailable() == false || isAuthorized.test(indexAbstraction)) {
} else if (indicesOptions.ignoreUnavailable() == false || isAuthorized.test(indexAbstraction, selectorString)) {
// Unauthorized names are considered unavailable, so if `ignoreUnavailable` is `true` they should be silently
// discarded from the `finalIndices` list. Other "ways of unavailable" must be handled by the action
// handler, see: https://github.com/elastic/elasticsearch/issues/90215
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,13 @@ private List<String> resolveAbstractionsSelectorAllowed(List<String> expressions
}

private List<String> resolveAbstractions(List<String> expressions, IndicesOptions indicesOptions, Supplier<Set<String>> mask) {
return indexAbstractionResolver.resolveIndexAbstractions(expressions, indicesOptions, metadata, mask, (idx) -> true, true);
return indexAbstractionResolver.resolveIndexAbstractions(
expressions,
indicesOptions,
metadata,
mask,
(idx, selector) -> true,
true
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,11 @@ interface AuthorizedIndices {
/**
* Checks if an index-like resource name is authorized, for an action by a user. The resource might or might not exist.
*/
boolean check(String name);
default boolean check(String name) {
return check(name, null);
}

boolean check(String name, String selector);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ public Builder addGroup(
FieldPermissions fieldPermissions,
@Nullable Set<BytesReference> query,
boolean allowRestrictedIndices,
IndexComponentSelector selector,
String... indices
) {
groups.add(new Group(privilege, fieldPermissions, query, allowRestrictedIndices, restrictedIndices, indices));
groups.add(new Group(privilege, fieldPermissions, query, allowRestrictedIndices, restrictedIndices, selector, indices));
return this;
}

Expand Down Expand Up @@ -142,17 +143,37 @@ public boolean hasFieldOrDocumentLevelSecurity() {
}

private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String action) {
final Set<String> ordinaryIndices = new HashSet<>();
final Set<String> restrictedIndices = new HashSet<>();
final Set<String> dataAccessOrdinaryIndices = new HashSet<>();
final Set<String> failureAccessOrdinaryIndices = new HashSet<>();
final Set<String> dataAccessRestrictedIndices = new HashSet<>();
final Set<String> failureAccessRestrictedIndices = new HashSet<>();

// TODO do we need to worry about failure access here?
final Set<String> grantMappingUpdatesOnIndices = new HashSet<>();
final Set<String> grantMappingUpdatesOnRestrictedIndices = new HashSet<>();
final boolean isMappingUpdateAction = isMappingUpdateAction(action);
for (final Group group : groups) {
if (group.actionMatcher.test(action)) {
if (group.allowRestrictedIndices) {
restrictedIndices.addAll(Arrays.asList(group.indices()));
switch (group.selector) {
case DATA -> dataAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
case FAILURES -> failureAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
case ALL_APPLICABLE -> {
dataAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
failureAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
}
default -> throw new IllegalStateException("unexpected selector [" + group.selector + "]");
}
} else {
ordinaryIndices.addAll(Arrays.asList(group.indices()));
switch (group.selector) {
case DATA -> dataAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
case FAILURES -> failureAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
case ALL_APPLICABLE -> {
dataAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
failureAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
}
default -> throw new IllegalStateException("unexpected selector [" + group.selector + "]");
}
}
} 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
Expand All @@ -164,40 +185,55 @@ private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String
}
}
}
final StringMatcher nameMatcher = indexMatcher(ordinaryIndices, restrictedIndices);
final StringMatcher dataAccessNameMatcher = indexMatcher(dataAccessOrdinaryIndices, dataAccessRestrictedIndices);
final StringMatcher failureAccessNameMatcher = indexMatcher(failureAccessOrdinaryIndices, failureAccessRestrictedIndices);
final StringMatcher bwcSpecialCaseMatcher = indexMatcher(grantMappingUpdatesOnIndices, grantMappingUpdatesOnRestrictedIndices);
return new IsResourceAuthorizedPredicate(nameMatcher, bwcSpecialCaseMatcher);
return new IsResourceAuthorizedPredicate(dataAccessNameMatcher, failureAccessNameMatcher, bwcSpecialCaseMatcher);
}

/**
* This encapsulates the authorization test for resources.
* There is an additional test for resources that are missing or that are not a datastream or a backing index.
*/
public static class IsResourceAuthorizedPredicate implements BiPredicate<String, IndexAbstraction> {
public static class IsResourceAuthorizedPredicate {

private final BiPredicate<String, IndexAbstraction> biPredicate;
private final BiPredicate<String, IndexAbstraction> failureAccessBiPredicate;

// public for tests
public IsResourceAuthorizedPredicate(StringMatcher resourceNameMatcher, StringMatcher additionalNonDatastreamNameMatcher) {
public IsResourceAuthorizedPredicate(
StringMatcher resourceNameMatcher,
StringMatcher failureAccessNameMatcher,
StringMatcher additionalNonDatastreamNameMatcher
) {
this((String name, @Nullable IndexAbstraction indexAbstraction) -> {
assert indexAbstraction == null || name.equals(indexAbstraction.getName());
return resourceNameMatcher.test(name)
|| (isPartOfDatastream(indexAbstraction) == false && additionalNonDatastreamNameMatcher.test(name));
}, (String name, @Nullable IndexAbstraction indexAbstraction) -> {
assert indexAbstraction == null || name.equals(indexAbstraction.getName());
return failureAccessNameMatcher.test(name);
});
}

private IsResourceAuthorizedPredicate(BiPredicate<String, IndexAbstraction> biPredicate) {
private IsResourceAuthorizedPredicate(
BiPredicate<String, IndexAbstraction> biPredicate,
BiPredicate<String, IndexAbstraction> failureAccessBiPredicate
) {
this.biPredicate = biPredicate;
this.failureAccessBiPredicate = failureAccessBiPredicate;
}

/**
* Given another {@link IsResourceAuthorizedPredicate} instance in {@param other},
* return a new {@link IsResourceAuthorizedPredicate} instance that is equivalent to the conjunction of
* authorization tests of that other instance and this one.
*/
@Override
public final IsResourceAuthorizedPredicate and(BiPredicate<? super String, ? super IndexAbstraction> other) {
return new IsResourceAuthorizedPredicate(this.biPredicate.and(other));
public final IsResourceAuthorizedPredicate and(IsResourceAuthorizedPredicate other) {
return new IsResourceAuthorizedPredicate(
this.biPredicate.and(other.biPredicate),
this.failureAccessBiPredicate.and(other.failureAccessBiPredicate)
);
}

/**
Expand All @@ -206,7 +242,11 @@ public final IsResourceAuthorizedPredicate and(BiPredicate<? super String, ? sup
* Returns {@code true} if access to the given resource is authorized or {@code false} otherwise.
*/
public final boolean test(IndexAbstraction indexAbstraction) {
return test(indexAbstraction.getName(), indexAbstraction);
return test(indexAbstraction.getName(), indexAbstraction, null);
}

public final boolean test(IndexAbstraction indexAbstraction, @Nullable String selector) {
return test(indexAbstraction.getName(), indexAbstraction, selector);
}

/**
Expand All @@ -215,9 +255,19 @@ public final boolean test(IndexAbstraction indexAbstraction) {
* if it doesn't.
* Returns {@code true} if access to the given resource is authorized or {@code false} otherwise.
*/
@Override
public boolean test(String name, @Nullable IndexAbstraction indexAbstraction) {
return biPredicate.test(name, indexAbstraction);
return test(name, indexAbstraction, null);
}

public boolean test(String name, @Nullable IndexAbstraction indexAbstraction, @Nullable String selector) {
if (selector == null || IndexComponentSelector.DATA.getKey().equals(selector)) {
return biPredicate.test(name, indexAbstraction);
} else if (IndexComponentSelector.FAILURES.getKey().equals(selector)) {
return failureAccessBiPredicate.test(name, indexAbstraction);
} else {
assert selector.equals(IndexComponentSelector.ALL_APPLICABLE.getKey()) : "unexpected selector [" + selector + "]";
return biPredicate.test(name, indexAbstraction) && failureAccessBiPredicate.test(name, indexAbstraction);
}
}

private static boolean isPartOfDatastream(IndexAbstraction indexAbstraction) {
Expand Down Expand Up @@ -246,13 +296,15 @@ public boolean checkResourcePrivileges(
Set<String> checkForIndexPatterns,
boolean allowRestrictedIndices,
Set<String> checkForPrivileges,
@Nullable IndexComponentSelector selector,
@Nullable ResourcePrivilegesMap.Builder resourcePrivilegesMapBuilder
) {
return checkResourcePrivileges(
checkForIndexPatterns,
allowRestrictedIndices,
checkForPrivileges,
false,
selector,
resourcePrivilegesMapBuilder
);
}
Expand All @@ -276,11 +328,13 @@ public boolean checkResourcePrivileges(
boolean allowRestrictedIndices,
Set<String> checkForPrivileges,
boolean combineIndexGroups,
@Nullable IndexComponentSelector selector,
@Nullable ResourcePrivilegesMap.Builder resourcePrivilegesMapBuilder
) {
boolean allMatch = true;
Map<Automaton, Automaton> indexGroupAutomatons = indexGroupAutomatons(
combineIndexGroups && checkForIndexPatterns.stream().anyMatch(Automatons::isLuceneRegex)
combineIndexGroups && checkForIndexPatterns.stream().anyMatch(Automatons::isLuceneRegex),
selector
);
for (String forIndexPattern : checkForIndexPatterns) {
Automaton checkIndexAutomaton = Automatons.patterns(forIndexPattern);
Expand Down Expand Up @@ -340,7 +394,8 @@ public boolean checkResourcePrivileges(
public Automaton allowedActionsMatcher(String index) {
List<Automaton> automatonList = new ArrayList<>();
for (Group group : groups) {
if (group.indexNameMatcher.test(index)) {
// TODO failure store?
if (group.checkIndex(index) && group.checkSelector(null)) {
automatonList.add(group.privilege.getAutomaton());
}
}
Expand Down Expand Up @@ -412,10 +467,15 @@ public boolean checkIndex(Group group) {
final DataStream ds = indexAbstraction == null ? null : indexAbstraction.getParentDataStream();
if (ds != null) {
if (group.checkIndex(ds.getName())) {
return true;
final IndexComponentSelector selectorToCheck = indexAbstraction.isFailureIndexOfDataStream()
? IndexComponentSelector.FAILURES
: selector;
if (group.checkSelector(selectorToCheck)) {
return true;
}
}
}
return group.checkIndex(name);
return group.checkIndex(name) && group.checkSelector(selector);
}

/**
Expand Down Expand Up @@ -754,10 +814,13 @@ private static boolean containsPrivilegeThatGrantsMappingUpdatesForBwc(Group gro
*
* @return a map of all index and privilege pattern automatons
*/
private Map<Automaton, Automaton> indexGroupAutomatons(boolean combine) {
private Map<Automaton, Automaton> indexGroupAutomatons(boolean combine, @Nullable IndexComponentSelector selector) {
// Map of privilege automaton object references (cached by IndexPrivilege::CACHE)
Map<Automaton, Automaton> allAutomatons = new HashMap<>();
for (Group group : groups) {
if (false == group.checkSelector(selector)) {
continue;
}
Automaton indexAutomaton = group.getIndexMatcherAutomaton();
allAutomatons.compute(
group.privilege().getAutomaton(),
Expand Down Expand Up @@ -803,13 +866,15 @@ public static class Group {
// users. Setting this flag true eliminates the special status for the purpose of this permission - restricted indices still have
// to be covered by the "indices"
private final boolean allowRestrictedIndices;
public final IndexComponentSelector selector;

public Group(
IndexPrivilege privilege,
FieldPermissions fieldPermissions,
@Nullable Set<BytesReference> query,
boolean allowRestrictedIndices,
RestrictedIndices restrictedIndices,
IndexComponentSelector selector,
String... indices
) {
assert indices.length != 0;
Expand All @@ -830,6 +895,7 @@ public Group(
}
this.fieldPermissions = Objects.requireNonNull(fieldPermissions);
this.query = query;
this.selector = selector;
}

public IndexPrivilege privilege() {
Expand Down Expand Up @@ -858,6 +924,15 @@ private boolean checkIndex(String index) {
return indexNameMatcher.test(index);
}

private boolean checkSelector(@Nullable IndexComponentSelector selectorToCheck) {
if (this.selector == IndexComponentSelector.ALL_APPLICABLE) {
return true;
}
boolean includeData = selectorToCheck == null || selectorToCheck.shouldIncludeData();
boolean includeFailures = selectorToCheck != null && selectorToCheck.shouldIncludeFailures();
return includeData == this.selector.shouldIncludeData() && includeFailures == this.selector.shouldIncludeFailures();
}

boolean hasQuery() {
return query != null;
}
Expand All @@ -871,6 +946,7 @@ public Automaton getIndexMatcherAutomaton() {
}

boolean isTotal() {
// TODO add selector?
return allowRestrictedIndices
&& indexNameMatcher.isTotal()
&& privilege == IndexPrivilege.ALL
Expand All @@ -880,6 +956,7 @@ boolean isTotal() {

@Override
public String toString() {
// TODO add selector?
return "Group{"
+ "privilege="
+ privilege
Expand Down
Loading
Loading