Skip to content

Commit b67863e

Browse files
ywangdywangd
authored
Explicitly require that derived API keys have no privileges (elastic#53647)
The current implicit behaviour is that when an API keys is used to create another API key, the child key is created without any privilege. This implicit behaviour is surprising and is a source of confusion for users. This change makes that behaviour explicit.
1 parent 3608be6 commit b67863e

File tree

4 files changed

+146
-0
lines changed

4 files changed

+146
-0
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,16 @@ public int hashCode() {
212212
return result;
213213
}
214214

215+
216+
public boolean isEmpty() {
217+
return clusterPrivileges.length == 0
218+
&& configurableClusterPrivileges.length == 0
219+
&& indicesPrivileges.length == 0
220+
&& applicationPrivileges.length == 0
221+
&& runAs.length == 0
222+
&& metadata.size() == 0;
223+
}
224+
215225
@Override
216226
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
217227
return toXContent(builder, params, false);

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
1919
import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse;
2020
import org.elasticsearch.xpack.core.security.authc.Authentication;
21+
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
2122
import org.elasticsearch.xpack.security.authc.ApiKeyService;
2223
import org.elasticsearch.xpack.security.authc.support.ApiKeyGenerator;
2324
import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
@@ -44,7 +45,18 @@ protected void doExecute(Task task, CreateApiKeyRequest request, ActionListener<
4445
if (authentication == null) {
4546
listener.onFailure(new IllegalStateException("authentication is required"));
4647
} else {
48+
if (Authentication.AuthenticationType.API_KEY == authentication.getAuthenticationType() && grantsAnyPrivileges(request)) {
49+
listener.onFailure(new IllegalArgumentException(
50+
"creating derived api keys requires an explicit role descriptor that is empty (has no privileges)"));
51+
return;
52+
}
4753
generator.generateApiKey(authentication, request, listener);
4854
}
4955
}
56+
57+
private boolean grantsAnyPrivileges(CreateApiKeyRequest request) {
58+
return request.getRoleDescriptors() == null
59+
|| request.getRoleDescriptors().isEmpty()
60+
|| false == request.getRoleDescriptors().stream().allMatch(RoleDescriptor::isEmpty);
61+
}
5062
}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
import com.google.common.collect.Sets;
1010
import org.elasticsearch.ElasticsearchSecurityException;
1111
import org.elasticsearch.action.DocWriteResponse;
12+
import org.elasticsearch.action.admin.indices.refresh.RefreshAction;
13+
import org.elasticsearch.action.admin.indices.refresh.RefreshRequestBuilder;
1214
import org.elasticsearch.action.admin.indices.refresh.RefreshResponse;
1315
import org.elasticsearch.action.support.PlainActionFuture;
1416
import org.elasticsearch.action.support.WriteRequest;
@@ -758,6 +760,75 @@ public void testApiKeyWithManageOwnPrivilegeIsAbleToInvalidateItselfButNotAnyOth
758760
assertThat(invalidateResponse.getErrors().size(), equalTo(0));
759761
}
760762

763+
public void testDerivedKeys() throws ExecutionException, InterruptedException {
764+
Client client = client().filterWithHeader(Collections.singletonMap("Authorization",
765+
UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER,
766+
SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
767+
final CreateApiKeyResponse response = new CreateApiKeyRequestBuilder(client)
768+
.setName("key-1")
769+
.setRoleDescriptors(Collections.singletonList(
770+
new RoleDescriptor("role", new String[] { "manage_api_key" }, null, null)))
771+
.get();
772+
773+
assertEquals("key-1", response.getName());
774+
assertNotNull(response.getId());
775+
assertNotNull(response.getKey());
776+
777+
// use the first ApiKey for authorized action
778+
final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString(
779+
(response.getId() + ":" + response.getKey().toString()).getBytes(StandardCharsets.UTF_8));
780+
final Client clientKey1 = client().filterWithHeader(Collections.singletonMap("Authorization", "ApiKey " + base64ApiKeyKeyValue));
781+
782+
final String expectedMessage = "creating derived api keys requires an explicit role descriptor that is empty";
783+
784+
final IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class,
785+
() -> new CreateApiKeyRequestBuilder(clientKey1).setName("key-2").get());
786+
assertThat(e1.getMessage(), containsString(expectedMessage));
787+
788+
final IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class,
789+
() -> new CreateApiKeyRequestBuilder(clientKey1).setName("key-3")
790+
.setRoleDescriptors(Collections.emptyList()).get());
791+
assertThat(e2.getMessage(), containsString(expectedMessage));
792+
793+
final IllegalArgumentException e3 = expectThrows(IllegalArgumentException.class,
794+
() -> new CreateApiKeyRequestBuilder(clientKey1).setName("key-4")
795+
.setRoleDescriptors(Collections.singletonList(
796+
new RoleDescriptor("role", new String[] {"manage_own_api_key"}, null, null)
797+
)).get());
798+
assertThat(e3.getMessage(), containsString(expectedMessage));
799+
800+
final List<RoleDescriptor> roleDescriptors = randomList(2, 10,
801+
() -> new RoleDescriptor("role", null, null, null));
802+
roleDescriptors.set(randomInt(roleDescriptors.size() - 1),
803+
new RoleDescriptor("role", new String[] {"manage_own_api_key"}, null, null));
804+
805+
final IllegalArgumentException e4 = expectThrows(IllegalArgumentException.class,
806+
() -> new CreateApiKeyRequestBuilder(clientKey1).setName("key-5")
807+
.setRoleDescriptors(roleDescriptors).get());
808+
assertThat(e4.getMessage(), containsString(expectedMessage));
809+
810+
final CreateApiKeyResponse key100Response = new CreateApiKeyRequestBuilder(clientKey1).setName("key-100")
811+
.setRoleDescriptors(Collections.singletonList(
812+
new RoleDescriptor("role", null, null, null)
813+
)).get();
814+
assertEquals("key-100", key100Response.getName());
815+
assertNotNull(key100Response.getId());
816+
assertNotNull(key100Response.getKey());
817+
818+
// Check at the end to allow sometime for the operation to happen. Since an erroneous creation is
819+
// asynchronous so that the document is not available immediately.
820+
assertApiKeyNotCreated(client, "key-2");
821+
assertApiKeyNotCreated(client, "key-3");
822+
assertApiKeyNotCreated(client, "key-4");
823+
assertApiKeyNotCreated(client, "key-5");
824+
}
825+
826+
private void assertApiKeyNotCreated(Client client, String keyName) throws ExecutionException, InterruptedException {
827+
new RefreshRequestBuilder(client, RefreshAction.INSTANCE).setIndices(SECURITY_MAIN_ALIAS).execute().get();
828+
assertEquals(0, client.execute(GetApiKeyAction.INSTANCE,
829+
GetApiKeyRequest.usingApiKeyName(keyName, false)).get().getApiKeyInfos().length);
830+
}
831+
761832
private void verifyGetResponse(int expectedNumberOfApiKeys, List<CreateApiKeyResponse> responses,
762833
GetApiKeyResponse response, Set<String> validApiKeyIds, List<String> invalidatedApiKeyIds) {
763834
verifyGetResponse(SecuritySettingsSource.TEST_SUPERUSER, expectedNumberOfApiKeys, responses, response, validApiKeyIds,

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RoleDescriptorTests.java

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@
3232
import java.io.IOException;
3333
import java.util.Arrays;
3434
import java.util.Collections;
35+
import java.util.HashMap;
3536
import java.util.LinkedHashSet;
37+
import java.util.List;
3638
import java.util.Map;
3739

3840
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -325,4 +327,55 @@ public void testParseIndicesPrivilegesFailsWhenExceptFieldsAreNotSubsetOfGranted
325327
assertThat(epe, TestMatchers.throwableWithMessage(containsString("f2")));
326328
assertThat(epe, TestMatchers.throwableWithMessage(containsString("f3")));
327329
}
330+
331+
public void testIsEmpty() {
332+
assertTrue(new RoleDescriptor(
333+
randomAlphaOfLengthBetween(1, 10), null, null, null, null, null, null, null)
334+
.isEmpty());
335+
336+
assertTrue(new RoleDescriptor(
337+
randomAlphaOfLengthBetween(1, 10),
338+
new String[0],
339+
new RoleDescriptor.IndicesPrivileges[0],
340+
new RoleDescriptor.ApplicationResourcePrivileges[0],
341+
new ConfigurableClusterPrivilege[0],
342+
new String[0],
343+
new HashMap<>(),
344+
new HashMap<>())
345+
.isEmpty());
346+
347+
final List<Boolean> booleans = Arrays.asList(
348+
randomBoolean(),
349+
randomBoolean(),
350+
randomBoolean(),
351+
randomBoolean(),
352+
randomBoolean(),
353+
randomBoolean());
354+
355+
final RoleDescriptor roleDescriptor = new RoleDescriptor(
356+
randomAlphaOfLengthBetween(1, 10),
357+
booleans.get(0) ? new String[0] : new String[] { "foo" },
358+
booleans.get(1) ?
359+
new RoleDescriptor.IndicesPrivileges[0] :
360+
new RoleDescriptor.IndicesPrivileges[] {
361+
RoleDescriptor.IndicesPrivileges.builder().indices("idx").privileges("foo").build() },
362+
booleans.get(2) ?
363+
new RoleDescriptor.ApplicationResourcePrivileges[0] :
364+
new RoleDescriptor.ApplicationResourcePrivileges[] {
365+
RoleDescriptor.ApplicationResourcePrivileges.builder()
366+
.application("app").privileges("foo").resources("res").build() },
367+
booleans.get(3) ?
368+
new ConfigurableClusterPrivilege[0] :
369+
new ConfigurableClusterPrivilege[] {
370+
new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Collections.singleton("foo")) },
371+
booleans.get(4) ? new String[0] : new String[] { "foo" },
372+
booleans.get(5) ? new HashMap<>() : Collections.singletonMap("foo", "bar"),
373+
Collections.singletonMap("foo", "bar"));
374+
375+
if (booleans.stream().anyMatch(e -> e.equals(false))) {
376+
assertFalse(roleDescriptor.isEmpty());
377+
} else {
378+
assertTrue(roleDescriptor.isEmpty());
379+
}
380+
}
328381
}

0 commit comments

Comments
 (0)