Fix Has Privilege API check on restricted indices (#41226)
The Has Privileges API allows to tap into the authorization process, to validate privileges without actually running the operations to be authorized. This commit fixes a bug, in which the Has Privilege API returned spurious results when checking for index privileges over restricted indices (currently .security, .security-6, .security-7). The actual authorization process is not affected by the bug.
This commit is contained in:
parent
7e3875d781
commit
fe5789ada1
|
@ -29,11 +29,13 @@ privilege is assigned to the user.
|
|||
|
||||
`index`::
|
||||
`names`::: (list) A list of indices.
|
||||
`allow_restricted_indices`::: (boolean) If `names` contains internal restricted
|
||||
that also have to be covered by the has-privilege check, then this has to be
|
||||
set to `true`. By default this is `false` because restricted indices should
|
||||
generaly not be "visible" to APIs. For most use cases it is safe to ignore
|
||||
this parameter.
|
||||
`allow_restricted_indices`::: (boolean) This needs to be set to `true` (default
|
||||
is `false`) if using wildcards or regexps for patterns that cover restricted
|
||||
indices. Implicitly, restricted indices do not match index patterns because
|
||||
restricted indices usually have limited privileges and including them in
|
||||
pattern tests would render most such tests `false`. If restricted indices are
|
||||
explicitly included in the `names` list, privileges will be checked against
|
||||
them regardless of the value of `allow_restricted_indices`.
|
||||
`privileges`::: (list) A list of the privileges that you want to check for the
|
||||
specified indices.
|
||||
|
||||
|
|
|
@ -138,27 +138,40 @@ public final class IndicesPermission {
|
|||
final ResourcePrivilegesMap.Builder resourcePrivilegesMapBuilder = ResourcePrivilegesMap.builder();
|
||||
final Map<IndicesPermission.Group, Automaton> predicateCache = new HashMap<>();
|
||||
for (String forIndexPattern : checkForIndexPatterns) {
|
||||
final Automaton checkIndexAutomaton = IndicesPermission.Group.buildIndexMatcherAutomaton(allowRestrictedIndices,
|
||||
forIndexPattern);
|
||||
Automaton allowedIndexPrivilegesAutomaton = null;
|
||||
for (Group group : groups) {
|
||||
final Automaton groupIndexAutomaton = predicateCache.computeIfAbsent(group,
|
||||
g -> IndicesPermission.Group.buildIndexMatcherAutomaton(g.allowRestrictedIndices(), g.indices()));
|
||||
if (Operations.subsetOf(checkIndexAutomaton, groupIndexAutomaton)) {
|
||||
if (allowedIndexPrivilegesAutomaton != null) {
|
||||
allowedIndexPrivilegesAutomaton = Automatons
|
||||
.unionAndMinimize(Arrays.asList(allowedIndexPrivilegesAutomaton, group.privilege().getAutomaton()));
|
||||
} else {
|
||||
allowedIndexPrivilegesAutomaton = group.privilege().getAutomaton();
|
||||
Automaton checkIndexAutomaton = Automatons.patterns(forIndexPattern);
|
||||
if (false == allowRestrictedIndices && false == RestrictedIndicesNames.RESTRICTED_NAMES.contains(forIndexPattern)) {
|
||||
checkIndexAutomaton = Automatons.minusAndMinimize(checkIndexAutomaton, RestrictedIndicesNames.NAMES_AUTOMATON);
|
||||
}
|
||||
if (false == Operations.isEmpty(checkIndexAutomaton)) {
|
||||
Automaton allowedIndexPrivilegesAutomaton = null;
|
||||
for (Group group : groups) {
|
||||
final Automaton groupIndexAutomaton = predicateCache.computeIfAbsent(group,
|
||||
g -> IndicesPermission.Group.buildIndexMatcherAutomaton(g.allowRestrictedIndices(), g.indices()));
|
||||
if (Operations.subsetOf(checkIndexAutomaton, groupIndexAutomaton)) {
|
||||
if (allowedIndexPrivilegesAutomaton != null) {
|
||||
allowedIndexPrivilegesAutomaton = Automatons
|
||||
.unionAndMinimize(Arrays.asList(allowedIndexPrivilegesAutomaton, group.privilege().getAutomaton()));
|
||||
} else {
|
||||
allowedIndexPrivilegesAutomaton = group.privilege().getAutomaton();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
for (String privilege : checkForPrivileges) {
|
||||
IndexPrivilege indexPrivilege = IndexPrivilege.get(Collections.singleton(privilege));
|
||||
if (allowedIndexPrivilegesAutomaton != null
|
||||
&& Operations.subsetOf(indexPrivilege.getAutomaton(), allowedIndexPrivilegesAutomaton)) {
|
||||
resourcePrivilegesMapBuilder.addResourcePrivilege(forIndexPattern, privilege, Boolean.TRUE);
|
||||
} else {
|
||||
for (String privilege : checkForPrivileges) {
|
||||
IndexPrivilege indexPrivilege = IndexPrivilege.get(Collections.singleton(privilege));
|
||||
if (allowedIndexPrivilegesAutomaton != null
|
||||
&& Operations.subsetOf(indexPrivilege.getAutomaton(), allowedIndexPrivilegesAutomaton)) {
|
||||
resourcePrivilegesMapBuilder.addResourcePrivilege(forIndexPattern, privilege, Boolean.TRUE);
|
||||
} else {
|
||||
resourcePrivilegesMapBuilder.addResourcePrivilege(forIndexPattern, privilege, Boolean.FALSE);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// the index pattern produced the empty automaton, presumably because the requested pattern expands exclusively inside the
|
||||
// restricted indices namespace - a namespace of indices that are normally hidden when granting/checking privileges - and
|
||||
// the pattern was not marked as `allowRestrictedIndices`. We try to anticipate this by considering _explicit_ restricted
|
||||
// indices even if `allowRestrictedIndices` is false.
|
||||
// TODO The `false` result is a _safe_ default but this is actually an error. Make it an error.
|
||||
for (String privilege : checkForPrivileges) {
|
||||
resourcePrivilegesMapBuilder.addResourcePrivilege(forIndexPattern, privilege, Boolean.FALSE);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -49,7 +49,6 @@ public class Role {
|
|||
this.runAs = Objects.requireNonNull(runAs);
|
||||
}
|
||||
|
||||
|
||||
public String[] names() {
|
||||
return names;
|
||||
}
|
||||
|
@ -116,7 +115,7 @@ public class Role {
|
|||
* @return an instance of {@link ResourcePrivilegesMap}
|
||||
*/
|
||||
public ResourcePrivilegesMap checkIndicesPrivileges(Set<String> checkForIndexPatterns, boolean allowRestrictedIndices,
|
||||
Set<String> checkForPrivileges) {
|
||||
Set<String> checkForPrivileges) {
|
||||
return indices.checkResourcePrivileges(checkForIndexPatterns, allowRestrictedIndices, checkForPrivileges);
|
||||
}
|
||||
|
||||
|
|
|
@ -48,6 +48,7 @@ import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivileg
|
|||
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
|
||||
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilege;
|
||||
import org.elasticsearch.xpack.core.security.authz.privilege.ConditionalClusterPrivileges.ManageApplicationPrivileges;
|
||||
import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames;
|
||||
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
|
||||
import org.elasticsearch.xpack.core.security.authz.privilege.Privilege;
|
||||
import org.elasticsearch.xpack.core.security.user.User;
|
||||
|
@ -493,6 +494,130 @@ public class RBACEngineTests extends ESTestCase {
|
|||
));
|
||||
}
|
||||
|
||||
public void testCheckExplicitRestrictedIndexPermissions() throws Exception {
|
||||
User user = new User(randomAlphaOfLengthBetween(4, 12));
|
||||
Authentication authentication = mock(Authentication.class);
|
||||
when(authentication.getUser()).thenReturn(user);
|
||||
final boolean restrictedIndexPermission = randomBoolean();
|
||||
final boolean restrictedMonitorPermission = randomBoolean();
|
||||
Role role = Role.builder("role")
|
||||
.add(FieldPermissions.DEFAULT, null, IndexPrivilege.INDEX, restrictedIndexPermission, ".sec*")
|
||||
.add(FieldPermissions.DEFAULT, null, IndexPrivilege.MONITOR, restrictedMonitorPermission, ".security*")
|
||||
.build();
|
||||
RBACAuthorizationInfo authzInfo = new RBACAuthorizationInfo(role, null);
|
||||
|
||||
String explicitRestrictedIndex = randomFrom(RestrictedIndicesNames.RESTRICTED_NAMES);
|
||||
HasPrivilegesResponse response = hasPrivileges(RoleDescriptor.IndicesPrivileges.builder()
|
||||
.indices(new String[] {".secret-non-restricted", explicitRestrictedIndex})
|
||||
.privileges("index", "monitor")
|
||||
.allowRestrictedIndices(false) // explicit false for test
|
||||
.build(), authentication, authzInfo, Collections.emptyList(), Strings.EMPTY_ARRAY);
|
||||
assertThat(response.isCompleteMatch(), is(false));
|
||||
assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(2));
|
||||
assertThat(response.getIndexPrivileges(), containsInAnyOrder(
|
||||
ResourcePrivileges.builder(".secret-non-restricted") // matches ".sec*" but not ".security*"
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", true).put("monitor", false).map()).build(),
|
||||
ResourcePrivileges.builder(explicitRestrictedIndex) // matches both ".sec*" and ".security*"
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", restrictedIndexPermission).put("monitor", restrictedMonitorPermission).map()).build()));
|
||||
|
||||
explicitRestrictedIndex = randomFrom(RestrictedIndicesNames.RESTRICTED_NAMES);
|
||||
response = hasPrivileges(RoleDescriptor.IndicesPrivileges.builder()
|
||||
.indices(new String[] {".secret-non-restricted", explicitRestrictedIndex})
|
||||
.privileges("index", "monitor")
|
||||
.allowRestrictedIndices(true) // explicit true for test
|
||||
.build(), authentication, authzInfo, Collections.emptyList(), Strings.EMPTY_ARRAY);
|
||||
assertThat(response.isCompleteMatch(), is(false));
|
||||
assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(2));
|
||||
assertThat(response.getIndexPrivileges(), containsInAnyOrder(
|
||||
ResourcePrivileges.builder(".secret-non-restricted") // matches ".sec*" but not ".security*"
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", true).put("monitor", false).map()).build(),
|
||||
ResourcePrivileges.builder(explicitRestrictedIndex) // matches both ".sec*" and ".security*"
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", restrictedIndexPermission).put("monitor", restrictedMonitorPermission).map()).build()));
|
||||
}
|
||||
|
||||
public void testCheckRestrictedIndexWildcardPermissions() throws Exception {
|
||||
User user = new User(randomAlphaOfLengthBetween(4, 12));
|
||||
Authentication authentication = mock(Authentication.class);
|
||||
when(authentication.getUser()).thenReturn(user);
|
||||
Role role = Role.builder("role")
|
||||
.add(FieldPermissions.DEFAULT, null, IndexPrivilege.INDEX, false, ".sec*")
|
||||
.add(FieldPermissions.DEFAULT, null, IndexPrivilege.MONITOR, true, ".security*")
|
||||
.build();
|
||||
RBACAuthorizationInfo authzInfo = new RBACAuthorizationInfo(role, null);
|
||||
|
||||
HasPrivilegesResponse response = hasPrivileges(RoleDescriptor.IndicesPrivileges.builder()
|
||||
.indices(".sec*", ".security*")
|
||||
.privileges("index", "monitor")
|
||||
.build(), authentication, authzInfo, Collections.emptyList(), Strings.EMPTY_ARRAY);
|
||||
assertThat(response.isCompleteMatch(), is(false));
|
||||
assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(2));
|
||||
assertThat(response.getIndexPrivileges(), containsInAnyOrder(
|
||||
ResourcePrivileges.builder(".sec*")
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", true).put("monitor", false).map()).build(),
|
||||
ResourcePrivileges.builder(".security*")
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", true).put("monitor", true).map()).build()
|
||||
));
|
||||
|
||||
response = hasPrivileges(RoleDescriptor.IndicesPrivileges.builder()
|
||||
.indices(".sec*", ".security*")
|
||||
.privileges("index", "monitor")
|
||||
.allowRestrictedIndices(true)
|
||||
.build(), authentication, authzInfo, Collections.emptyList(), Strings.EMPTY_ARRAY);
|
||||
assertThat(response.isCompleteMatch(), is(false));
|
||||
assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(2));
|
||||
assertThat(response.getIndexPrivileges(), containsInAnyOrder(
|
||||
ResourcePrivileges.builder(".sec*")
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", false).put("monitor", false).map()).build(),
|
||||
ResourcePrivileges.builder(".security*")
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", false).put("monitor", true).map()).build()
|
||||
));
|
||||
|
||||
role = Role.builder("role")
|
||||
.add(FieldPermissions.DEFAULT, null, IndexPrivilege.INDEX, true, ".sec*")
|
||||
.add(FieldPermissions.DEFAULT, null, IndexPrivilege.MONITOR, false, ".security*")
|
||||
.build();
|
||||
authzInfo = new RBACAuthorizationInfo(role, null);
|
||||
|
||||
response = hasPrivileges(RoleDescriptor.IndicesPrivileges.builder()
|
||||
.indices(".sec*", ".security*")
|
||||
.privileges("index", "monitor")
|
||||
.build(), authentication, authzInfo, Collections.emptyList(), Strings.EMPTY_ARRAY);
|
||||
assertThat(response.isCompleteMatch(), is(false));
|
||||
assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(2));
|
||||
assertThat(response.getIndexPrivileges(), containsInAnyOrder(
|
||||
ResourcePrivileges.builder(".sec*")
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", true).put("monitor", false).map()).build(),
|
||||
ResourcePrivileges.builder(".security*")
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", true).put("monitor", true).map()).build()
|
||||
));
|
||||
|
||||
response = hasPrivileges(RoleDescriptor.IndicesPrivileges.builder()
|
||||
.indices(".sec*", ".security*")
|
||||
.privileges("index", "monitor")
|
||||
.allowRestrictedIndices(true)
|
||||
.build(), authentication, authzInfo, Collections.emptyList(), Strings.EMPTY_ARRAY);
|
||||
assertThat(response.isCompleteMatch(), is(false));
|
||||
assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(2));
|
||||
assertThat(response.getIndexPrivileges(), containsInAnyOrder(
|
||||
ResourcePrivileges.builder(".sec*")
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", true).put("monitor", false).map()).build(),
|
||||
ResourcePrivileges.builder(".security*")
|
||||
.addPrivileges(MapBuilder.newMapBuilder(new LinkedHashMap<String, Boolean>())
|
||||
.put("index", true).put("monitor", false).map()).build()
|
||||
));
|
||||
}
|
||||
|
||||
public void testCheckingApplicationPrivilegesOnDifferentApplicationsAndResources() throws Exception {
|
||||
List<ApplicationPrivilegeDescriptor> privs = new ArrayList<>();
|
||||
final ApplicationPrivilege app1Read = defineApplicationPrivilege(privs, "app1", "read", "data:read/*");
|
||||
|
|
Loading…
Reference in New Issue