unknown run as users should not be allowed to execute any APIs
If a authenticated user with run as permission attempts to run as an unknown user, the unknown user will be assigned the default role and anonymous role if enabled. This change prevents this from happening as we require the run as user to have been looked up by a realm. Closes elastic/elasticsearch#3878 Original commit: elastic/x-pack-elasticsearch@034f44757d
This commit is contained in:
parent
4badf28a8d
commit
27cb25581b
|
@ -54,6 +54,14 @@ public class Authentication {
|
||||||
return user;
|
return user;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* returns true if this authentication represents a authentication object with a authenticated user that is different than the user the
|
||||||
|
* request should be run as
|
||||||
|
*/
|
||||||
|
public boolean isRunAs() {
|
||||||
|
return getUser().equals(getRunAsUser()) == false;
|
||||||
|
}
|
||||||
|
|
||||||
public RealmRef getAuthenticatedBy() {
|
public RealmRef getAuthenticatedBy() {
|
||||||
return authenticatedBy;
|
return authenticatedBy;
|
||||||
}
|
}
|
||||||
|
|
|
@ -292,6 +292,9 @@ public class AuthenticationService extends AbstractComponent {
|
||||||
// the requested run as user does not exist, but we don't throw an error here otherwise this could let
|
// the requested run as user does not exist, but we don't throw an error here otherwise this could let
|
||||||
// information leak about users in the system... instead we'll just let the authz service fail throw an
|
// information leak about users in the system... instead we'll just let the authz service fail throw an
|
||||||
// authorization error
|
// authorization error
|
||||||
|
if (lookedupBy != null) {
|
||||||
|
throw new IllegalStateException("we could not lookup the user but created a realm reference");
|
||||||
|
}
|
||||||
user = new User(user.principal(), user.roles(), new User(runAsUsername, Strings.EMPTY_ARRAY));
|
user = new User(user.principal(), user.roles(), new User(runAsUsername, Strings.EMPTY_ARRAY));
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
logger.debug(
|
logger.debug(
|
||||||
|
|
|
@ -128,7 +128,7 @@ public class AuthorizationService extends AbstractComponent {
|
||||||
// get the roles of the authenticated user, which may be different than the effective
|
// get the roles of the authenticated user, which may be different than the effective
|
||||||
GlobalPermission permission = permission(roles);
|
GlobalPermission permission = permission(roles);
|
||||||
|
|
||||||
final boolean isRunAs = authentication.getUser() != authentication.getRunAsUser();
|
final boolean isRunAs = authentication.isRunAs();
|
||||||
// permission can be empty as it might be that the user's role is unknown
|
// permission can be empty as it might be that the user's role is unknown
|
||||||
if (permission.isEmpty()) {
|
if (permission.isEmpty()) {
|
||||||
if (isRunAs) {
|
if (isRunAs) {
|
||||||
|
@ -140,6 +140,12 @@ public class AuthorizationService extends AbstractComponent {
|
||||||
}
|
}
|
||||||
// check if the request is a run as request
|
// check if the request is a run as request
|
||||||
if (isRunAs) {
|
if (isRunAs) {
|
||||||
|
// if we are running as a user we looked up then the authentication must contain a lookedUpBy. If it doesn't then this user
|
||||||
|
// doesn't really exist but the authc service allowed it through to avoid leaking users that exist in the system
|
||||||
|
if (authentication.getLookedUpBy() == null) {
|
||||||
|
throw denyRunAs(authentication, action, request);
|
||||||
|
}
|
||||||
|
|
||||||
// first we must authorize for the RUN_AS action
|
// first we must authorize for the RUN_AS action
|
||||||
RunAsPermission runAs = permission.runAs();
|
RunAsPermission runAs = permission.runAs();
|
||||||
if (runAs != null && runAs.check(authentication.getRunAsUser().principal())) {
|
if (runAs != null && runAs.check(authentication.getRunAsUser().principal())) {
|
||||||
|
|
|
@ -91,7 +91,7 @@ public final class AuthorizationUtils {
|
||||||
setRunAsRoles(Collections.emptyList());
|
setRunAsRoles(Collections.emptyList());
|
||||||
} else {
|
} else {
|
||||||
service.roles(authentication.getUser(), ActionListener.wrap(this::setUserRoles, listener::onFailure));
|
service.roles(authentication.getUser(), ActionListener.wrap(this::setUserRoles, listener::onFailure));
|
||||||
if (authentication.getUser().equals(authentication.getRunAsUser()) == false) {
|
if (authentication.isRunAs()) {
|
||||||
assert authentication.getRunAsUser() != null : "runAs user is null but shouldn't";
|
assert authentication.getRunAsUser() != null : "runAs user is null but shouldn't";
|
||||||
service.roles(authentication.getRunAsUser(), ActionListener.wrap(this::setRunAsRoles, listener::onFailure));
|
service.roles(authentication.getRunAsUser(), ActionListener.wrap(this::setRunAsRoles, listener::onFailure));
|
||||||
} else {
|
} else {
|
||||||
|
|
|
@ -72,7 +72,7 @@ public class DefaultRole extends Role {
|
||||||
// we need to verify that this user was authenticated by or looked up by a realm type that support password changes
|
// we need to verify that this user was authenticated by or looked up by a realm type that support password changes
|
||||||
// otherwise we open ourselves up to issues where a user in a different realm could be created with the same username
|
// otherwise we open ourselves up to issues where a user in a different realm could be created with the same username
|
||||||
// and do malicious things
|
// and do malicious things
|
||||||
final boolean isRunAs = authentication.getUser() != authentication.getRunAsUser();
|
final boolean isRunAs = authentication.isRunAs();
|
||||||
final String realmType;
|
final String realmType;
|
||||||
if (isRunAs) {
|
if (isRunAs) {
|
||||||
realmType = authentication.getLookedUpBy().getType();
|
realmType = authentication.getLookedUpBy().getType();
|
||||||
|
|
|
@ -67,6 +67,7 @@ import org.elasticsearch.cluster.ClusterState;
|
||||||
import org.elasticsearch.cluster.metadata.IndexMetaData;
|
import org.elasticsearch.cluster.metadata.IndexMetaData;
|
||||||
import org.elasticsearch.cluster.metadata.MetaData;
|
import org.elasticsearch.cluster.metadata.MetaData;
|
||||||
import org.elasticsearch.cluster.service.ClusterService;
|
import org.elasticsearch.cluster.service.ClusterService;
|
||||||
|
import org.elasticsearch.common.Strings;
|
||||||
import org.elasticsearch.common.collect.Tuple;
|
import org.elasticsearch.common.collect.Tuple;
|
||||||
import org.elasticsearch.common.settings.Settings;
|
import org.elasticsearch.common.settings.Settings;
|
||||||
import org.elasticsearch.common.util.concurrent.ThreadContext;
|
import org.elasticsearch.common.util.concurrent.ThreadContext;
|
||||||
|
@ -74,6 +75,8 @@ import org.elasticsearch.test.ESTestCase;
|
||||||
import org.elasticsearch.threadpool.ThreadPool;
|
import org.elasticsearch.threadpool.ThreadPool;
|
||||||
import org.elasticsearch.transport.TransportRequest;
|
import org.elasticsearch.transport.TransportRequest;
|
||||||
import org.elasticsearch.xpack.security.SecurityTemplateService;
|
import org.elasticsearch.xpack.security.SecurityTemplateService;
|
||||||
|
import org.elasticsearch.xpack.security.action.user.AuthenticateAction;
|
||||||
|
import org.elasticsearch.xpack.security.action.user.AuthenticateRequest;
|
||||||
import org.elasticsearch.xpack.security.audit.AuditTrailService;
|
import org.elasticsearch.xpack.security.audit.AuditTrailService;
|
||||||
import org.elasticsearch.xpack.security.authc.Authentication;
|
import org.elasticsearch.xpack.security.authc.Authentication;
|
||||||
import org.elasticsearch.xpack.security.authc.Authentication.RealmRef;
|
import org.elasticsearch.xpack.security.authc.Authentication.RealmRef;
|
||||||
|
@ -407,6 +410,19 @@ public class AuthorizationServiceTests extends ESTestCase {
|
||||||
verifyNoMoreInteractions(auditTrail);
|
verifyNoMoreInteractions(auditTrail);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public void testRunAsRequestWithoutLookedUpBy() {
|
||||||
|
AuthenticateRequest request = new AuthenticateRequest("run as me");
|
||||||
|
roleMap.put("can run as", SuperuserRole.INSTANCE);
|
||||||
|
User user = new User("test user", new String[] { "can run as" }, new User("run as me", Strings.EMPTY_ARRAY));
|
||||||
|
Authentication authentication = new Authentication(user, new RealmRef("foo", "bar", "baz"), null);
|
||||||
|
assertThat(user.runAs(), is(notNullValue()));
|
||||||
|
assertThrowsAuthorizationExceptionRunAs(
|
||||||
|
() -> authorize(authentication, AuthenticateAction.NAME, request),
|
||||||
|
AuthenticateAction.NAME, "test user", "run as me"); // run as [run as me]
|
||||||
|
verify(auditTrail).runAsDenied(user, AuthenticateAction.NAME, request);
|
||||||
|
verifyNoMoreInteractions(auditTrail);
|
||||||
|
}
|
||||||
|
|
||||||
public void testRunAsRequestRunningAsUnAllowedUser() {
|
public void testRunAsRequestRunningAsUnAllowedUser() {
|
||||||
TransportRequest request = mock(TransportRequest.class);
|
TransportRequest request = mock(TransportRequest.class);
|
||||||
User user = new User("test user", new String[] { "can run as" }, new User("run as me", "doesn't exist"));
|
User user = new User("test user", new String[] { "can run as" }, new User("run as me", "doesn't exist"));
|
||||||
|
|
|
@ -145,6 +145,7 @@ public class DefaultRoleTests extends ESTestCase {
|
||||||
when(authentication.getRunAsUser()).thenReturn(runAs);
|
when(authentication.getRunAsUser()).thenReturn(runAs);
|
||||||
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
|
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
|
||||||
when(authentication.getLookedUpBy()).thenReturn(lookedUpBy);
|
when(authentication.getLookedUpBy()).thenReturn(lookedUpBy);
|
||||||
|
when(authentication.isRunAs()).thenReturn(true);
|
||||||
when(lookedUpBy.getType())
|
when(lookedUpBy.getType())
|
||||||
.thenReturn(changePasswordRequest ? randomFrom(ReservedRealm.TYPE, NativeRealm.TYPE) : randomAsciiOfLengthBetween(4, 12));
|
.thenReturn(changePasswordRequest ? randomFrom(ReservedRealm.TYPE, NativeRealm.TYPE) : randomAsciiOfLengthBetween(4, 12));
|
||||||
|
|
||||||
|
@ -162,6 +163,7 @@ public class DefaultRoleTests extends ESTestCase {
|
||||||
final RealmRef authenticatedBy = mock(RealmRef.class);
|
final RealmRef authenticatedBy = mock(RealmRef.class);
|
||||||
when(authentication.getUser()).thenReturn(user);
|
when(authentication.getUser()).thenReturn(user);
|
||||||
when(authentication.getRunAsUser()).thenReturn(user);
|
when(authentication.getRunAsUser()).thenReturn(user);
|
||||||
|
when(authentication.isRunAs()).thenReturn(false);
|
||||||
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
|
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
|
||||||
when(authenticatedBy.getType()).thenReturn(randomFrom(LdapRealm.TYPE, FileRealm.TYPE, ActiveDirectoryRealm.TYPE, PkiRealm.TYPE,
|
when(authenticatedBy.getType()).thenReturn(randomFrom(LdapRealm.TYPE, FileRealm.TYPE, ActiveDirectoryRealm.TYPE, PkiRealm.TYPE,
|
||||||
randomAsciiOfLengthBetween(4, 12)));
|
randomAsciiOfLengthBetween(4, 12)));
|
||||||
|
@ -169,9 +171,9 @@ public class DefaultRoleTests extends ESTestCase {
|
||||||
assertThat(request, instanceOf(UserRequest.class));
|
assertThat(request, instanceOf(UserRequest.class));
|
||||||
assertThat(DefaultRole.INSTANCE.cluster().check(action, request, authentication), is(false));
|
assertThat(DefaultRole.INSTANCE.cluster().check(action, request, authentication), is(false));
|
||||||
verify(authenticatedBy).getType();
|
verify(authenticatedBy).getType();
|
||||||
verify(authentication, times(2)).getRunAsUser();
|
verify(authentication).getRunAsUser();
|
||||||
verify(authentication).getUser();
|
|
||||||
verify(authentication).getAuthenticatedBy();
|
verify(authentication).getAuthenticatedBy();
|
||||||
|
verify(authentication).isRunAs();
|
||||||
verifyNoMoreInteractions(authenticatedBy, authentication);
|
verifyNoMoreInteractions(authenticatedBy, authentication);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -185,6 +187,7 @@ public class DefaultRoleTests extends ESTestCase {
|
||||||
final RealmRef lookedUpBy = mock(RealmRef.class);
|
final RealmRef lookedUpBy = mock(RealmRef.class);
|
||||||
when(authentication.getUser()).thenReturn(user);
|
when(authentication.getUser()).thenReturn(user);
|
||||||
when(authentication.getRunAsUser()).thenReturn(runAs);
|
when(authentication.getRunAsUser()).thenReturn(runAs);
|
||||||
|
when(authentication.isRunAs()).thenReturn(true);
|
||||||
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
|
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
|
||||||
when(authentication.getLookedUpBy()).thenReturn(lookedUpBy);
|
when(authentication.getLookedUpBy()).thenReturn(lookedUpBy);
|
||||||
when(lookedUpBy.getType()).thenReturn(randomFrom(LdapRealm.TYPE, FileRealm.TYPE, ActiveDirectoryRealm.TYPE, PkiRealm.TYPE,
|
when(lookedUpBy.getType()).thenReturn(randomFrom(LdapRealm.TYPE, FileRealm.TYPE, ActiveDirectoryRealm.TYPE, PkiRealm.TYPE,
|
||||||
|
@ -193,8 +196,8 @@ public class DefaultRoleTests extends ESTestCase {
|
||||||
assertThat(request, instanceOf(UserRequest.class));
|
assertThat(request, instanceOf(UserRequest.class));
|
||||||
assertThat(DefaultRole.INSTANCE.cluster().check(action, request, authentication), is(false));
|
assertThat(DefaultRole.INSTANCE.cluster().check(action, request, authentication), is(false));
|
||||||
verify(authentication).getLookedUpBy();
|
verify(authentication).getLookedUpBy();
|
||||||
verify(authentication, times(2)).getRunAsUser();
|
verify(authentication).getRunAsUser();
|
||||||
verify(authentication).getUser();
|
verify(authentication).isRunAs();
|
||||||
verify(lookedUpBy).getType();
|
verify(lookedUpBy).getType();
|
||||||
verifyNoMoreInteractions(authentication, lookedUpBy, authenticatedBy);
|
verifyNoMoreInteractions(authentication, lookedUpBy, authenticatedBy);
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue