From 27cb25581b6398da8341f9c85cb52f99225c5f33 Mon Sep 17 00:00:00 2001 From: jaymode Date: Wed, 26 Oct 2016 12:50:36 -0400 Subject: [PATCH] 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@034f44757db8ca76d7e05fc9698423b7cf55250f --- .../xpack/security/authc/Authentication.java | 8 ++++++++ .../security/authc/AuthenticationService.java | 3 +++ .../security/authz/AuthorizationService.java | 8 +++++++- .../xpack/security/authz/AuthorizationUtils.java | 2 +- .../security/authz/permission/DefaultRole.java | 2 +- .../authz/AuthorizationServiceTests.java | 16 ++++++++++++++++ .../authz/permission/DefaultRoleTests.java | 11 +++++++---- 7 files changed, 43 insertions(+), 7 deletions(-) diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/Authentication.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/Authentication.java index 30f69d3c0ee..deaf1806575 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/Authentication.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/Authentication.java @@ -54,6 +54,14 @@ public class Authentication { 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() { return authenticatedBy; } diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java index 38bbb7cfaab..ebedd4353c9 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java @@ -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 // information leak about users in the system... instead we'll just let the authz service fail throw an // 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)); } catch (Exception e) { logger.debug( diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index 170464eaf43..f614f58196f 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -128,7 +128,7 @@ public class AuthorizationService extends AbstractComponent { // get the roles of the authenticated user, which may be different than the effective 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 if (permission.isEmpty()) { if (isRunAs) { @@ -140,6 +140,12 @@ public class AuthorizationService extends AbstractComponent { } // check if the request is a run as request 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 RunAsPermission runAs = permission.runAs(); if (runAs != null && runAs.check(authentication.getRunAsUser().principal())) { diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationUtils.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationUtils.java index f4ba6cb9fb8..0cf19e2d75d 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationUtils.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationUtils.java @@ -91,7 +91,7 @@ public final class AuthorizationUtils { setRunAsRoles(Collections.emptyList()); } else { 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"; service.roles(authentication.getRunAsUser(), ActionListener.wrap(this::setRunAsRoles, listener::onFailure)); } else { diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/permission/DefaultRole.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/permission/DefaultRole.java index db391287847..8ed29b0999c 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/permission/DefaultRole.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/permission/DefaultRole.java @@ -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 // 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 - final boolean isRunAs = authentication.getUser() != authentication.getRunAsUser(); + final boolean isRunAs = authentication.isRunAs(); final String realmType; if (isRunAs) { realmType = authentication.getLookedUpBy().getType(); diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index 730949bd325..3898eed2600 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -67,6 +67,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -74,6 +75,8 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportRequest; 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.authc.Authentication; import org.elasticsearch.xpack.security.authc.Authentication.RealmRef; @@ -407,6 +410,19 @@ public class AuthorizationServiceTests extends ESTestCase { 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() { TransportRequest request = mock(TransportRequest.class); User user = new User("test user", new String[] { "can run as" }, new User("run as me", "doesn't exist")); diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/permission/DefaultRoleTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/permission/DefaultRoleTests.java index c62c5e6ad23..9a5a436d05e 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/permission/DefaultRoleTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authz/permission/DefaultRoleTests.java @@ -145,6 +145,7 @@ public class DefaultRoleTests extends ESTestCase { when(authentication.getRunAsUser()).thenReturn(runAs); when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authentication.getLookedUpBy()).thenReturn(lookedUpBy); + when(authentication.isRunAs()).thenReturn(true); when(lookedUpBy.getType()) .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); when(authentication.getUser()).thenReturn(user); when(authentication.getRunAsUser()).thenReturn(user); + when(authentication.isRunAs()).thenReturn(false); when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authenticatedBy.getType()).thenReturn(randomFrom(LdapRealm.TYPE, FileRealm.TYPE, ActiveDirectoryRealm.TYPE, PkiRealm.TYPE, randomAsciiOfLengthBetween(4, 12))); @@ -169,9 +171,9 @@ public class DefaultRoleTests extends ESTestCase { assertThat(request, instanceOf(UserRequest.class)); assertThat(DefaultRole.INSTANCE.cluster().check(action, request, authentication), is(false)); verify(authenticatedBy).getType(); - verify(authentication, times(2)).getRunAsUser(); - verify(authentication).getUser(); + verify(authentication).getRunAsUser(); verify(authentication).getAuthenticatedBy(); + verify(authentication).isRunAs(); verifyNoMoreInteractions(authenticatedBy, authentication); } @@ -185,6 +187,7 @@ public class DefaultRoleTests extends ESTestCase { final RealmRef lookedUpBy = mock(RealmRef.class); when(authentication.getUser()).thenReturn(user); when(authentication.getRunAsUser()).thenReturn(runAs); + when(authentication.isRunAs()).thenReturn(true); when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authentication.getLookedUpBy()).thenReturn(lookedUpBy); 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(DefaultRole.INSTANCE.cluster().check(action, request, authentication), is(false)); verify(authentication).getLookedUpBy(); - verify(authentication, times(2)).getRunAsUser(); - verify(authentication).getUser(); + verify(authentication).getRunAsUser(); + verify(authentication).isRunAs(); verify(lookedUpBy).getType(); verifyNoMoreInteractions(authentication, lookedUpBy, authenticatedBy); }