Skip to content
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

Disallow mapping updates for doc ingestion privileges #58784

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
d62250d
Done maybe
albertzaharovits Jun 30, 2020
547faa3
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jun 30, 2020
9dce3fc
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 6, 2020
00bbb0f
Merge fallout
albertzaharovits Jul 6, 2020
f4fb1c7
Auto put mapping is also bwc supported
albertzaharovits Jul 6, 2020
ac0966d
IndicesPermission
albertzaharovits Jul 7, 2020
63e7ade
WIP
albertzaharovits Jul 7, 2020
905e458
Maybeee?
albertzaharovits Jul 7, 2020
e2dc3f9
authorizeMappingUpdateBwcSpecialCase
albertzaharovits Jul 7, 2020
9a5ff34
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 7, 2020
ec96511
ReservedRolesStoreTests
albertzaharovits Jul 7, 2020
e155998
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 7, 2020
04f95a9
WIP adjust tests
albertzaharovits Jul 7, 2020
a99f9f0
Existing tests compiled
albertzaharovits Jul 7, 2020
0e7efed
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 7, 2020
4dfe44a
Checkstyle
albertzaharovits Jul 7, 2020
66e244d
Checkstyle
albertzaharovits Jul 7, 2020
2d34b46
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 7, 2020
44e0517
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 8, 2020
3c53f41
XPackRestIT
albertzaharovits Jul 8, 2020
008ec5a
index -> write for ml yaml tests
albertzaharovits Jul 8, 2020
aa62846
SmokeTestSecurityWithMustacheClientYamlTestSuiteIT
albertzaharovits Jul 8, 2020
c09a20b
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 8, 2020
3ec62d3
Data stream tests
albertzaharovits Jul 8, 2020
774a423
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 8, 2020
4c6383c
IndexPrivilege tests
albertzaharovits Jul 8, 2020
75bb67e
Nit
albertzaharovits Jul 8, 2020
b114c13
IndicesMatcher action
albertzaharovits Jul 8, 2020
4a2394c
Almost
albertzaharovits Jul 8, 2020
e08624a
Done testing
albertzaharovits Jul 8, 2020
4071e95
Review
albertzaharovits Jul 8, 2020
e6bd53f
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 13, 2020
eb50c54
Merge fallout
albertzaharovits Jul 13, 2020
49b92c6
Review partway
albertzaharovits Jul 13, 2020
50f393b
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 14, 2020
f5a85d7
Review
albertzaharovits Jul 14, 2020
3d3f64d
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 14, 2020
44c3473
Review partway
albertzaharovits Jul 14, 2020
fd45d7b
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 14, 2020
3e4f130
Message
albertzaharovits Jul 14, 2020
148057c
Deprecation msgs fallout
albertzaharovits Jul 14, 2020
9e12382
Update more deprecation logs
albertzaharovits Jul 14, 2020
f3fec33
Merge branch 'master' into remove_put_mapping_priv
albertzaharovits Jul 14, 2020
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 @@ -10,6 +10,7 @@
import org.apache.lucene.util.automaton.Operations;
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingAction;
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.common.Nullable;
Expand Down Expand Up @@ -46,14 +47,29 @@ public final class IndicesPermission {

public static final IndicesPermission NONE = new IndicesPermission();

private final ConcurrentMap<String, Predicate<String>> allowedIndicesMatchersForAction = new ConcurrentHashMap<>();
private final ConcurrentMap<String, Predicate<IndexAbstraction>> allowedIndicesMatchersForAction = new ConcurrentHashMap<>();

private final Group[] groups;

public IndicesPermission(Group... groups) {
this.groups = groups;
}

public static Predicate<String> indexMatcher(Collection<String> ordinaryIndices, Collection<String> restrictedIndices) {
final Predicate<String> namePredicate;
if (restrictedIndices.isEmpty()) {
namePredicate = indexMatcher(ordinaryIndices)
.and(index -> false == RestrictedIndicesNames.isRestricted(index));
} else if (ordinaryIndices.isEmpty()) {
namePredicate = indexMatcher(restrictedIndices);
} else {
namePredicate = indexMatcher(restrictedIndices)
.or(indexMatcher(ordinaryIndices)
.and(index -> false == RestrictedIndicesNames.isRestricted(index)));
}
return namePredicate;
}

public static Predicate<String> indexMatcher(Collection<String> indices) {
Set<String> exactMatch = new HashSet<>();
List<String> nonExactMatch = new ArrayList<>();
Expand Down Expand Up @@ -105,7 +121,7 @@ public Group[] groups() {
* @return A predicate that will match all the indices that this permission
* has the privilege for executing the given action on.
*/
public Predicate<String> allowedIndicesMatcher(String action) {
public Predicate<IndexAbstraction> allowedIndicesMatcher(String action) {
return allowedIndicesMatchersForAction.computeIfAbsent(action, a -> Group.buildIndexMatcherPredicateForAction(a, groups));
}

Expand All @@ -117,7 +133,7 @@ public Predicate<String> allowedIndicesMatcher(String action) {
*/
public boolean check(String action) {
for (Group group : groups) {
if (group.check(action)) {
if (group.checkAction(action) || authorizeMappingUpdateBwcSpecialCase(group, action)) {
return true;
}
}
Expand Down Expand Up @@ -213,7 +229,9 @@ public Map<String, IndicesAccessControl.IndexAccessControl> authorize(String act
}

for (Group group : groups) {
if (group.check(action, indexOrAlias)) {
if (group.checkIndex(indexOrAlias) &&
(group.checkAction(action) || (authorizeMappingUpdateBwcSpecialCase(group, action) &&
(indexAbstraction == null || indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM)))) {
granted = true;
for (String index : concreteIndices) {
Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.computeIfAbsent(index, (k) -> new HashSet<>());
Expand Down Expand Up @@ -276,6 +294,14 @@ private boolean isConcreteRestrictedIndex(String indexPattern) {
return RestrictedIndicesNames.isRestricted(indexPattern);
}

private static boolean authorizeMappingUpdateBwcSpecialCase(Group group, String action) {
return action.equals(PutMappingAction.NAME) ||
(group.privilege().name().containsAll(IndexPrivilege.CREATE_DOC.name()) ||
group.privilege().name().containsAll(IndexPrivilege.CREATE.name()) ||
group.privilege().name().containsAll(IndexPrivilege.INDEX.name()) ||
group.privilege().name().containsAll(IndexPrivilege.WRITE.name()));
}

public static class Group {
private final IndexPrivilege privilege;
private final Predicate<String> actionMatcher;
Expand Down Expand Up @@ -317,14 +343,13 @@ public FieldPermissions getFieldPermissions() {
return fieldPermissions;
}

private boolean check(String action) {
private boolean checkAction(String action) {
return actionMatcher.test(action);
}

private boolean check(String action, String index) {
private boolean checkIndex(String index) {
assert index != null;
return check(action) && indexNameMatcher.test(index)
&& (allowRestrictedIndices || (false == RestrictedIndicesNames.isRestricted(index)));
return indexNameMatcher.test(index) && (allowRestrictedIndices || (false == RestrictedIndicesNames.isRestricted(index)));
}

boolean hasQuery() {
Expand All @@ -344,30 +369,36 @@ public static Automaton buildIndexMatcherAutomaton(boolean allowRestrictedIndice
}
}

private static Predicate<String> buildIndexMatcherPredicateForAction(String action, Group... groups) {
private static Predicate<IndexAbstraction> buildIndexMatcherPredicateForAction(String action, Group... groups) {
final Set<String> ordinaryIndices = new HashSet<>();
final Set<String> restrictedIndices = new HashSet<>();
final Set<String> grantMappingUpdatesOnIndices = new HashSet<>();
final Set<String> grantMappingUpdatesOnRestrictedIndices = new HashSet<>();
for (final Group group : groups) {
if (group.actionMatcher.test(action)) {
if (group.allowRestrictedIndices) {
restrictedIndices.addAll(Arrays.asList(group.indices()));
} else {
ordinaryIndices.addAll(Arrays.asList(group.indices()));
}
} else if (authorizeMappingUpdateBwcSpecialCase(group, action)) {
// special BWC case for certain privileges: allow put mapping on indices and aliases (but not on data streams), even if
// the privilege definition does not currently allow it
if (group.allowRestrictedIndices) {
grantMappingUpdatesOnRestrictedIndices.addAll(Arrays.asList(group.indices()));
} else {
grantMappingUpdatesOnIndices.addAll(Arrays.asList(group.indices()));
}
}
}
final Predicate<String> predicate;
if (restrictedIndices.isEmpty()) {
predicate = indexMatcher(ordinaryIndices)
.and(index -> false == RestrictedIndicesNames.isRestricted(index));
} else if (ordinaryIndices.isEmpty()) {
predicate = indexMatcher(restrictedIndices);
} else {
predicate = indexMatcher(restrictedIndices)
.or(indexMatcher(ordinaryIndices)
.and(index -> false == RestrictedIndicesNames.isRestricted(index)));
}
return predicate;
final Predicate<String> namePredicate = indexMatcher(ordinaryIndices, restrictedIndices);
final Predicate<String> bwcSpecialCaseNamePredicate = indexMatcher(grantMappingUpdatesOnIndices,
grantMappingUpdatesOnRestrictedIndices);
return indexAbstraction -> {
return namePredicate.test(indexAbstraction.getName()) ||
(indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM &&
bwcSpecialCaseNamePredicate.test(indexAbstraction.getName()));
};
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ public IndicesAccessControl authorize(String action, Set<String> requestedIndice
* action on.
*/
@Override
public Predicate<String> allowedIndicesMatcher(String action) {
Predicate<String> predicate = super.indices().allowedIndicesMatcher(action);
public Predicate<IndexAbstraction> allowedIndicesMatcher(String action) {
Predicate<IndexAbstraction> predicate = super.indices().allowedIndicesMatcher(action);
predicate = predicate.and(limitedBy.indices().allowedIndicesMatcher(action));
return predicate;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public static Builder builder(RoleDescriptor rd, FieldPermissionsCache fieldPerm
* @return A predicate that will match all the indices that this role
* has the privilege for executing the given action on.
*/
public Predicate<String> allowedIndicesMatcher(String action) {
public Predicate<IndexAbstraction> allowedIndicesMatcher(String action) {
return indices.allowedIndicesMatcher(action);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,13 @@ public final class IndexPrivilege extends Privilege {
private static final Automaton READ_CROSS_CLUSTER_AUTOMATON = patterns("internal:transport/proxy/indices:data/read/*",
ClusterSearchShardsAction.NAME);
private static final Automaton CREATE_AUTOMATON = patterns("indices:data/write/index*", "indices:data/write/bulk*",
PutMappingAction.NAME, AutoPutMappingAction.NAME);
AutoPutMappingAction.NAME);
private static final Automaton CREATE_DOC_AUTOMATON = patterns("indices:data/write/index", "indices:data/write/index[*",
"indices:data/write/index:op_type/create", "indices:data/write/bulk*", PutMappingAction.NAME, AutoPutMappingAction.NAME);
"indices:data/write/index:op_type/create", "indices:data/write/bulk*", AutoPutMappingAction.NAME);
private static final Automaton INDEX_AUTOMATON = patterns("indices:data/write/index*", "indices:data/write/bulk*",
"indices:data/write/update*", PutMappingAction.NAME, AutoPutMappingAction.NAME);
"indices:data/write/update*", AutoPutMappingAction.NAME);
private static final Automaton DELETE_AUTOMATON = patterns("indices:data/write/delete*", "indices:data/write/bulk*");
private static final Automaton WRITE_AUTOMATON = patterns("indices:data/write/*", PutMappingAction.NAME,
AutoPutMappingAction.NAME);
private static final Automaton WRITE_AUTOMATON = patterns("indices:data/write/*", AutoPutMappingAction.NAME);
private static final Automaton MONITOR_AUTOMATON = patterns("indices:monitor/*");
private static final Automaton MANAGE_AUTOMATON =
unionAndMinimize(Arrays.asList(MONITOR_AUTOMATON, patterns("indices:admin/*")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,17 +501,16 @@ GetUserPrivilegesResponse buildUserPrivilegesResponseObject(Role userRole) {
}

static List<String> resolveAuthorizedIndicesFromRole(Role role, String action, Map<String, IndexAbstraction> aliasAndIndexLookup) {
Predicate<String> predicate = role.allowedIndicesMatcher(action);
Predicate<IndexAbstraction> predicate = role.allowedIndicesMatcher(action);

List<String> indicesAndAliases = new ArrayList<>();
List<String> authorizedIndicesForAction = new ArrayList<>();
// TODO: can this be done smarter? I think there are usually more indices/aliases in the cluster then indices defined a roles?
for (Map.Entry<String, IndexAbstraction> entry : aliasAndIndexLookup.entrySet()) {
String aliasOrIndex = entry.getKey();
if (predicate.test(aliasOrIndex)) {
indicesAndAliases.add(aliasOrIndex);
if (predicate.test(entry.getValue())) {
authorizedIndicesForAction.add(entry.getValue().getName());
}
}
return Collections.unmodifiableList(indicesAndAliases);
return Collections.unmodifiableList(authorizedIndicesForAction);
}

private void buildIndicesAccessControl(Authentication authentication, String action,
Expand Down