From 4d1bed01e8764141ddd210f62cc8b331a174e4ff Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad <902768+bizybot@users.noreply.github.com> Date: Fri, 16 Aug 2019 07:50:53 +1000 Subject: [PATCH] Allow API key to retrieve its own information with no API key privilege (#45433) Unless the API key has `manage_api_key` privilege, it cannot get its own API key information when authenticating using an API key. There can be a use case wherein we do not wish the user authenticating using an API key to be able to invalidate or view any other API keys but only view information about itself. This commit addresses this by allowing the request when API key id from the `GetApiKeyRequest` matches the API key id present in the `authentication` metadata. Relates: #40031 --- .../xpack/security/authc/ApiKeyService.java | 2 + .../security/authc/AuthenticationService.java | 2 +- .../xpack/security/authz/RBACEngine.java | 54 ++++++++++++------- .../security/authc/ApiKeyIntegTests.java | 27 ++++++++-- .../xpack/security/authz/RBACEngineTests.java | 51 ++++++++++++++++++ 5 files changed, 113 insertions(+), 23 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index ec7c2b7870840..b8777b62f1f23 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -107,6 +107,8 @@ public class ApiKeyService { private static final Logger logger = LogManager.getLogger(ApiKeyService.class); private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger); public static final String API_KEY_ID_KEY = "_security_api_key_id"; + public static final String API_KEY_REALM_NAME = "_es_api_key"; + public static final String API_KEY_REALM_TYPE = "_es_api_key"; static final String API_KEY_ROLE_DESCRIPTORS_KEY = "_security_api_key_role_descriptors"; static final String API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY = "_security_api_key_limited_by_role_descriptors"; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java index 1fe3ed67f7337..1d54c3e8dddd2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java @@ -269,7 +269,7 @@ private void checkForApiKey() { apiKeyService.authenticateWithApiKeyIfPresent(threadContext, ActionListener.wrap(authResult -> { if (authResult.isAuthenticated()) { final User user = authResult.getUser(); - authenticatedBy = new RealmRef("_es_api_key", "_es_api_key", nodeName); + authenticatedBy = new RealmRef(ApiKeyService.API_KEY_REALM_NAME, ApiKeyService.API_KEY_REALM_TYPE, nodeName); writeAuthToContext(new Authentication(user, authenticatedBy, null, Version.CURRENT, Authentication.AuthenticationType.API_KEY, authResult.getMetadata())); } else if (authResult.getStatus() == AuthenticationResult.Status.TERMINATE) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java index df00474f6d69d..a17cca5283211 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java @@ -32,6 +32,8 @@ import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.transport.TransportActionProxy; import org.elasticsearch.transport.TransportRequest; +import org.elasticsearch.xpack.core.security.action.GetApiKeyAction; +import org.elasticsearch.xpack.core.security.action.GetApiKeyRequest; import org.elasticsearch.xpack.core.security.action.user.AuthenticateAction; import org.elasticsearch.xpack.core.security.action.user.ChangePasswordAction; import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesAction; @@ -62,6 +64,7 @@ import org.elasticsearch.xpack.core.security.authz.privilege.Privilege; import org.elasticsearch.xpack.core.security.support.Automatons; import org.elasticsearch.xpack.core.security.user.User; +import org.elasticsearch.xpack.security.authc.ApiKeyService; import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm; import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore; @@ -86,7 +89,7 @@ public class RBACEngine implements AuthorizationEngine { private static final Predicate SAME_USER_PRIVILEGE = Automatons.predicate( - ChangePasswordAction.NAME, AuthenticateAction.NAME, HasPrivilegesAction.NAME, GetUserPrivilegesAction.NAME); + ChangePasswordAction.NAME, AuthenticateAction.NAME, HasPrivilegesAction.NAME, GetUserPrivilegesAction.NAME, GetApiKeyAction.NAME); private static final String INDEX_SUB_REQUEST_PRIMARY = IndexAction.NAME + "[p]"; private static final String INDEX_SUB_REQUEST_REPLICA = IndexAction.NAME + "[r]"; private static final String DELETE_SUB_REQUEST_PRIMARY = DeleteAction.NAME + "[p]"; @@ -154,26 +157,39 @@ public void authorizeClusterAction(RequestInfo requestInfo, AuthorizationInfo au boolean checkSameUserPermissions(String action, TransportRequest request, Authentication authentication) { final boolean actionAllowed = SAME_USER_PRIVILEGE.test(action); if (actionAllowed) { - if (request instanceof UserRequest == false) { - assert false : "right now only a user request should be allowed"; - return false; - } - UserRequest userRequest = (UserRequest) request; - String[] usernames = userRequest.usernames(); - if (usernames == null || usernames.length != 1 || usernames[0] == null) { - assert false : "this role should only be used for actions to apply to a single user"; + if (request instanceof UserRequest) { + UserRequest userRequest = (UserRequest) request; + String[] usernames = userRequest.usernames(); + if (usernames == null || usernames.length != 1 || usernames[0] == null) { + assert false : "this role should only be used for actions to apply to a single user"; + return false; + } + final String username = usernames[0]; + final boolean sameUsername = authentication.getUser().principal().equals(username); + if (sameUsername && ChangePasswordAction.NAME.equals(action)) { + return checkChangePasswordAction(authentication); + } + + assert AuthenticateAction.NAME.equals(action) || HasPrivilegesAction.NAME.equals(action) + || GetUserPrivilegesAction.NAME.equals(action) || sameUsername == false + : "Action '" + action + "' should not be possible when sameUsername=" + sameUsername; + return sameUsername; + } else if (request instanceof GetApiKeyRequest) { + GetApiKeyRequest getApiKeyRequest = (GetApiKeyRequest) request; + if (authentication.getAuthenticatedBy().getType().equals(ApiKeyService.API_KEY_REALM_TYPE)) { + assert authentication.getLookedUpBy() == null : "runAs not supported for api key authentication"; + // if authenticated by API key then the request must also contain same API key id + String authenticatedApiKeyId = (String) authentication.getMetadata().get(ApiKeyService.API_KEY_ID_KEY); + if (Strings.hasText(getApiKeyRequest.getApiKeyId())) { + return getApiKeyRequest.getApiKeyId().equals(authenticatedApiKeyId); + } else { + return false; + } + } + } else { + assert false : "right now only a user request or get api key request should be allowed"; return false; } - final String username = usernames[0]; - final boolean sameUsername = authentication.getUser().principal().equals(username); - if (sameUsername && ChangePasswordAction.NAME.equals(action)) { - return checkChangePasswordAction(authentication); - } - - assert AuthenticateAction.NAME.equals(action) || HasPrivilegesAction.NAME.equals(action) - || GetUserPrivilegesAction.NAME.equals(action) || sameUsername == false - : "Action '" + action + "' should not be possible when sameUsername=" + sameUsername; - return sameUsername; } return false; } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java index f73719cf25937..bec82b17c1495 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java @@ -47,6 +47,7 @@ import java.util.Base64; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -532,9 +533,25 @@ public void testInvalidateApiKeysOwnedByCurrentAuthenticatedUser() throws Interr verifyInvalidateResponse(noOfApiKeysForUserWithManageApiKeyRole, userWithManageApiKeyRoleApiKeys, invalidateResponse); } - private void verifyGetResponse(int expectedNumberOfApiKeys, List responses, GetApiKeyResponse response, - Set validApiKeyIds, - List invalidatedApiKeyIds) { + public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformation() throws InterruptedException, ExecutionException { + List responses = createApiKeys(2, null); + final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString( + (responses.get(0).getId() + ":" + responses.get(0).getKey().toString()).getBytes(StandardCharsets.UTF_8)); + Client client = client().filterWithHeader(Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue)); + PlainActionFuture listener = new PlainActionFuture<>(); + client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(0).getId(), false), listener); + GetApiKeyResponse response = listener.get(); + verifyGetResponse(1, responses, response, Collections.singleton(responses.get(0).getId()), null); + + final PlainActionFuture failureListener = new PlainActionFuture<>(); + // for any other API key id, it must deny access + client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(1).getId(), false), failureListener); + ElasticsearchSecurityException ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener.actionGet()); + assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", SecuritySettingsSource.TEST_SUPERUSER); + } + + private void verifyGetResponse(int expectedNumberOfApiKeys, List responses, + GetApiKeyResponse response, Set validApiKeyIds, List invalidatedApiKeyIds) { verifyGetResponse(SecuritySettingsSource.TEST_SUPERUSER, expectedNumberOfApiKeys, responses, response, validApiKeyIds, invalidatedApiKeyIds); } @@ -584,4 +601,8 @@ private List createApiKeys(String user, int noOfApiKeys, T assertThat(responses.size(), is(noOfApiKeys)); return responses; } + + private void assertErrorMessage(final ElasticsearchSecurityException ese, String action, String userName) { + assertThat(ese.getMessage(), is("action [" + action + "] is unauthorized for user [" + userName + "]")); + } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java index 0a431ec95f5ee..fd84afea365be 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java @@ -21,6 +21,8 @@ import org.elasticsearch.license.GetLicenseAction; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.transport.TransportRequest; +import org.elasticsearch.xpack.core.security.action.GetApiKeyAction; +import org.elasticsearch.xpack.core.security.action.GetApiKeyRequest; import org.elasticsearch.xpack.core.security.action.user.AuthenticateAction; import org.elasticsearch.xpack.core.security.action.user.AuthenticateRequest; import org.elasticsearch.xpack.core.security.action.user.AuthenticateRequestBuilder; @@ -51,6 +53,7 @@ import org.elasticsearch.xpack.core.security.authz.privilege.Privilege; import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames; import org.elasticsearch.xpack.core.security.user.User; +import org.elasticsearch.xpack.security.authc.ApiKeyService; import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm; import org.elasticsearch.xpack.security.authz.RBACEngine.RBACAuthorizationInfo; import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore; @@ -63,6 +66,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; import static java.util.Collections.emptyMap; @@ -232,6 +236,53 @@ public void testSameUserPermissionDoesNotAllowChangePasswordForLookedUpByOtherRe verifyNoMoreInteractions(authentication, lookedUpBy, authenticatedBy); } + public void testSameUserPermissionAllowsSelfApiKeyInfoRetrievalWhenAuthenticatedByApiKey() { + final User user = new User("joe"); + final String apiKeyId = randomAlphaOfLengthBetween(4, 7); + final TransportRequest request = GetApiKeyRequest.usingApiKeyId(apiKeyId, false); + final Authentication authentication = mock(Authentication.class); + final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); + when(authentication.getUser()).thenReturn(user); + when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); + when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE); + when(authentication.getMetadata()).thenReturn(Map.of(ApiKeyService.API_KEY_ID_KEY, apiKeyId)); + + assertTrue(engine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication)); + } + + public void testSameUserPermissionDeniesApiKeyInfoRetrievalWhenAuthenticatedByADifferentApiKey() { + final User user = new User("joe"); + final String apiKeyId = randomAlphaOfLengthBetween(4, 7); + final TransportRequest request = GetApiKeyRequest.usingApiKeyId(apiKeyId, false); + final Authentication authentication = mock(Authentication.class); + final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); + when(authentication.getUser()).thenReturn(user); + when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); + when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE); + when(authentication.getMetadata()).thenReturn(Map.of(ApiKeyService.API_KEY_ID_KEY, randomAlphaOfLengthBetween(4, 7))); + + assertFalse(engine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication)); + } + + public void testSameUserPermissionDeniesApiKeyInfoRetrievalWhenLookedupByIsPresent() { + final User user = new User("joe"); + final String apiKeyId = randomAlphaOfLengthBetween(4, 7); + final TransportRequest request = GetApiKeyRequest.usingApiKeyId(apiKeyId, false); + final Authentication authentication = mock(Authentication.class); + final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); + final Authentication.RealmRef lookedupBy = mock(Authentication.RealmRef.class); + when(authentication.getUser()).thenReturn(user); + when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); + when(authentication.getLookedUpBy()).thenReturn(lookedupBy); + when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE); + when(authentication.getMetadata()).thenReturn(Map.of(ApiKeyService.API_KEY_ID_KEY, randomAlphaOfLengthBetween(4, 7))); + + final AssertionError assertionError = expectThrows(AssertionError.class, () -> engine.checkSameUserPermissions(GetApiKeyAction.NAME, + request, authentication)); + assertNotNull(assertionError); + assertThat(assertionError.getLocalizedMessage(), is("runAs not supported for api key authentication")); + } + /** * This tests that action names in the request are considered "matched" by the relevant named privilege * (in this case that {@link DeleteAction} and {@link IndexAction} are satisfied by {@link IndexPrivilege#WRITE}).