Skip to content

Commit 1c5f530

Browse files
ywangdywangd
authored
Explicitly require that derived API keys have no privileges (#53647) (#53650)
* Explicitly require that derived API keys have no privileges (#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 17d968d commit 1c5f530

File tree

4 files changed

+153
-0
lines changed

4 files changed

+153
-0
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,15 @@ public int hashCode() {
216216
return result;
217217
}
218218

219+
public boolean isEmpty() {
220+
return clusterPrivileges.length == 0
221+
&& conditionalClusterPrivileges.length == 0
222+
&& indicesPrivileges.length == 0
223+
&& applicationPrivileges.length == 0
224+
&& runAs.length == 0
225+
&& metadata.size() == 0;
226+
}
227+
219228
@Override
220229
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
221230
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
@@ -19,6 +19,7 @@
1919
import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
2020
import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse;
2121
import org.elasticsearch.xpack.core.security.authc.Authentication;
22+
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
2223
import org.elasticsearch.xpack.security.authc.ApiKeyService;
2324
import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
2425

@@ -51,9 +52,20 @@ protected void doExecute(CreateApiKeyRequest request, ActionListener<CreateApiKe
5152
if (authentication == null) {
5253
listener.onFailure(new IllegalStateException("authentication is required"));
5354
} else {
55+
if (Authentication.AuthenticationType.API_KEY == authentication.getAuthenticationType() && grantsAnyPrivileges(request)) {
56+
listener.onFailure(new IllegalArgumentException(
57+
"creating derived api keys requires an explicit role descriptor that is empty (has no privileges)"));
58+
return;
59+
}
5460
rolesStore.getRoleDescriptors(new HashSet<>(Arrays.asList(authentication.getUser().roles())),
5561
ActionListener.wrap(roleDescriptors -> apiKeyService.createApiKey(authentication, request, roleDescriptors, listener),
5662
listener::onFailure));
5763
}
5864
}
65+
66+
private boolean grantsAnyPrivileges(CreateApiKeyRequest request) {
67+
return request.getRoleDescriptors() == null
68+
|| request.getRoleDescriptors().isEmpty()
69+
|| false == request.getRoleDescriptors().stream().allMatch(RoleDescriptor::isEmpty);
70+
}
5971
}

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
import org.elasticsearch.ElasticsearchSecurityException;
1212
import org.elasticsearch.action.DocWriteResponse;
1313
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
14+
import org.elasticsearch.action.admin.indices.refresh.RefreshAction;
15+
import org.elasticsearch.action.admin.indices.refresh.RefreshRequestBuilder;
1416
import org.elasticsearch.action.admin.indices.refresh.RefreshResponse;
1517
import org.elasticsearch.action.support.PlainActionFuture;
1618
import org.elasticsearch.action.support.WriteRequest;
@@ -52,6 +54,7 @@
5254
import java.util.concurrent.TimeUnit;
5355
import java.util.stream.Collectors;
5456

57+
import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.SECURITY_INDEX_NAME;
5558
import static org.hamcrest.Matchers.containsInAnyOrder;
5659
import static org.hamcrest.Matchers.containsString;
5760
import static org.hamcrest.Matchers.equalTo;
@@ -518,6 +521,82 @@ public void testGetApiKeysForApiKeyName() throws InterruptedException, Execution
518521
verifyGetResponse(1, responses, response, Collections.singleton(responses.get(0).getId()), null);
519522
}
520523

524+
public void testDerivedKeys() throws ExecutionException, InterruptedException {
525+
final Client client = client().filterWithHeader(Collections.singletonMap("Authorization",
526+
UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER,
527+
SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
528+
529+
final CreateApiKeyResponse response = new SecurityClient(client)
530+
.prepareCreateApiKey()
531+
.setName("key-1")
532+
.setRoleDescriptors(Collections.singletonList(
533+
new RoleDescriptor("role", new String[] { "manage_security" }, null, null)))
534+
.get();
535+
536+
assertEquals("key-1", response.getName());
537+
assertNotNull(response.getId());
538+
assertNotNull(response.getKey());
539+
540+
// use the first ApiKey for authorized action
541+
final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString(
542+
(response.getId() + ":" + response.getKey().toString()).getBytes(StandardCharsets.UTF_8));
543+
final SecurityClient clientKey1 = new SecurityClient(
544+
client().filterWithHeader(Collections.singletonMap("Authorization", "ApiKey " + base64ApiKeyKeyValue)));
545+
546+
final String expectedMessage = "creating derived api keys requires an explicit role descriptor that is empty";
547+
548+
final IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class,
549+
() -> clientKey1.prepareCreateApiKey().setName("key-2").get());
550+
assertThat(e1.getMessage(), containsString(expectedMessage));
551+
552+
final IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class,
553+
() -> clientKey1.prepareCreateApiKey().setName("key-3")
554+
.setRoleDescriptors(Collections.emptyList()).get());
555+
assertThat(e2.getMessage(), containsString(expectedMessage));
556+
557+
final IllegalArgumentException e3 = expectThrows(IllegalArgumentException.class,
558+
() -> clientKey1.prepareCreateApiKey().setName("key-4")
559+
.setRoleDescriptors(Collections.singletonList(
560+
new RoleDescriptor("role", new String[] {"manage_security"}, null, null)
561+
)).get());
562+
assertThat(e3.getMessage(), containsString(expectedMessage));
563+
564+
final List<RoleDescriptor> roleDescriptors = new ArrayList<>();
565+
for (int i = 0; i < randomIntBetween(2, 10); i++) {
566+
roleDescriptors.add(new RoleDescriptor("role", null, null, null));
567+
}
568+
roleDescriptors.set(randomInt(roleDescriptors.size() - 1),
569+
new RoleDescriptor("role", new String[] {"manage_security"}, null, null));
570+
571+
final IllegalArgumentException e4 = expectThrows(IllegalArgumentException.class,
572+
() -> clientKey1.prepareCreateApiKey().setName("key-5")
573+
.setRoleDescriptors(roleDescriptors).get());
574+
assertThat(e4.getMessage(), containsString(expectedMessage));
575+
576+
final CreateApiKeyResponse key100Response = clientKey1.prepareCreateApiKey().setName("key-100")
577+
.setRoleDescriptors(Collections.singletonList(
578+
new RoleDescriptor("role", null, null, null)
579+
)).get();
580+
assertEquals("key-100", key100Response.getName());
581+
assertNotNull(key100Response.getId());
582+
assertNotNull(key100Response.getKey());
583+
584+
// Check at the end to allow sometime for the operation to happen. Since an erroneous creation is
585+
// asynchronous so that the document is not available immediately.
586+
assertApiKeyNotCreated(client,"key-2");
587+
assertApiKeyNotCreated(client,"key-3");
588+
assertApiKeyNotCreated(client,"key-4");
589+
assertApiKeyNotCreated(client,"key-5");
590+
}
591+
592+
private void assertApiKeyNotCreated(Client client, String keyName) throws ExecutionException, InterruptedException {
593+
new RefreshRequestBuilder(client, RefreshAction.INSTANCE).setIndices(SECURITY_INDEX_NAME).execute().get();
594+
PlainActionFuture<GetApiKeyResponse> getApiKeyResponseListener = new PlainActionFuture<>();
595+
new SecurityClient(client).getApiKey(
596+
GetApiKeyRequest.usingApiKeyName(keyName), getApiKeyResponseListener);
597+
assertEquals(0, getApiKeyResponseListener.get().getApiKeyInfos().length);
598+
}
599+
521600
private void verifyGetResponse(int noOfApiKeys, List<CreateApiKeyResponse> responses, GetApiKeyResponse response,
522601
Set<String> validApiKeyIds,
523602
List<String> invalidatedApiKeyIds) {

‎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
@@ -27,7 +27,9 @@
2727

2828
import java.util.Arrays;
2929
import java.util.Collections;
30+
import java.util.HashMap;
3031
import java.util.LinkedHashSet;
32+
import java.util.List;
3133
import java.util.Map;
3234

3335
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -277,4 +279,55 @@ public void testParseIgnoresTransientMetadata() throws Exception {
277279
assertEquals(1, parsed.getTransientMetadata().size());
278280
assertEquals(true, parsed.getTransientMetadata().get("enabled"));
279281
}
282+
283+
public void testIsEmpty() {
284+
assertTrue(new RoleDescriptor(
285+
randomAlphaOfLengthBetween(1, 10), null, null, null, null, null, null, null)
286+
.isEmpty());
287+
288+
assertTrue(new RoleDescriptor(
289+
randomAlphaOfLengthBetween(1, 10),
290+
new String[0],
291+
new RoleDescriptor.IndicesPrivileges[0],
292+
new RoleDescriptor.ApplicationResourcePrivileges[0],
293+
new ConditionalClusterPrivileges.ManageApplicationPrivileges[0],
294+
new String[0],
295+
new HashMap<>(),
296+
new HashMap<>())
297+
.isEmpty());
298+
299+
final List<Boolean> booleans = Arrays.asList(
300+
randomBoolean(),
301+
randomBoolean(),
302+
randomBoolean(),
303+
randomBoolean(),
304+
randomBoolean(),
305+
randomBoolean());
306+
307+
final RoleDescriptor roleDescriptor = new RoleDescriptor(
308+
randomAlphaOfLengthBetween(1, 10),
309+
booleans.get(0) ? new String[0] : new String[] { "foo" },
310+
booleans.get(1) ?
311+
new RoleDescriptor.IndicesPrivileges[0] :
312+
new RoleDescriptor.IndicesPrivileges[] {
313+
RoleDescriptor.IndicesPrivileges.builder().indices("idx").privileges("foo").build() },
314+
booleans.get(2) ?
315+
new RoleDescriptor.ApplicationResourcePrivileges[0] :
316+
new RoleDescriptor.ApplicationResourcePrivileges[] {
317+
RoleDescriptor.ApplicationResourcePrivileges.builder()
318+
.application("app").privileges("foo").resources("res").build() },
319+
booleans.get(3) ?
320+
new ConditionalClusterPrivileges.ManageApplicationPrivileges[0] :
321+
new ConditionalClusterPrivileges.ManageApplicationPrivileges[] {
322+
new ConditionalClusterPrivileges.ManageApplicationPrivileges(Collections.singleton("foo")) },
323+
booleans.get(4) ? new String[0] : new String[] { "foo" },
324+
booleans.get(5) ? new HashMap<>() : Collections.singletonMap("foo", "bar"),
325+
Collections.singletonMap("foo", "bar"));
326+
327+
if (booleans.stream().anyMatch(e -> e.equals(false))) {
328+
assertFalse(roleDescriptor.isEmpty());
329+
} else {
330+
assertTrue(roleDescriptor.isEmpty());
331+
}
332+
}
280333
}

0 commit comments

Comments
 (0)