From 8e1dc66f9f73e360238323177d2c7aef495a332c Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 23 Dec 2021 16:49:41 +1100 Subject: [PATCH] Add validation for API key role descriptors Put Role API prevents creation of invalidate role descriptors by validating that the given cluster privileges and index previleges can be resolved. However, the same validation is not performed when creating API keys. As a result, users are able to create invalidate API keys which then fail at use time. The experience is not user friendly and inconsistent. This PR fixes it by adding the same validation logic for API key creation. Resolves: #67311 --- .../security/action/CreateApiKeyRequest.java | 6 +- .../security/action/role/PutRoleRequest.java | 53 +------------ .../role/RoleDescriptorRequestValidator.java | 78 +++++++++++++++++++ .../action/CreateApiKeyRequestTests.java | 35 ++++++++- 4 files changed, 118 insertions(+), 54 deletions(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/RoleDescriptorRequestValidator.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequest.java index 716015e7eea34..dd36380984f4f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequest.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.xpack.core.security.action.role.RoleDescriptorRequestValidator; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.support.MetadataUtils; @@ -159,10 +160,13 @@ public ActionRequestValidationException validate() { } if (metadata != null && MetadataUtils.containsReservedMetadata(metadata)) { validationException = addValidationError( - "metadata keys may not start with [" + MetadataUtils.RESERVED_PREFIX + "]", + "API key metadata keys may not start with [" + MetadataUtils.RESERVED_PREFIX + "]", validationException ); } + for (RoleDescriptor roleDescriptor : roleDescriptors) { + validationException = RoleDescriptorRequestValidator.validate(roleDescriptor, validationException); + } return validationException; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequest.java index 5c5170cd41d6a..627b3d6f3e778 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequest.java @@ -15,12 +15,8 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; -import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; -import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges; -import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; -import org.elasticsearch.xpack.core.security.support.MetadataUtils; import java.io.IOException; import java.util.ArrayList; @@ -28,9 +24,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; - -import static org.elasticsearch.action.ValidateActions.addValidationError; /** * Request object for adding a role to the security index @@ -66,51 +59,7 @@ public PutRoleRequest() {} @Override public ActionRequestValidationException validate() { - ActionRequestValidationException validationException = null; - if (name == null) { - validationException = addValidationError("role name is missing", validationException); - } - if (clusterPrivileges != null) { - for (String cp : clusterPrivileges) { - try { - ClusterPrivilegeResolver.resolve(cp); - } catch (IllegalArgumentException ile) { - validationException = addValidationError(ile.getMessage(), validationException); - } - } - } - if (indicesPrivileges != null) { - for (RoleDescriptor.IndicesPrivileges idp : indicesPrivileges) { - try { - IndexPrivilege.get(Set.of(idp.getPrivileges())); - } catch (IllegalArgumentException ile) { - validationException = addValidationError(ile.getMessage(), validationException); - } - } - } - if (applicationPrivileges != null) { - for (RoleDescriptor.ApplicationResourcePrivileges privilege : applicationPrivileges) { - try { - ApplicationPrivilege.validateApplicationNameOrWildcard(privilege.getApplication()); - } catch (IllegalArgumentException e) { - validationException = addValidationError(e.getMessage(), validationException); - } - for (String privilegeName : privilege.getPrivileges()) { - try { - ApplicationPrivilege.validatePrivilegeOrActionName(privilegeName); - } catch (IllegalArgumentException e) { - validationException = addValidationError(e.getMessage(), validationException); - } - } - } - } - if (metadata != null && MetadataUtils.containsReservedMetadata(metadata)) { - validationException = addValidationError( - "metadata keys may not start with [" + MetadataUtils.RESERVED_PREFIX + "]", - validationException - ); - } - return validationException; + return RoleDescriptorRequestValidator.validate(roleDescriptor()); } public void name(String name) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/RoleDescriptorRequestValidator.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/RoleDescriptorRequestValidator.java new file mode 100644 index 0000000000000..26b50baff16bb --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/RoleDescriptorRequestValidator.java @@ -0,0 +1,78 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.core.security.action.role; + +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; +import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; +import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; +import org.elasticsearch.xpack.core.security.support.MetadataUtils; + +import java.util.Set; + +import static org.elasticsearch.action.ValidateActions.addValidationError; + +public class RoleDescriptorRequestValidator { + + private RoleDescriptorRequestValidator() {} + + public static ActionRequestValidationException validate(RoleDescriptor roleDescriptor) { + return validate(roleDescriptor, null); + } + + public static ActionRequestValidationException validate( + RoleDescriptor roleDescriptor, + ActionRequestValidationException validationException + ) { + if (roleDescriptor.getName() == null) { + validationException = addValidationError("role name is missing", validationException); + } + if (roleDescriptor.getClusterPrivileges() != null) { + for (String cp : roleDescriptor.getClusterPrivileges()) { + try { + ClusterPrivilegeResolver.resolve(cp); + } catch (IllegalArgumentException ile) { + validationException = addValidationError(ile.getMessage(), validationException); + } + } + } + if (roleDescriptor.getIndicesPrivileges() != null) { + for (RoleDescriptor.IndicesPrivileges idp : roleDescriptor.getIndicesPrivileges()) { + try { + IndexPrivilege.get(Set.of(idp.getPrivileges())); + } catch (IllegalArgumentException ile) { + validationException = addValidationError(ile.getMessage(), validationException); + } + } + } + if (roleDescriptor.getApplicationPrivileges() != null) { + for (RoleDescriptor.ApplicationResourcePrivileges privilege : roleDescriptor.getApplicationPrivileges()) { + try { + ApplicationPrivilege.validateApplicationNameOrWildcard(privilege.getApplication()); + } catch (IllegalArgumentException e) { + validationException = addValidationError(e.getMessage(), validationException); + } + for (String privilegeName : privilege.getPrivileges()) { + try { + ApplicationPrivilege.validatePrivilegeOrActionName(privilegeName); + } catch (IllegalArgumentException e) { + validationException = addValidationError(e.getMessage(), validationException); + } + } + } + } + if (roleDescriptor.getMetadata() != null && MetadataUtils.containsReservedMetadata(roleDescriptor.getMetadata())) { + validationException = addValidationError( + "role descriptor metadata keys may not start with [" + MetadataUtils.RESERVED_PREFIX + "]", + validationException + ); + } + return validationException; + } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequestTests.java index bba492e70553b..83bf95ee2fb19 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/CreateApiKeyRequestTests.java @@ -22,6 +22,7 @@ import java.util.Map; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.containsStringIgnoringCase; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -82,7 +83,39 @@ public void testMetadataKeyValidation() { final ActionRequestValidationException ve = request.validate(); assertNotNull(ve); assertThat(ve.validationErrors().size(), equalTo(1)); - assertThat(ve.validationErrors().get(0), containsString("metadata keys may not start with [_]")); + assertThat(ve.validationErrors().get(0), containsString("API key metadata keys may not start with [_]")); + } + + public void testRoleDescriptorValidation() { + final CreateApiKeyRequest request1 = new CreateApiKeyRequest( + randomAlphaOfLength(5), + List.of( + new RoleDescriptor( + randomAlphaOfLength(5), + new String[] { "manage_index_template" }, + new RoleDescriptor.IndicesPrivileges[] { + RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("rad").build() }, + new RoleDescriptor.ApplicationResourcePrivileges[] { + RoleDescriptor.ApplicationResourcePrivileges.builder() + .application(randomFrom("app*tab", "app 1")) + .privileges(randomFrom(" ", "\n")) + .resources("resource") + .build() }, + null, + null, + Map.of("_key", "value"), + null + ) + ), + null + ); + final ActionRequestValidationException ve1 = request1.validate(); + assertNotNull(ve1); + assertThat(ve1.validationErrors().get(0), containsString("unknown cluster privilege")); + assertThat(ve1.validationErrors().get(1), containsString("unknown index privilege")); + assertThat(ve1.validationErrors().get(2), containsStringIgnoringCase("application name")); + assertThat(ve1.validationErrors().get(3), containsStringIgnoringCase("Application privilege names")); + assertThat(ve1.validationErrors().get(4), containsStringIgnoringCase("role descriptor metadata keys may not start with ")); } public void testSerialization() throws IOException {