Disallow derived cross-cluster API keys

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 {

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<CreateApiKeyResponse> 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<CreateApiKeyResponse> 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();

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);
         }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

@@ -302,6 +302,8 @@ public void createApiKey(
         Set<RoleDescriptor> userRoleDescriptors,
         ActionListener<CreateApiKeyResponse> 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();

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyActionTests.java

@@ -12,54 +12,89 @@
 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;
 
 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<CreateApiKeyResponse> 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<CreateApiKeyResponse> 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<CreateApiKeyResponse> 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);
+    }
 }

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);