From 223e37574885353feca63d3e08a759234f99ee6c Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 18 Jan 2022 15:16:55 +1100 Subject: [PATCH] Fix incorrect assumptions for api_key authentication type (#81564) This is essentially a follow-up PR for #79809 which enables run-as for API keys. The code still has a few places where it assumes API keys cannot perform run-as. This PR fixes the incorrect assumptions and adds relevant tests. Resolves: #81425 --- .../xpack/core/security/SecurityContext.java | 3 +- .../core/security/authc/Authentication.java | 20 ++- .../ManageOwnApiKeyClusterPrivilege.java | 5 +- .../security/authc/AuthenticationTests.java | 23 ++- .../ManageOwnApiKeyClusterPrivilegeTests.java | 38 ++++- .../security/authc/ApiKeyIntegTests.java | 31 ++++ .../action/TransportCreateApiKeyAction.java | 2 +- .../token/TransportCreateTokenAction.java | 2 +- .../user/TransportAuthenticateAction.java | 3 +- .../audit/logfile/LoggingAuditTrail.java | 2 +- .../xpack/security/authc/ApiKeyService.java | 18 +-- .../authc/service/ServiceAccountService.java | 2 +- .../security/authz/AuthorizationService.java | 7 +- .../xpack/security/authz/RBACEngine.java | 5 +- .../ingest/SetSecurityUserProcessor.java | 2 +- .../TransportAuthenticateActionTests.java | 136 ++++++++++++++---- .../security/authc/ApiKeyServiceTests.java | 1 + .../xpack/security/authz/RBACEngineTests.java | 13 +- 18 files changed, 237 insertions(+), 76 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java index 2261ec18880de..a7c9071ef9c25 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java @@ -186,7 +186,8 @@ public void executeAfterRewritingAuthentication(Consumer consumer @SuppressWarnings("unchecked") private Map rewriteMetadataForApiKeyRoleDescriptors(Version streamVersion, Authentication authentication) { Map metadata = authentication.getMetadata(); - if (authentication.getAuthenticationType() == AuthenticationType.API_KEY) { + // If authentication type is API key, regardless whether it has run-as, the metadata must contain API key role descriptors + if (authentication.isAuthenticatedWithApiKey()) { if (authentication.getVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES) && streamVersion.before(VERSION_API_KEY_ROLES_AS_BYTES)) { metadata = new HashMap<>(metadata); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index 4306699146c8d..e47cd0bc8d3c8 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -125,6 +125,20 @@ public boolean isAuthenticatedWithApiKey() { return AuthenticationType.API_KEY.equals(getAuthenticationType()); } + /** + * Authenticate with a service account and no run-as + */ + public boolean isServiceAccount() { + return isAuthenticatedWithServiceAccount() && false == getUser().isRunAs(); + } + + /** + * Authenticated with an API key and no run-as + */ + public boolean isApiKey() { + return isAuthenticatedWithApiKey() && false == getUser().isRunAs(); + } + /** * Writes the authentication to the context. There must not be an existing authentication in the context and if there is an * {@link IllegalStateException} will be thrown @@ -170,11 +184,11 @@ public void writeTo(StreamOutput out) throws IOException { * security limitations */ public boolean canAccessResourcesOf(Authentication other) { - if (AuthenticationType.API_KEY == getAuthenticationType() && AuthenticationType.API_KEY == other.getAuthenticationType()) { + if (isApiKey() && other.isApiKey()) { final boolean sameKeyId = getMetadata().get(AuthenticationField.API_KEY_ID_KEY) .equals(other.getMetadata().get(AuthenticationField.API_KEY_ID_KEY)); if (sameKeyId) { - assert getUser().principal().equals(other.getUser().principal()) + assert getUser().principal().equals(getUser().principal()) : "The same API key ID cannot be attributed to two different usernames"; } return sameKeyId; @@ -278,7 +292,7 @@ public void toXContentFragment(XContentBuilder builder) throws IOException { } private void assertApiKeyMetadata() { - assert (AuthenticationType.API_KEY.equals(this.type) == false) || (this.metadata.get(AuthenticationField.API_KEY_ID_KEY) != null) + assert (false == isAuthenticatedWithApiKey()) || (this.metadata.get(AuthenticationField.API_KEY_ID_KEY) != null) : "API KEY authentication requires metadata to contain API KEY id, and the value must be non-null."; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index 104cef4ef9136..aa19d63221ec1 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -14,7 +14,6 @@ import org.elasticsearch.xpack.core.security.action.InvalidateApiKeyRequest; import org.elasticsearch.xpack.core.security.action.apikey.QueryApiKeyRequest; import org.elasticsearch.xpack.core.security.authc.Authentication; -import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; import org.elasticsearch.xpack.core.security.authc.AuthenticationField; import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.support.Automatons; @@ -120,7 +119,7 @@ private boolean checkIfUserIsOwnerOfApiKeys( * TODO bizybot we need to think on how we can propagate appropriate error message to the end user when username, realm name * is missing. This is similar to the problem of propagating right error messages in case of access denied. */ - if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { + if (authentication.isApiKey()) { // API key cannot own any other API key so deny access return false; } else if (ownedByAuthenticatedUser) { @@ -135,7 +134,7 @@ private boolean checkIfUserIsOwnerOfApiKeys( } private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authentication authentication, String apiKeyId) { - if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { + if (authentication.isApiKey()) { // API key id from authentication must match the id from request final String authenticatedApiKeyId = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_ID_KEY); if (Strings.hasText(apiKeyId)) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java index 69c6abcf065ce..c9076ea01f7f9 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java @@ -96,6 +96,27 @@ public void testCanAccessResourcesOf() { randomApiKeyAuthentication(randomFrom(user1, user2), apiKeyId1), randomApiKeyAuthentication(randomFrom(user1, user2), apiKeyId2) ); + + final User user3 = randomValueOtherThanMany( + u -> u.principal().equals(user1.principal()) || u.principal().equals(user2.principal()), + AuthenticationTests::randomUser + ); + + // Same API key but run-as different user are not the same owner + assertCannotAccessResources( + randomApiKeyAuthentication(new User(user2, user1), apiKeyId1), + randomApiKeyAuthentication(new User(user3, user1), apiKeyId1) + ); + + // Same or different API key run-as the same user are the same owner + checkCanAccessResources( + randomApiKeyAuthentication(new User(user3, user1), apiKeyId1), + randomApiKeyAuthentication(new User(user3, user1), apiKeyId1) + ); + checkCanAccessResources( + randomApiKeyAuthentication(new User(user3, user1), apiKeyId1), + randomApiKeyAuthentication(new User(user3, user2), apiKeyId2) + ); } public void testIsServiceAccount() { @@ -213,7 +234,7 @@ public static Authentication randomApiKeyAuthentication(User user, String apiKey return new Authentication( user, apiKeyRealm, - null, + user.isRunAs() ? new RealmRef("lookup_realm", "lookup_realm", randomAlphaOfLength(5)) : null, VersionUtils.randomVersionBetween(random(), Version.V_7_0_0, Version.CURRENT), AuthenticationType.API_KEY, metadata diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java index 8fda71fc22c6f..078cfd5bbf4f4 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java @@ -38,7 +38,8 @@ public void testAuthenticationWithApiKeyAllowsAccessToApiKeyActionsWhenItIsOwner final HashMap metadata = new HashMap<>(); metadata.put(AuthenticationField.API_KEY_ID_KEY, apiKeyId); metadata.put(AuthenticationField.API_KEY_NAME_KEY, randomAlphaOfLengthBetween(1, 16)); - final Authentication authentication = createMockAuthentication("joe", "_es_api_key", AuthenticationType.API_KEY, metadata); + final User userJoe = new User("joe"); + final Authentication authentication = createMockAuthentication(userJoe, "_es_api_key", AuthenticationType.API_KEY, metadata); final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); @@ -55,7 +56,8 @@ public void testAuthenticationWithApiKeyDeniesAccessToApiKeyActionsWhenItIsNotOw final HashMap metadata = new HashMap<>(); metadata.put(AuthenticationField.API_KEY_ID_KEY, randomAlphaOfLength(7)); metadata.put(AuthenticationField.API_KEY_NAME_KEY, randomBoolean() ? null : randomAlphaOfLengthBetween(1, 16)); - final Authentication authentication = createMockAuthentication("joe", "_es_api_key", AuthenticationType.API_KEY, metadata); + final User userJoe = new User("joe"); + final Authentication authentication = createMockAuthentication(userJoe, "_es_api_key", AuthenticationType.API_KEY, metadata); final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); @@ -67,7 +69,14 @@ public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner() final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()) .build(); - final Authentication authentication = createMockAuthentication("joe", "realm1", AuthenticationType.REALM, Map.of()); + final boolean isRunAs = randomBoolean(); + final User userJoe = new User("joe"); + final Authentication authentication = createMockAuthentication( + isRunAs ? new User(userJoe, new User("not-joe")) : userJoe, + "realm1", + isRunAs ? randomFrom(AuthenticationType.REALM, AuthenticationType.API_KEY) : AuthenticationType.REALM, + Map.of() + ); final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"); final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe"); @@ -80,7 +89,14 @@ public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner_W final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()) .build(); - final Authentication authentication = createMockAuthentication("joe", "realm1", AuthenticationType.REALM, Map.of()); + final boolean isRunAs = randomBoolean(); + final User userJoe = new User("joe"); + final Authentication authentication = createMockAuthentication( + isRunAs ? new User(userJoe, new User("not-joe")) : userJoe, + "realm1", + isRunAs ? randomFrom(AuthenticationType.REALM, AuthenticationType.API_KEY) : AuthenticationType.REALM, + Map.of() + ); final TransportRequest getApiKeyRequest = GetApiKeyRequest.forOwnedApiKeys(); final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.forOwnedApiKeys(); @@ -93,7 +109,14 @@ public void testAuthenticationWithUserDeniesAccessToApiKeyActionsWhenItIsNotOwne final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()) .build(); - final Authentication authentication = createMockAuthentication("joe", "realm1", AuthenticationType.REALM, Map.of()); + final boolean isRunAs = randomBoolean(); + final User userJoe = new User("joe"); + final Authentication authentication = createMockAuthentication( + isRunAs ? new User(userJoe, new User("not-joe")) : userJoe, + "realm1", + isRunAs ? randomFrom(AuthenticationType.REALM, AuthenticationType.API_KEY) : AuthenticationType.REALM, + Map.of() + ); final TransportRequest getApiKeyRequest = randomFrom( GetApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), @@ -153,12 +176,11 @@ public void testCheckQueryApiKeyRequest() { } private Authentication createMockAuthentication( - String username, + User user, String realmName, AuthenticationType authenticationType, Map metadata ) { - final User user = new User(username); final Authentication authentication = mock(Authentication.class); final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); when(authentication.getUser()).thenReturn(user); @@ -166,6 +188,8 @@ private Authentication createMockAuthentication( when(authentication.getAuthenticationType()).thenReturn(authenticationType); when(authenticatedBy.getName()).thenReturn(realmName); when(authentication.getMetadata()).thenReturn(metadata); + when(authentication.isAuthenticatedWithApiKey()).thenCallRealMethod(); + when(authentication.isApiKey()).thenCallRealMethod(); return authentication; } diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java index ec094c3ea6a96..db3964edac998 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java @@ -89,6 +89,7 @@ import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.INTERNAL_SECURITY_MAIN_INDEX_7; import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.SECURITY_MAIN_ALIAS; import static org.elasticsearch.xpack.security.Security.SECURITY_CRYPTO_THREAD_POOL_NAME; +import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -1179,6 +1180,36 @@ public void testDerivedKeys() throws ExecutionException, InterruptedException { assertApiKeyNotCreated(client, "key-5"); } + public void testApiKeyRunAsAnotherUserCanCreateApiKey() { + final RoleDescriptor descriptor = new RoleDescriptor("role", Strings.EMPTY_ARRAY, null, new String[] { ES_TEST_ROOT_USER }); + Client client = client().filterWithHeader( + Map.of("Authorization", basicAuthHeaderValue(ES_TEST_ROOT_USER, TEST_PASSWORD_SECURE_STRING)) + ); + final CreateApiKeyResponse response1 = new CreateApiKeyRequestBuilder(client).setName("run-as-key") + .setRoleDescriptors(List.of(descriptor)) + .setMetadata(ApiKeyTests.randomMetadata()) + .get(); + + final String base64ApiKeyKeyValue = Base64.getEncoder() + .encodeToString((response1.getId() + ":" + response1.getKey()).getBytes(StandardCharsets.UTF_8)); + + final CreateApiKeyResponse response2 = new CreateApiKeyRequestBuilder( + client().filterWithHeader( + Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue, "es-security-runas-user", ES_TEST_ROOT_USER) + ) + ).setName("create-by run-as user").setRoleDescriptors(List.of(new RoleDescriptor("a", new String[] { "all" }, null, null))).get(); + + final GetApiKeyResponse getApiKeyResponse = client.execute( + GetApiKeyAction.INSTANCE, + GetApiKeyRequest.usingApiKeyId(response2.getId(), true) + ).actionGet(); + assertThat(getApiKeyResponse.getApiKeyInfos(), arrayWithSize(1)); + final ApiKey apiKeyInfo = getApiKeyResponse.getApiKeyInfos()[0]; + assertThat(apiKeyInfo.getId(), equalTo(response2.getId())); + assertThat(apiKeyInfo.getUsername(), equalTo(ES_TEST_ROOT_USER)); + assertThat(apiKeyInfo.getRealm(), equalTo("file")); + } + public void testCreationAndAuthenticationReturns429WhenThreadPoolIsSaturated() throws Exception { final String nodeName = randomFrom(internalCluster().getNodeNames()); final Settings settings = internalCluster().getInstance(Settings.class, nodeName); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportCreateApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportCreateApiKeyAction.java index 0a17d4e349dd7..8efea7110d407 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportCreateApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportCreateApiKeyAction.java @@ -52,7 +52,7 @@ protected void doExecute(Task task, CreateApiKeyRequest request, ActionListener< if (authentication == null) { listener.onFailure(new IllegalStateException("authentication is required")); } else { - if (Authentication.AuthenticationType.API_KEY == authentication.getAuthenticationType() && grantsAnyPrivileges(request)) { + if (authentication.isApiKey() && grantsAnyPrivileges(request)) { listener.onFailure( new IllegalArgumentException( "creating derived api keys requires an explicit role descriptor that is empty (has no privileges)" diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java index 7787809edde83..4918fd7081d91 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java @@ -72,7 +72,7 @@ protected void doExecute(Task task, CreateTokenRequest request, ActionListener authenticateAndCreateToken(type, request, listener); case CLIENT_CREDENTIALS -> { Authentication authentication = securityContext.getAuthentication(); - if (authentication.isAuthenticatedWithServiceAccount() && false == authentication.getUser().isRunAs()) { + if (authentication.isServiceAccount()) { // Service account itself cannot create OAuth2 tokens. // But it is possible to create an oauth2 token if the service account run-as a different user. // In this case, the token will be created for the run-as user (not the service account). diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateAction.java index 4c9cbcf0e5f65..1f9ed4ba2c1bc 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateAction.java @@ -55,7 +55,8 @@ protected void doExecute(Task task, AuthenticateRequest request, ActionListener< final User user = authentication.getUser(); final boolean shouldAddAnonymousRoleNames = anonymousUser.enabled() && false == anonymousUser.equals(user) - && authentication.getAuthenticationType() != Authentication.AuthenticationType.API_KEY; + && false == authentication.isApiKey() + && false == authentication.isServiceAccount(); if (shouldAddAnonymousRoleNames) { final String[] allRoleNames = Stream.concat(Stream.of(user.roles()), Stream.of(anonymousUser.roles())) .toArray(String[]::new); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java index d7669a16d9c65..1c453bf665d1a 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java @@ -1492,7 +1492,7 @@ LogEntryBuilder withXForwardedFor(ThreadContext threadContext) { LogEntryBuilder withAuthentication(Authentication authentication) { logEntry.with(PRINCIPAL_FIELD_NAME, authentication.getUser().principal()); logEntry.with(AUTHENTICATION_TYPE_FIELD_NAME, authentication.getAuthenticationType().toString()); - if (Authentication.AuthenticationType.API_KEY == authentication.getAuthenticationType()) { + if (authentication.isAuthenticatedWithApiKey()) { logEntry.with(API_KEY_ID_FIELD_NAME, (String) authentication.getMetadata().get(AuthenticationField.API_KEY_ID_KEY)); String apiKeyName = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_NAME_KEY); if (apiKeyName != null) { 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 839b3a15f3569..84927b415c9c8 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 @@ -138,7 +138,6 @@ import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; import static org.elasticsearch.xpack.core.ClientHelper.SECURITY_ORIGIN; import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin; -import static org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.SECURITY_MAIN_ALIAS; import static org.elasticsearch.xpack.security.Security.SECURITY_CRYPTO_THREAD_POOL_NAME; @@ -791,17 +790,6 @@ ApiKeyCredentials getCredentialsFromHeader(ThreadContext threadContext) { return null; } - public static boolean isApiKeyAuthentication(Authentication authentication) { - final Authentication.AuthenticationType authType = authentication.getAuthenticationType(); - if (Authentication.AuthenticationType.API_KEY == authType) { - assert AuthenticationField.API_KEY_REALM_TYPE.equals(authentication.getAuthenticatedBy().getType()) - : "API key authentication must have API key realm type"; - return true; - } else { - return false; - } - } - void computeHashForApiKey(SecureString apiKey, ActionListener listener) { threadPool.executor(SECURITY_CRYPTO_THREAD_POOL_NAME).execute(ActionRunnable.supply(listener, () -> hasher.hash(apiKey))); } @@ -1357,7 +1345,7 @@ AtomicLong getLastEvictionCheckedAt() { * @return realm name */ public static String getCreatorRealmName(final Authentication authentication) { - if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { + if (authentication.isAuthenticatedWithApiKey()) { return (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_NAME); } else { return authentication.getSourceRealm().getName(); @@ -1373,7 +1361,7 @@ public static String getCreatorRealmName(final Authentication authentication) { * @return realm type */ public static String getCreatorRealmType(final Authentication authentication) { - if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { + if (authentication.isAuthenticatedWithApiKey()) { return (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_TYPE); } else { return authentication.getSourceRealm().getType(); @@ -1387,7 +1375,7 @@ public static String getCreatorRealmType(final Authentication authentication) { * @return A map for the metadata or an empty map if no metadata is found. */ public static Map getApiKeyMetadata(Authentication authentication) { - if (AuthenticationType.API_KEY != authentication.getAuthenticationType()) { + if (false == authentication.isAuthenticatedWithApiKey()) { throw new IllegalArgumentException( "authentication type must be [api_key], got [" + authentication.getAuthenticationType().name().toLowerCase(Locale.ROOT) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountService.java index 0f3f47ca5b0ca..6b555f77cd924 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountService.java @@ -163,7 +163,7 @@ public void findTokensFor(GetServiceAccountCredentialsRequest request, ActionLis findIndexTokens(accountId, listener); } - // TODO: remove since authentication is dealt centrally by AuthenticationContext and freinds + // TODO: remove since authentication is dealt centrally by AuthenticationContext and friends public void getRoleDescriptor(Authentication authentication, ActionListener listener) { assert authentication.isAuthenticatedWithServiceAccount() : "authentication is not for service account: " + authentication; final String principal = authentication.getUser().principal(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index 8e778a538f3ad..0730b73ee4661 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -50,7 +50,6 @@ import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest; import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; import org.elasticsearch.xpack.core.security.authc.Authentication; -import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; import org.elasticsearch.xpack.core.security.authc.AuthenticationFailureHandler; import org.elasticsearch.xpack.core.security.authc.AuthenticationField; import org.elasticsearch.xpack.core.security.authc.esnative.ClientReservedRealm; @@ -886,7 +885,7 @@ private ElasticsearchSecurityException denialException( userText = userText + " run as [" + authentication.getUser().principal() + "]"; } // check for authentication by API key - if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { + if (authentication.isAuthenticatedWithApiKey()) { final String apiKeyId = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_ID_KEY); assert apiKeyId != null : "api key id must be present in the metadata"; userText = "API key id [" + apiKeyId + "] of " + userText; @@ -895,9 +894,7 @@ private ElasticsearchSecurityException denialException( // The run-as user is always from a realm. So it must have roles that can be printed. // If the user is not run-as, we cannot print the roles if it's an API key or a service account (both do not have // roles, but privileges) - if (authentication.getUser().isRunAs() - || (false == authentication.isAuthenticatedWithServiceAccount() - && AuthenticationType.API_KEY != authentication.getAuthenticationType())) { + if (false == authentication.isServiceAccount() && false == authentication.isApiKey()) { userText = userText + " with roles [" + Strings.arrayToCommaDelimitedString(authentication.getUser().roles()) + "]"; } 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 80f0c924dee78..4ee424e95a592 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 @@ -188,9 +188,8 @@ boolean checkSameUserPermissions(String action, TransportRequest request, Authen || sameUsername == false : "Action '" + action + "' should not be possible when sameUsername=" + sameUsername; return sameUsername; } else if (request instanceof GetApiKeyRequest getApiKeyRequest) { - if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { - 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 + if (authentication.isApiKey()) { + // if the authentication is an API key then the request must also contain same API key id String authenticatedApiKeyId = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_ID_KEY); if (Strings.hasText(getApiKeyRequest.getApiKeyId())) { return getApiKeyRequest.getApiKeyId().equals(authenticatedApiKeyId); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java index fcf21acc01f19..772e0a423e285 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessor.java @@ -140,7 +140,7 @@ public IngestDocument execute(IngestDocument ingestDocument) throws Exception { } break; case API_KEY: - if (Authentication.AuthenticationType.API_KEY == authentication.getAuthenticationType()) { + if (authentication.isAuthenticatedWithApiKey()) { final String apiKey = "api_key"; final Object existingApiKeyField = userObject.get(apiKey); @SuppressWarnings("unchecked") diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java index 488a52a77762f..e77f8cb83b256 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java @@ -7,9 +7,11 @@ package org.elasticsearch.xpack.security.action.user; import org.elasticsearch.ElasticsearchSecurityException; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.ArrayUtils; import org.elasticsearch.tasks.Task; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.transport.Transport; @@ -18,6 +20,7 @@ import org.elasticsearch.xpack.core.security.action.user.AuthenticateRequest; import org.elasticsearch.xpack.core.security.action.user.AuthenticateResponse; import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.security.user.AsyncSearchUser; import org.elasticsearch.xpack.core.security.user.ElasticUser; @@ -28,10 +31,17 @@ import org.elasticsearch.xpack.core.security.user.XPackUser; import java.util.Collections; -import java.util.List; +import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import static org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; +import static org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef; +import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_ID_KEY; +import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_REALM_NAME; +import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_REALM_TYPE; +import static org.hamcrest.Matchers.arrayContainingInAnyOrder; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -45,7 +55,7 @@ public void testInternalUser() { SecurityContext securityContext = mock(SecurityContext.class); final Authentication authentication = new Authentication( randomFrom(SystemUser.INSTANCE, XPackUser.INSTANCE, XPackSecurityUser.INSTANCE, AsyncSearchUser.INSTANCE), - new Authentication.RealmRef("native", "default_native", "node1"), + new RealmRef("native", "default_native", "node1"), null ); when(securityContext.getAuthentication()).thenReturn(authentication); @@ -122,36 +132,94 @@ public void onFailure(Exception e) { } public void testValidAuthentication() { + final AnonymousUser anonymousUser = prepareAnonymousUser(); + final boolean isRunAs = randomBoolean(); final User user = randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe")); + final User effectiveUser = isRunAs ? new User(user, new User("bar")) : user; + + final AuthenticationType authenticationType = isRunAs + ? randomFrom(AuthenticationType.API_KEY, AuthenticationType.REALM, AuthenticationType.TOKEN) + : randomFrom(AuthenticationType.REALM, AuthenticationType.TOKEN); + final RealmRef authenticatedBy = new RealmRef(randomAlphaOfLength(5), randomAlphaOfLength(5), randomAlphaOfLength(5)); + final RealmRef lookedUpBy = isRunAs ? new RealmRef(randomAlphaOfLength(5), randomAlphaOfLength(5), randomAlphaOfLength(5)) : null; final Authentication authentication = new Authentication( - user, - new Authentication.RealmRef("native_realm", "native", "node1"), - null + effectiveUser, + authenticatedBy, + lookedUpBy, + Version.CURRENT, + authenticationType, + authenticationType == AuthenticationType.API_KEY ? Map.of(API_KEY_ID_KEY, randomAlphaOfLength(20)) : Map.of() ); - SecurityContext securityContext = mock(SecurityContext.class); - when(securityContext.getAuthentication()).thenReturn(authentication); - when(securityContext.getUser()).thenReturn(user); + TransportAuthenticateAction action = prepareAction(anonymousUser, effectiveUser, authentication); + + final AtomicReference throwableRef = new AtomicReference<>(); + final AtomicReference responseRef = new AtomicReference<>(); + action.doExecute(mock(Task.class), new AuthenticateRequest(), new ActionListener<>() { + @Override + public void onResponse(AuthenticateResponse authenticateResponse) { + responseRef.set(authenticateResponse); + } + + @Override + public void onFailure(Exception e) { + throwableRef.set(e); + } + }); + + assertThat(responseRef.get(), notNullValue()); + if (anonymousUser.enabled()) { + final Authentication auth = responseRef.get().authentication(); + final User authUser = auth.getUser(); + assertThat( + authUser.roles(), + arrayContainingInAnyOrder(ArrayUtils.concat(authentication.getUser().roles(), anonymousUser.roles())) + ); + assertThat(authUser.authenticatedUser(), sameInstance(effectiveUser.authenticatedUser())); + assertThat(auth.getAuthenticatedBy(), sameInstance(auth.getAuthenticatedBy())); + assertThat(auth.getLookedUpBy(), sameInstance(auth.getLookedUpBy())); + assertThat(auth.getVersion(), sameInstance(auth.getVersion())); + assertThat(auth.getAuthenticationType(), sameInstance(auth.getAuthenticationType())); + assertThat(auth.getMetadata(), sameInstance(auth.getMetadata())); + } else { + assertThat(responseRef.get().authentication(), sameInstance(authentication)); + } + assertThat(throwableRef.get(), nullValue()); + } + + public void testShouldNotAddAnonymousRolesForApiKeyOrServiceAccount() { final AnonymousUser anonymousUser = prepareAnonymousUser(); - TransportService transportService = new TransportService( - Settings.EMPTY, - mock(Transport.class), - null, - TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, - null, - Collections.emptySet() - ); - TransportAuthenticateAction action = new TransportAuthenticateAction( - transportService, - mock(ActionFilters.class), - securityContext, - anonymousUser - ); + + final User user; + final Authentication authentication; + + if (randomBoolean()) { + user = new User("joe"); + authentication = new Authentication( + user, + new RealmRef(API_KEY_REALM_NAME, API_KEY_REALM_TYPE, "node1"), + null, + Version.CURRENT, + AuthenticationType.API_KEY, + Map.of(API_KEY_ID_KEY, randomAlphaOfLength(20)) + ); + } else { + user = new User("elastic/fleet-server"); + authentication = new Authentication( + user, + new RealmRef(ServiceAccountSettings.REALM_NAME, ServiceAccountSettings.REALM_TYPE, "node1"), + null, + Version.CURRENT, + AuthenticationType.TOKEN, + Map.of() + ); + } + + TransportAuthenticateAction action = prepareAction(anonymousUser, user, authentication); final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>(); - action.doExecute(mock(Task.class), new AuthenticateRequest(), new ActionListener() { + action.doExecute(mock(Task.class), new AuthenticateRequest(), new ActionListener<>() { @Override public void onResponse(AuthenticateResponse authenticateResponse) { responseRef.set(authenticateResponse); @@ -167,8 +235,7 @@ public void onFailure(Exception e) { if (anonymousUser.enabled()) { final Authentication auth = responseRef.get().authentication(); final User authUser = auth.getUser(); - List.of(authUser.roles()).containsAll(List.of(authentication.getUser().roles())); - List.of(authUser.roles()).containsAll(List.of(anonymousUser.roles())); + assertThat(authUser.roles(), emptyArray()); assertThat(authUser.authenticatedUser(), sameInstance(user.authenticatedUser())); assertThat(auth.getAuthenticatedBy(), sameInstance(auth.getAuthenticatedBy())); assertThat(auth.getLookedUpBy(), sameInstance(auth.getLookedUpBy())); @@ -181,6 +248,23 @@ public void onFailure(Exception e) { assertThat(throwableRef.get(), nullValue()); } + private TransportAuthenticateAction prepareAction(AnonymousUser anonymousUser, User user, Authentication authentication) { + SecurityContext securityContext = mock(SecurityContext.class); + when(securityContext.getAuthentication()).thenReturn(authentication); + when(securityContext.getUser()).thenReturn(user); + + TransportService transportService = new TransportService( + Settings.EMPTY, + mock(Transport.class), + null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + x -> null, + null, + Collections.emptySet() + ); + return new TransportAuthenticateAction(transportService, mock(ActionFilters.class), securityContext, anonymousUser); + } + private AnonymousUser prepareAnonymousUser() { final AnonymousUser anonymousUser = mock(AnonymousUser.class); if (randomBoolean()) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index 821281b9c5368..0a4145f3c7455 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java @@ -1434,6 +1434,7 @@ public void testApiKeyDocDeserializationWithNullValues() throws IOException { public void testGetApiKeyMetadata() throws IOException { final Authentication apiKeyAuthentication = mock(Authentication.class); when(apiKeyAuthentication.getAuthenticationType()).thenReturn(AuthenticationType.API_KEY); + when(apiKeyAuthentication.isAuthenticatedWithApiKey()).thenCallRealMethod(); final Map apiKeyMetadata = ApiKeyTests.randomMetadata(); if (apiKeyMetadata == null) { when(apiKeyAuthentication.getMetadata()).thenReturn(Map.of()); 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 05d2f85f1b519..e5d9e4bd53d11 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 @@ -329,6 +329,8 @@ public void testSameUserPermissionAllowsSelfApiKeyInfoRetrievalWhenAuthenticated when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authentication.getAuthenticationType()).thenReturn(AuthenticationType.API_KEY); when(authentication.getMetadata()).thenReturn(Map.of(AuthenticationField.API_KEY_ID_KEY, apiKeyId)); + when(authentication.isAuthenticatedWithApiKey()).thenCallRealMethod(); + when(authentication.isApiKey()).thenCallRealMethod(); assertTrue(engine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication)); } @@ -343,6 +345,8 @@ public void testSameUserPermissionDeniesApiKeyInfoRetrievalWhenAuthenticatedByAD when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authenticatedBy.getType()).thenReturn(AuthenticationField.API_KEY_REALM_TYPE); when(authentication.getMetadata()).thenReturn(Map.of(AuthenticationField.API_KEY_ID_KEY, randomAlphaOfLengthBetween(4, 7))); + when(authentication.isAuthenticatedWithApiKey()).thenCallRealMethod(); + when(authentication.isApiKey()).thenCallRealMethod(); assertFalse(engine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication)); } @@ -359,13 +363,10 @@ public void testSameUserPermissionDeniesApiKeyInfoRetrievalWhenLookedupByIsPrese when(authentication.getLookedUpBy()).thenReturn(lookedupBy); when(authentication.getAuthenticationType()).thenReturn(AuthenticationType.API_KEY); when(authentication.getMetadata()).thenReturn(Map.of(AuthenticationField.API_KEY_ID_KEY, randomAlphaOfLengthBetween(4, 7))); + when(authentication.isAuthenticatedWithApiKey()).thenCallRealMethod(); + when(authentication.isApiKey()).thenCallRealMethod(); - 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")); + assertFalse(engine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication)); } /**