Skip to content

Commit

Permalink
Ignore app priv failures when resolving superuser (#85519) (#85588)
Browse files Browse the repository at this point in the history 
In #81400 we changed `superuser` to no longer have _every_ privilege.
Consequently, we also removed the special case code that existed that
would ignore all other roles for any user that had superuser role.

However, we added some special handling so that failing to resolve those
other roles would not block superuser access - when a user has superuser
role, any failures in role resolution will be effectively ignored, and
the user will be given the superuser role only.

However, this failure handling did not account for the loading of
application privileges. If application privileges needed to be loaded,
but failed, this could prevent resolution of the superuser role.

This change extends the failure handling to encompass the full
resolution of roles, and fallback to superuser only, whenever other
roles or application privileges are unavailable

Relates: #85312
  • Loading branch information
@tvernum
tvernum committed Apr 1, 2022
1 parent 1683e90 commit b690154
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 25 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/85519.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 85519
summary: Ignore app priv failures when resolving superuser
area: Authorization
type: bug
issues: []
35 changes: 18 additions & 17 deletions ...urity/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,22 +240,7 @@ public void buildRoleFromRoleReference(RoleReference roleReference, ActionListen
final Role existing = roleCache.get(roleKey);
if (existing == null) {
final long invalidationCounter = numInvalidation.get();
roleReference.resolve(roleReferenceResolver, ActionListener.wrap(rolesRetrievalResult -> {
if (RolesRetrievalResult.EMPTY == rolesRetrievalResult) {
roleActionListener.onResponse(Role.EMPTY);
} else if (RolesRetrievalResult.SUPERUSER == rolesRetrievalResult) {
roleActionListener.onResponse(superuserRole);
} else {
buildThenMaybeCacheRole(
roleKey,
rolesRetrievalResult.getRoleDescriptors(),
rolesRetrievalResult.getMissingRoles(),
rolesRetrievalResult.isSuccess(),
invalidationCounter,
roleActionListener
);
}
}, e -> {
final Consumer<Exception> failureHandler = e -> {
// Because superuser does not have write access to restricted indices, it is valid to mix superuser with other roles to
// gain addition access. However, if retrieving those roles fails for some reason, then that could leave admins in a
// situation where they are unable to administer their cluster (in order to resolve the problem that is leading to failures
Expand All @@ -274,7 +259,23 @@ public void buildRoleFromRoleReference(RoleReference roleReference, ActionListen
} else {
roleActionListener.onFailure(e);
}
}));
};
roleReference.resolve(roleReferenceResolver, ActionListener.wrap(rolesRetrievalResult -> {
if (RolesRetrievalResult.EMPTY == rolesRetrievalResult) {
roleActionListener.onResponse(Role.EMPTY);
} else if (RolesRetrievalResult.SUPERUSER == rolesRetrievalResult) {
roleActionListener.onResponse(superuserRole);
} else {
buildThenMaybeCacheRole(
roleKey,
rolesRetrievalResult.getRoleDescriptors(),
rolesRetrievalResult.getMissingRoles(),
rolesRetrievalResult.isSuccess(),
invalidationCounter,
ActionListener.wrap(roleActionListener::onResponse, failureHandler)
);
}
}, failureHandler));
} else {
roleActionListener.onResponse(existing);
}
Expand Down
76 changes: 68 additions & 8 deletions .../src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
import org.elasticsearch.xpack.security.authc.service.ServiceAccountService;
import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
import org.elasticsearch.xpack.security.support.SecurityIndexManager;
import org.hamcrest.Matcher;
import org.hamcrest.Matchers;

import java.io.IOException;
Expand All @@ -117,6 +118,7 @@
import java.util.function.Predicate;

import static org.elasticsearch.test.ActionListenerUtils.anyActionListener;
import static org.elasticsearch.test.TestMatchers.throwableWithMessage;
import static org.elasticsearch.xpack.core.security.SecurityField.DOCUMENT_LEVEL_SECURITY_FEATURE;
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_ID_KEY;
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY;
Expand Down Expand Up @@ -324,6 +326,59 @@ public void testRolesWhenDlsFlsLicensed() throws IOException {
}

public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
final boolean criticalFailure = randomBoolean();
final Consumer<ActionListener<RoleRetrievalResult>> rolesHandler = callback -> {
final RuntimeException exception = new RuntimeException("Test(" + getTestName() + ") - native roles unavailable");
if (criticalFailure) {
callback.onFailure(exception);
} else {
callback.onResponse(RoleRetrievalResult.failure(exception));
}
};
final Consumer<ActionListener<Collection<ApplicationPrivilegeDescriptor>>> privilegesHandler = callback -> callback.onResponse(
Collections.emptyList()
);

final CompositeRolesStore compositeRolesStore = setupRolesStore(rolesHandler, privilegesHandler);
trySuccessfullyLoadSuperuserRole(compositeRolesStore);
if (criticalFailure) {
// A failure RoleRetrievalResult doesn't block role building, only a throw exception does
tryFailOnNonSuperuserRole(compositeRolesStore, throwableWithMessage(containsString("native roles unavailable")));
}
}

public void testSuperuserIsEffectiveWhenApplicationPrivilegesAreUnavailable() {
final RoleDescriptor role = new RoleDescriptor(
"_mock_role",
new String[0],
new IndicesPrivileges[0],
new RoleDescriptor.ApplicationResourcePrivileges[] {
RoleDescriptor.ApplicationResourcePrivileges.builder()
.application(randomAlphaOfLengthBetween(5, 12))
.privileges("all")
.resources("*")
.build() },
new ConfigurableClusterPrivilege[0],
new String[0],
Map.of(),
Map.of()
);
final Consumer<ActionListener<RoleRetrievalResult>> rolesHandler = callback -> callback.onResponse(
RoleRetrievalResult.success(Set.of(role))
);
final Consumer<ActionListener<Collection<ApplicationPrivilegeDescriptor>>> privilegesHandler = callback -> callback.onFailure(
new RuntimeException("No privileges for you!")
);

final CompositeRolesStore compositeRolesStore = setupRolesStore(rolesHandler, privilegesHandler);
trySuccessfullyLoadSuperuserRole(compositeRolesStore);
tryFailOnNonSuperuserRole(compositeRolesStore, throwableWithMessage(containsString("No privileges for you!")));
}

private CompositeRolesStore setupRolesStore(
Consumer<ActionListener<RoleRetrievalResult>> rolesHandler,
Consumer<ActionListener<Collection<ApplicationPrivilegeDescriptor>>> privilegesHandler
) {
final FileRolesStore fileRolesStore = mock(FileRolesStore.class);
doCallRealMethod().when(fileRolesStore).accept(anySet(), anyActionListener());
when(fileRolesStore.roleDescriptors(anySet())).thenReturn(Collections.emptySet());
Expand All @@ -333,13 +388,7 @@ public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
doAnswer((invocationOnMock) -> {
@SuppressWarnings("unchecked")
ActionListener<RoleRetrievalResult> callback = (ActionListener<RoleRetrievalResult>) invocationOnMock.getArguments()[1];
final RuntimeException exception = new RuntimeException("Test(" + getTestName() + ") - native roles unavailable");
if (randomBoolean()) {
callback.onFailure(exception);
} else {
callback.onResponse(RoleRetrievalResult.failure(exception));

}
rolesHandler.accept(callback);
return null;
}).when(nativeRolesStore).getRoleDescriptors(isASet(), anyActionListener());

Expand All @@ -349,7 +398,7 @@ public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
@SuppressWarnings("unchecked")
ActionListener<Collection<ApplicationPrivilegeDescriptor>> callback = (ActionListener<
Collection<ApplicationPrivilegeDescriptor>>) invocationOnMock.getArguments()[2];
callback.onResponse(Collections.emptyList());
privilegesHandler.accept(callback);
return null;
}).when(nativePrivilegeStore).getPrivileges(anySet(), anySet(), anyActionListener());

Expand All @@ -365,7 +414,10 @@ public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
null,
null
);
return compositeRolesStore;
}

private void trySuccessfullyLoadSuperuserRole(CompositeRolesStore compositeRolesStore) {
final Set<String> roles = Set.of(randomAlphaOfLengthBetween(1, 6), "superuser", randomAlphaOfLengthBetween(7, 12));
PlainActionFuture<Role> future = new PlainActionFuture<>();
getRoleForRoleNames(compositeRolesStore, roles, future);
Expand All @@ -388,6 +440,14 @@ public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
assertThat(securityActionPredicate.test(IndexAction.NAME), is(false));
}

private void tryFailOnNonSuperuserRole(CompositeRolesStore compositeRolesStore, Matcher<? super Exception> exceptionMatcher) {
final Set<String> roles = Set.of(randomAlphaOfLengthBetween(1, 6), randomAlphaOfLengthBetween(7, 12));
PlainActionFuture<Role> future = new PlainActionFuture<>();
getRoleForRoleNames(compositeRolesStore, roles, future);
final Exception exception = expectThrows(Exception.class, future::actionGet);
assertThat(exception, exceptionMatcher);
}

public void testNegativeLookupsAreCached() {
final FileRolesStore fileRolesStore = mock(FileRolesStore.class);
doCallRealMethod().when(fileRolesStore).accept(anySet(), anyActionListener());
Expand Down

0 comments on commit b690154

Please sign in to comment.