diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java index d1e836ece480f..729b2f2652c9e 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java @@ -797,6 +797,37 @@ public void testCreateCrossClusterApiKey() throws IOException { assertThat(e.getMessage(), containsString("action [cluster:admin/xpack/security/cross_cluster/api_key/create] is unauthorized")); } + public void testCannotCreateDerivedCrossClusterApiKey() throws IOException { + assumeTrue("untrusted remote cluster feature flag must be enabled", TcpTransport.isUntrustedRemoteClusterEnabled()); + + final Request createRestApiKeyRequest = new Request("POST", "_security/api_key"); + setUserForRequest(createRestApiKeyRequest, MANAGE_SECURITY_USER, END_USER_PASSWORD); + createRestApiKeyRequest.setJsonEntity("{\"name\":\"rest-key\"}"); + final ObjectPath createRestApiKeyResponse = assertOKAndCreateObjectPath(client().performRequest(createRestApiKeyRequest)); + + final Request createDerivedRequest = new Request("POST", "/_security/cross_cluster/api_key"); + createDerivedRequest.setJsonEntity(""" + { + "name": "derived-cross-cluster-key", + "access": { + "replication": [ + { + "names": [ "logs" ] + } + ] + } + }"""); + createDerivedRequest.setOptions( + RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "ApiKey " + createRestApiKeyResponse.evaluate("encoded")) + ); + final ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(createDerivedRequest)); + assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(400)); + assertThat( + e.getMessage(), + containsString("authentication via API key not supported: An API key cannot be used to create a cross-cluster API key") + ); + } + public void testCrossClusterApiKeyDoesNotAllowEmptyAccess() throws IOException { assumeTrue("untrusted remote cluster feature flag must be enabled", TcpTransport.isUntrustedRemoteClusterEnabled()); @@ -1105,41 +1136,6 @@ public void testUpdateFailureCases() throws IOException { final ResponseException e8 = expectThrows(ResponseException.class, () -> client().performRequest(anotherUpdateRequest)); assertThat(e8.getResponse().getStatusLine().getStatusCode(), equalTo(403)); assertThat(e8.getMessage(), containsString("action [cluster:admin/xpack/security/cross_cluster/api_key/update] is unauthorized")); - - // Cross-cluster API key created by another API key cannot be updated - // This isn't the desired behaviour and more like a bug because we don't yet have a full story about API key's identity. - // Since we actively block it, we are checking it here. But it should be removed once we solve the issue of API key identity. - final Request createDerivedRequest = new Request("POST", "/_security/cross_cluster/api_key"); - createDerivedRequest.setJsonEntity(""" - { - "name": "derived-cross-cluster-key", - "access": { - "replication": [ - { - "names": [ "logs" ] - } - ] - } - }"""); - createDerivedRequest.setOptions( - RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "ApiKey " + createRestApiKeyResponse.evaluate("encoded")) - ); - final ObjectPath createDerivedResponse = assertOKAndCreateObjectPath(client().performRequest(createDerivedRequest)); - final String derivedApiKey = createDerivedResponse.evaluate("id"); - // cannot be updated by the original creator user - final Request updateDerivedRequest = new Request("PUT", "/_security/cross_cluster/api_key/" + derivedApiKey); - setUserForRequest(updateDerivedRequest, MANAGE_SECURITY_USER, END_USER_PASSWORD); - updateDerivedRequest.setJsonEntity("{\"metadata\":{}}"); - final ResponseException e9 = expectThrows(ResponseException.class, () -> client().performRequest(updateDerivedRequest)); - assertThat(e9.getResponse().getStatusLine().getStatusCode(), equalTo(404)); - assertThat(e9.getMessage(), containsString("no API key owned by requesting user found")); - // cannot be updated by the original API key either - updateDerivedRequest.setOptions( - RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "ApiKey " + createRestApiKeyResponse.evaluate("encoded")) - ); - final ResponseException e10 = expectThrows(ResponseException.class, () -> client().performRequest(updateDerivedRequest)); - assertThat(e10.getResponse().getStatusLine().getStatusCode(), equalTo(400)); - assertThat(e10.getMessage(), containsString("authentication via API key not supported: only the owner user can update an API key")); } public void testWorkflowsRestrictionSupportForApiKeys() throws IOException { diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java index 44affb667f61f..22d2686a744f1 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java @@ -435,33 +435,7 @@ public void testCreateCrossClusterApiKey() throws IOException { }"""); final PlainActionFuture future = new PlainActionFuture<>(); - // Cross-cluster API keys can be created by an API key as long as it has manage_security - final boolean createWithUser = randomBoolean(); - if (createWithUser) { - client().execute(CreateCrossClusterApiKeyAction.INSTANCE, request, future); - } else { - final CreateApiKeyResponse createAdminKeyResponse = new CreateApiKeyRequestBuilder(client()).setName("admin-key") - .setRoleDescriptors( - randomFrom( - List.of(new RoleDescriptor(randomAlphaOfLengthBetween(3, 8), new String[] { "manage_security" }, null, null)), - null - ) - ) - .execute() - .actionGet(); - client().filterWithHeader( - Map.of( - "Authorization", - "ApiKey " - + Base64.getEncoder() - .encodeToString( - (createAdminKeyResponse.getId() + ":" + createAdminKeyResponse.getKey().toString()).getBytes( - StandardCharsets.UTF_8 - ) - ) - ) - ).execute(CreateCrossClusterApiKeyAction.INSTANCE, request, future); - } + client().execute(CreateCrossClusterApiKeyAction.INSTANCE, request, future); final CreateApiKeyResponse createApiKeyResponse = future.actionGet(); final String apiKeyId = createApiKeyResponse.getId(); @@ -522,11 +496,7 @@ public void testCreateCrossClusterApiKey() throws IOException { assertThat(getApiKeyInfo.getLimitedBy(), nullValue()); assertThat(getApiKeyInfo.getMetadata(), anEmptyMap()); assertThat(getApiKeyInfo.getUsername(), equalTo("test_user")); - if (createWithUser) { - assertThat(getApiKeyInfo.getRealm(), equalTo("file")); - } else { - assertThat(getApiKeyInfo.getRealm(), equalTo("_es_api_key")); - } + assertThat(getApiKeyInfo.getRealm(), equalTo("file")); // Check the API key attributes with Query API final QueryApiKeyRequest queryApiKeyRequest = new QueryApiKeyRequest( @@ -545,11 +515,7 @@ public void testCreateCrossClusterApiKey() throws IOException { assertThat(queryApiKeyInfo.getLimitedBy(), nullValue()); assertThat(queryApiKeyInfo.getMetadata(), anEmptyMap()); assertThat(queryApiKeyInfo.getUsername(), equalTo("test_user")); - if (createWithUser) { - assertThat(queryApiKeyInfo.getRealm(), equalTo("file")); - } else { - assertThat(queryApiKeyInfo.getRealm(), equalTo("_es_api_key")); - } + assertThat(queryApiKeyInfo.getRealm(), equalTo("file")); } public void testUpdateCrossClusterApiKey() throws IOException { @@ -653,6 +619,41 @@ public void testUpdateCrossClusterApiKey() throws IOException { assertThat(queryApiKeyInfo.getRealm(), equalTo("file")); } + // Cross-cluster API keys cannot be created by an API key even if it has manage_security privilege + // This is intentional until we solve the issue of derived API key ownership + public void testCannotCreateDerivedCrossClusterApiKey() throws IOException { + assumeTrue("untrusted remote cluster feature flag must be enabled", TcpTransport.isUntrustedRemoteClusterEnabled()); + + final CreateApiKeyResponse createAdminKeyResponse = new CreateApiKeyRequestBuilder(client()).setName("admin-key") + .setRoleDescriptors( + randomFrom( + List.of(new RoleDescriptor(randomAlphaOfLengthBetween(3, 8), new String[] { "manage_security" }, null, null)), + null + ) + ) + .execute() + .actionGet(); + final String encoded = Base64.getEncoder() + .encodeToString( + (createAdminKeyResponse.getId() + ":" + createAdminKeyResponse.getKey().toString()).getBytes(StandardCharsets.UTF_8) + ); + + final var request = CreateCrossClusterApiKeyRequest.withNameAndAccess(randomAlphaOfLengthBetween(3, 8), """ + { + "search": [ {"names": ["logs"]} ] + }"""); + + final PlainActionFuture future = new PlainActionFuture<>(); + client().filterWithHeader(Map.of("Authorization", "ApiKey " + encoded)) + .execute(CreateCrossClusterApiKeyAction.INSTANCE, request, future); + + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, future::actionGet); + assertThat( + e.getMessage(), + containsString("authentication via API key not supported: An API key cannot be used to create a cross-cluster API key") + ); + } + private GrantApiKeyRequest buildGrantApiKeyRequest(String username, SecureString password, String runAsUsername) throws IOException { final SecureString clonedPassword = password.clone(); final GrantApiKeyRequest grantApiKeyRequest = new GrantApiKeyRequest(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyAction.java index 2c0df3cd59dfc..09f1d42ddf83e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyAction.java @@ -49,6 +49,12 @@ protected void doExecute(Task task, CreateCrossClusterApiKeyRequest request, Act final Authentication authentication = securityContext.getAuthentication(); if (authentication == null) { listener.onFailure(new IllegalStateException("authentication is required")); + } else if (authentication.isApiKey()) { + listener.onFailure( + new IllegalArgumentException( + "authentication via API key not supported: An API key cannot be used to create a cross-cluster API key" + ) + ); } else { apiKeyService.createApiKey(authentication, request, Set.of(), listener); } 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 faac576569e1d..def2c74f03b67 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 @@ -302,6 +302,8 @@ public void createApiKey( Set userRoleDescriptors, ActionListener listener ) { + assert request.getType() != ApiKey.Type.CROSS_CLUSTER || false == authentication.isApiKey() + : "cannot create derived cross-cluster API keys"; assert request.getType() != ApiKey.Type.CROSS_CLUSTER || userRoleDescriptors.isEmpty() : "owner user role descriptor must be empty for cross-cluster API keys"; ensureEnabled(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyActionTests.java index 26b594bca9a01..e2d1fc3a24568 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyActionTests.java @@ -12,12 +12,9 @@ import org.elasticsearch.tasks.Task; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xcontent.XContentParser; -import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xpack.core.security.SecurityContext; import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyResponse; import org.elasticsearch.xpack.core.security.action.apikey.CreateCrossClusterApiKeyRequest; -import org.elasticsearch.xpack.core.security.action.apikey.CrossClusterApiKeyRoleDescriptorBuilder; import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper; import org.elasticsearch.xpack.security.authc.ApiKeyService; @@ -25,41 +22,79 @@ import java.io.IOException; import java.util.Set; -import static org.elasticsearch.xcontent.json.JsonXContent.jsonXContent; +import static org.elasticsearch.xpack.core.security.action.apikey.CreateCrossClusterApiKeyRequestTests.randomCrossClusterApiKeyAccessField; +import static org.hamcrest.Matchers.containsString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; public class TransportCreateCrossClusterApiKeyActionTests extends ESTestCase { - public void testApiKeyWillBeCreatedWithEmptyUserRoleDescriptors() throws IOException { - final ApiKeyService apiKeyService = mock(ApiKeyService.class); - final SecurityContext securityContext = mock(SecurityContext.class); - final Authentication authentication = AuthenticationTestHelper.builder().build(); - when(securityContext.getAuthentication()).thenReturn(authentication); - final var action = new TransportCreateCrossClusterApiKeyAction( + private ApiKeyService apiKeyService; + private SecurityContext securityContext; + private TransportCreateCrossClusterApiKeyAction action; + + @Override + public void setUp() throws Exception { + super.setUp(); + apiKeyService = mock(ApiKeyService.class); + securityContext = mock(SecurityContext.class); + action = new TransportCreateCrossClusterApiKeyAction( mock(TransportService.class), mock(ActionFilters.class), apiKeyService, securityContext ); + } - final XContentParser parser = jsonXContent.createParser(XContentParserConfiguration.EMPTY, """ - { - "search": [ {"names": ["idx"]} ] - }"""); + public void testApiKeyWillBeCreatedWithEmptyUserRoleDescriptors() throws IOException { + final Authentication authentication = randomValueOtherThanMany( + Authentication::isApiKey, + () -> AuthenticationTestHelper.builder().build() + ); + when(securityContext.getAuthentication()).thenReturn(authentication); - final CreateCrossClusterApiKeyRequest request = new CreateCrossClusterApiKeyRequest( + final var request = CreateCrossClusterApiKeyRequest.withNameAndAccess( randomAlphaOfLengthBetween(3, 8), - CrossClusterApiKeyRoleDescriptorBuilder.PARSER.parse(parser, null), - null, - null + randomCrossClusterApiKeyAccessField() ); - final PlainActionFuture future = new PlainActionFuture<>(); action.doExecute(mock(Task.class), request, future); verify(apiKeyService).createApiKey(same(authentication), same(request), eq(Set.of()), same(future)); } + + public void testAuthenticationIsRequired() throws IOException { + final var request = CreateCrossClusterApiKeyRequest.withNameAndAccess( + randomAlphaOfLengthBetween(3, 8), + randomCrossClusterApiKeyAccessField() + ); + final PlainActionFuture future = new PlainActionFuture<>(); + action.doExecute(mock(Task.class), request, future); + + final IllegalStateException e = expectThrows(IllegalStateException.class, future::actionGet); + assertThat(e.getMessage(), containsString("authentication is required")); + verifyNoInteractions(apiKeyService); + } + + public void testCannotCreateDerivedCrossClusterApiKey() throws IOException { + final Authentication authentication = AuthenticationTestHelper.builder().apiKey().build(); + when(securityContext.getAuthentication()).thenReturn(authentication); + + final var request = CreateCrossClusterApiKeyRequest.withNameAndAccess( + randomAlphaOfLengthBetween(3, 8), + randomCrossClusterApiKeyAccessField() + ); + final PlainActionFuture future = new PlainActionFuture<>(); + action.doExecute(mock(Task.class), request, future); + + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, future::actionGet); + assertThat( + e.getMessage(), + containsString("authentication via API key not supported: An API key cannot be used to create a cross-cluster API key") + ); + verifyNoInteractions(apiKeyService); + } } 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 06bf0c23c4ad2..12e3a7e1b5bd2 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 @@ -2391,7 +2391,10 @@ public void testBuildDelimitedStringWithLimit() { } public void testCreateCrossClusterApiKeyMinVersionConstraint() { - final Authentication authentication = AuthenticationTestHelper.builder().build(); + final Authentication authentication = randomValueOtherThanMany( + Authentication::isApiKey, + () -> AuthenticationTestHelper.builder().build() + ); final AbstractCreateApiKeyRequest request = mock(AbstractCreateApiKeyRequest.class); when(request.getType()).thenReturn(ApiKey.Type.CROSS_CLUSTER);