Skip to content

Commit 83bfcc6

Browse files
ywangdywangd
authored
Disallow derived cross-cluster API keys (#96401)
This PR actively blocks creating cross-cluster API keys with another API key to avoid the issue of derived API key ownership. Relates: #95714
1 parent 0b8b5aa commit 83bfcc6

File tree

6 files changed

+135
-92
lines changed

6 files changed

+135
-92
lines changed

‎x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,37 @@ public void testCreateCrossClusterApiKey() throws IOException {
797797
assertThat(e.getMessage(), containsString("action [cluster:admin/xpack/security/cross_cluster/api_key/create] is unauthorized"));
798798
}
799799

800+
public void testCannotCreateDerivedCrossClusterApiKey() throws IOException {
801+
assumeTrue("untrusted remote cluster feature flag must be enabled", TcpTransport.isUntrustedRemoteClusterEnabled());
802+
803+
final Request createRestApiKeyRequest = new Request("POST", "_security/api_key");
804+
setUserForRequest(createRestApiKeyRequest, MANAGE_SECURITY_USER, END_USER_PASSWORD);
805+
createRestApiKeyRequest.setJsonEntity("{\"name\":\"rest-key\"}");
806+
final ObjectPath createRestApiKeyResponse = assertOKAndCreateObjectPath(client().performRequest(createRestApiKeyRequest));
807+
808+
final Request createDerivedRequest = new Request("POST", "/_security/cross_cluster/api_key");
809+
createDerivedRequest.setJsonEntity("""
810+
{
811+
"name": "derived-cross-cluster-key",
812+
"access": {
813+
"replication": [
814+
{
815+
"names": [ "logs" ]
816+
}
817+
]
818+
}
819+
}""");
820+
createDerivedRequest.setOptions(
821+
RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "ApiKey " + createRestApiKeyResponse.evaluate("encoded"))
822+
);
823+
final ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(createDerivedRequest));
824+
assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(400));
825+
assertThat(
826+
e.getMessage(),
827+
containsString("authentication via API key not supported: An API key cannot be used to create a cross-cluster API key")
828+
);
829+
}
830+
800831
public void testCrossClusterApiKeyDoesNotAllowEmptyAccess() throws IOException {
801832
assumeTrue("untrusted remote cluster feature flag must be enabled", TcpTransport.isUntrustedRemoteClusterEnabled());
802833

@@ -1105,41 +1136,6 @@ public void testUpdateFailureCases() throws IOException {
11051136
final ResponseException e8 = expectThrows(ResponseException.class, () -> client().performRequest(anotherUpdateRequest));
11061137
assertThat(e8.getResponse().getStatusLine().getStatusCode(), equalTo(403));
11071138
assertThat(e8.getMessage(), containsString("action [cluster:admin/xpack/security/cross_cluster/api_key/update] is unauthorized"));
1108-
1109-
// Cross-cluster API key created by another API key cannot be updated
1110-
// 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.
1111-
// Since we actively block it, we are checking it here. But it should be removed once we solve the issue of API key identity.
1112-
final Request createDerivedRequest = new Request("POST", "/_security/cross_cluster/api_key");
1113-
createDerivedRequest.setJsonEntity("""
1114-
{
1115-
"name": "derived-cross-cluster-key",
1116-
"access": {
1117-
"replication": [
1118-
{
1119-
"names": [ "logs" ]
1120-
}
1121-
]
1122-
}
1123-
}""");
1124-
createDerivedRequest.setOptions(
1125-
RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "ApiKey " + createRestApiKeyResponse.evaluate("encoded"))
1126-
);
1127-
final ObjectPath createDerivedResponse = assertOKAndCreateObjectPath(client().performRequest(createDerivedRequest));
1128-
final String derivedApiKey = createDerivedResponse.evaluate("id");
1129-
// cannot be updated by the original creator user
1130-
final Request updateDerivedRequest = new Request("PUT", "/_security/cross_cluster/api_key/" + derivedApiKey);
1131-
setUserForRequest(updateDerivedRequest, MANAGE_SECURITY_USER, END_USER_PASSWORD);
1132-
updateDerivedRequest.setJsonEntity("{\"metadata\":{}}");
1133-
final ResponseException e9 = expectThrows(ResponseException.class, () -> client().performRequest(updateDerivedRequest));
1134-
assertThat(e9.getResponse().getStatusLine().getStatusCode(), equalTo(404));
1135-
assertThat(e9.getMessage(), containsString("no API key owned by requesting user found"));
1136-
// cannot be updated by the original API key either
1137-
updateDerivedRequest.setOptions(
1138-
RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "ApiKey " + createRestApiKeyResponse.evaluate("encoded"))
1139-
);
1140-
final ResponseException e10 = expectThrows(ResponseException.class, () -> client().performRequest(updateDerivedRequest));
1141-
assertThat(e10.getResponse().getStatusLine().getStatusCode(), equalTo(400));
1142-
assertThat(e10.getMessage(), containsString("authentication via API key not supported: only the owner user can update an API key"));
11431139
}
11441140

11451141
public void testWorkflowsRestrictionSupportForApiKeys() throws IOException {

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java

Lines changed: 38 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -435,33 +435,7 @@ public void testCreateCrossClusterApiKey() throws IOException {
435435
}""");
436436

437437
final PlainActionFuture<CreateApiKeyResponse> future = new PlainActionFuture<>();
438-
// Cross-cluster API keys can be created by an API key as long as it has manage_security
439-
final boolean createWithUser = randomBoolean();
440-
if (createWithUser) {
441-
client().execute(CreateCrossClusterApiKeyAction.INSTANCE, request, future);
442-
} else {
443-
final CreateApiKeyResponse createAdminKeyResponse = new CreateApiKeyRequestBuilder(client()).setName("admin-key")
444-
.setRoleDescriptors(
445-
randomFrom(
446-
List.of(new RoleDescriptor(randomAlphaOfLengthBetween(3, 8), new String[] { "manage_security" }, null, null)),
447-
null
448-
)
449-
)
450-
.execute()
451-
.actionGet();
452-
client().filterWithHeader(
453-
Map.of(
454-
"Authorization",
455-
"ApiKey "
456-
+ Base64.getEncoder()
457-
.encodeToString(
458-
(createAdminKeyResponse.getId() + ":" + createAdminKeyResponse.getKey().toString()).getBytes(
459-
StandardCharsets.UTF_8
460-
)
461-
)
462-
)
463-
).execute(CreateCrossClusterApiKeyAction.INSTANCE, request, future);
464-
}
438+
client().execute(CreateCrossClusterApiKeyAction.INSTANCE, request, future);
465439
final CreateApiKeyResponse createApiKeyResponse = future.actionGet();
466440

467441
final String apiKeyId = createApiKeyResponse.getId();
@@ -522,11 +496,7 @@ public void testCreateCrossClusterApiKey() throws IOException {
522496
assertThat(getApiKeyInfo.getLimitedBy(), nullValue());
523497
assertThat(getApiKeyInfo.getMetadata(), anEmptyMap());
524498
assertThat(getApiKeyInfo.getUsername(), equalTo("test_user"));
525-
if (createWithUser) {
526-
assertThat(getApiKeyInfo.getRealm(), equalTo("file"));
527-
} else {
528-
assertThat(getApiKeyInfo.getRealm(), equalTo("_es_api_key"));
529-
}
499+
assertThat(getApiKeyInfo.getRealm(), equalTo("file"));
530500

531501
// Check the API key attributes with Query API
532502
final QueryApiKeyRequest queryApiKeyRequest = new QueryApiKeyRequest(
@@ -545,11 +515,7 @@ public void testCreateCrossClusterApiKey() throws IOException {
545515
assertThat(queryApiKeyInfo.getLimitedBy(), nullValue());
546516
assertThat(queryApiKeyInfo.getMetadata(), anEmptyMap());
547517
assertThat(queryApiKeyInfo.getUsername(), equalTo("test_user"));
548-
if (createWithUser) {
549-
assertThat(queryApiKeyInfo.getRealm(), equalTo("file"));
550-
} else {
551-
assertThat(queryApiKeyInfo.getRealm(), equalTo("_es_api_key"));
552-
}
518+
assertThat(queryApiKeyInfo.getRealm(), equalTo("file"));
553519
}
554520

555521
public void testUpdateCrossClusterApiKey() throws IOException {
@@ -653,6 +619,41 @@ public void testUpdateCrossClusterApiKey() throws IOException {
653619
assertThat(queryApiKeyInfo.getRealm(), equalTo("file"));
654620
}
655621

622+
// Cross-cluster API keys cannot be created by an API key even if it has manage_security privilege
623+
// This is intentional until we solve the issue of derived API key ownership
624+
public void testCannotCreateDerivedCrossClusterApiKey() throws IOException {
625+
assumeTrue("untrusted remote cluster feature flag must be enabled", TcpTransport.isUntrustedRemoteClusterEnabled());
626+
627+
final CreateApiKeyResponse createAdminKeyResponse = new CreateApiKeyRequestBuilder(client()).setName("admin-key")
628+
.setRoleDescriptors(
629+
randomFrom(
630+
List.of(new RoleDescriptor(randomAlphaOfLengthBetween(3, 8), new String[] { "manage_security" }, null, null)),
631+
null
632+
)
633+
)
634+
.execute()
635+
.actionGet();
636+
final String encoded = Base64.getEncoder()
637+
.encodeToString(
638+
(createAdminKeyResponse.getId() + ":" + createAdminKeyResponse.getKey().toString()).getBytes(StandardCharsets.UTF_8)
639+
);
640+
641+
final var request = CreateCrossClusterApiKeyRequest.withNameAndAccess(randomAlphaOfLengthBetween(3, 8), """
642+
{
643+
"search": [ {"names": ["logs"]} ]
644+
}""");
645+
646+
final PlainActionFuture<CreateApiKeyResponse> future = new PlainActionFuture<>();
647+
client().filterWithHeader(Map.of("Authorization", "ApiKey " + encoded))
648+
.execute(CreateCrossClusterApiKeyAction.INSTANCE, request, future);
649+
650+
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, future::actionGet);
651+
assertThat(
652+
e.getMessage(),
653+
containsString("authentication via API key not supported: An API key cannot be used to create a cross-cluster API key")
654+
);
655+
}
656+
656657
private GrantApiKeyRequest buildGrantApiKeyRequest(String username, SecureString password, String runAsUsername) throws IOException {
657658
final SecureString clonedPassword = password.clone();
658659
final GrantApiKeyRequest grantApiKeyRequest = new GrantApiKeyRequest();

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportCreateCrossClusterApiKeyAction.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ protected void doExecute(Task task, CreateCrossClusterApiKeyRequest request, Act
4949
final Authentication authentication = securityContext.getAuthentication();
5050
if (authentication == null) {
5151
listener.onFailure(new IllegalStateException("authentication is required"));
52+
} else if (authentication.isApiKey()) {
53+
listener.onFailure(
54+
new IllegalArgumentException(
55+
"authentication via API key not supported: An API key cannot be used to create a cross-cluster API key"
56+
)
57+
);
5258
} else {
5359
apiKeyService.createApiKey(authentication, request, Set.of(), listener);
5460
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,8 @@ public void createApiKey(
302302
Set<RoleDescriptor> userRoleDescriptors,
303303
ActionListener<CreateApiKeyResponse> listener
304304
) {
305+
assert request.getType() != ApiKey.Type.CROSS_CLUSTER || false == authentication.isApiKey()
306+
: "cannot create derived cross-cluster API keys";
305307
assert request.getType() != ApiKey.Type.CROSS_CLUSTER || userRoleDescriptors.isEmpty()
306308
: "owner user role descriptor must be empty for cross-cluster API keys";
307309
ensureEnabled();

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

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,54 +12,89 @@
1212
import org.elasticsearch.tasks.Task;
1313
import org.elasticsearch.test.ESTestCase;
1414
import org.elasticsearch.transport.TransportService;
15-
import org.elasticsearch.xcontent.XContentParser;
16-
import org.elasticsearch.xcontent.XContentParserConfiguration;
1715
import org.elasticsearch.xpack.core.security.SecurityContext;
1816
import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyResponse;
1917
import org.elasticsearch.xpack.core.security.action.apikey.CreateCrossClusterApiKeyRequest;
20-
import org.elasticsearch.xpack.core.security.action.apikey.CrossClusterApiKeyRoleDescriptorBuilder;
2118
import org.elasticsearch.xpack.core.security.authc.Authentication;
2219
import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper;
2320
import org.elasticsearch.xpack.security.authc.ApiKeyService;
2421

2522
import java.io.IOException;
2623
import java.util.Set;
2724

28-
import static org.elasticsearch.xcontent.json.JsonXContent.jsonXContent;
25+
import static org.elasticsearch.xpack.core.security.action.apikey.CreateCrossClusterApiKeyRequestTests.randomCrossClusterApiKeyAccessField;
26+
import static org.hamcrest.Matchers.containsString;
2927
import static org.mockito.ArgumentMatchers.eq;
3028
import static org.mockito.ArgumentMatchers.same;
3129
import static org.mockito.Mockito.mock;
3230
import static org.mockito.Mockito.verify;
31+
import static org.mockito.Mockito.verifyNoInteractions;
3332
import static org.mockito.Mockito.when;
3433

3534
public class TransportCreateCrossClusterApiKeyActionTests extends ESTestCase {
3635

37-
public void testApiKeyWillBeCreatedWithEmptyUserRoleDescriptors() throws IOException {
38-
final ApiKeyService apiKeyService = mock(ApiKeyService.class);
39-
final SecurityContext securityContext = mock(SecurityContext.class);
40-
final Authentication authentication = AuthenticationTestHelper.builder().build();
41-
when(securityContext.getAuthentication()).thenReturn(authentication);
42-
final var action = new TransportCreateCrossClusterApiKeyAction(
36+
private ApiKeyService apiKeyService;
37+
private SecurityContext securityContext;
38+
private TransportCreateCrossClusterApiKeyAction action;
39+
40+
@Override
41+
public void setUp() throws Exception {
42+
super.setUp();
43+
apiKeyService = mock(ApiKeyService.class);
44+
securityContext = mock(SecurityContext.class);
45+
action = new TransportCreateCrossClusterApiKeyAction(
4346
mock(TransportService.class),
4447
mock(ActionFilters.class),
4548
apiKeyService,
4649
securityContext
4750
);
51+
}
4852

49-
final XContentParser parser = jsonXContent.createParser(XContentParserConfiguration.EMPTY, """
50-
{
51-
"search": [ {"names": ["idx"]} ]
52-
}""");
53+
public void testApiKeyWillBeCreatedWithEmptyUserRoleDescriptors() throws IOException {
54+
final Authentication authentication = randomValueOtherThanMany(
55+
Authentication::isApiKey,
56+
() -> AuthenticationTestHelper.builder().build()
57+
);
58+
when(securityContext.getAuthentication()).thenReturn(authentication);
5359

54-
final CreateCrossClusterApiKeyRequest request = new CreateCrossClusterApiKeyRequest(
60+
final var request = CreateCrossClusterApiKeyRequest.withNameAndAccess(
5561
randomAlphaOfLengthBetween(3, 8),
56-
CrossClusterApiKeyRoleDescriptorBuilder.PARSER.parse(parser, null),
57-
null,
58-
null
62+
randomCrossClusterApiKeyAccessField()
5963
);
60-
6164
final PlainActionFuture<CreateApiKeyResponse> future = new PlainActionFuture<>();
6265
action.doExecute(mock(Task.class), request, future);
6366
verify(apiKeyService).createApiKey(same(authentication), same(request), eq(Set.of()), same(future));
6467
}
68+
69+
public void testAuthenticationIsRequired() throws IOException {
70+
final var request = CreateCrossClusterApiKeyRequest.withNameAndAccess(
71+
randomAlphaOfLengthBetween(3, 8),
72+
randomCrossClusterApiKeyAccessField()
73+
);
74+
final PlainActionFuture<CreateApiKeyResponse> future = new PlainActionFuture<>();
75+
action.doExecute(mock(Task.class), request, future);
76+
77+
final IllegalStateException e = expectThrows(IllegalStateException.class, future::actionGet);
78+
assertThat(e.getMessage(), containsString("authentication is required"));
79+
verifyNoInteractions(apiKeyService);
80+
}
81+
82+
public void testCannotCreateDerivedCrossClusterApiKey() throws IOException {
83+
final Authentication authentication = AuthenticationTestHelper.builder().apiKey().build();
84+
when(securityContext.getAuthentication()).thenReturn(authentication);
85+
86+
final var request = CreateCrossClusterApiKeyRequest.withNameAndAccess(
87+
randomAlphaOfLengthBetween(3, 8),
88+
randomCrossClusterApiKeyAccessField()
89+
);
90+
final PlainActionFuture<CreateApiKeyResponse> future = new PlainActionFuture<>();
91+
action.doExecute(mock(Task.class), request, future);
92+
93+
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, future::actionGet);
94+
assertThat(
95+
e.getMessage(),
96+
containsString("authentication via API key not supported: An API key cannot be used to create a cross-cluster API key")
97+
);
98+
verifyNoInteractions(apiKeyService);
99+
}
65100
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2391,7 +2391,10 @@ public void testBuildDelimitedStringWithLimit() {
23912391
}
23922392

23932393
public void testCreateCrossClusterApiKeyMinVersionConstraint() {
2394-
final Authentication authentication = AuthenticationTestHelper.builder().build();
2394+
final Authentication authentication = randomValueOtherThanMany(
2395+
Authentication::isApiKey,
2396+
() -> AuthenticationTestHelper.builder().build()
2397+
);
23952398
final AbstractCreateApiKeyRequest request = mock(AbstractCreateApiKeyRequest.class);
23962399
when(request.getType()).thenReturn(ApiKey.Type.CROSS_CLUSTER);
23972400

0 commit comments

Comments
 (0)