Skip to content

Commit

Permalink
Role APIs allow internal role names (#86604)
Browse files Browse the repository at this point in the history
Internal users have internal role names associated with them (e.g.,
_system and _xpack). Currently, these role names are "reserved" in
that end-users can't create roles with clashing names. This PR removes
this restriction. The reasoning is similar to #86326:

For one, internal role names are not directly used during
authorization. Authorization for internal users follows a separate
path, leveraging the role descriptor that each internal user defines
directly. As such, internal roles and end-user defined roles do not
interfere with each other.

By removing the restriction, the PR decouples the implementation
details of internal users from our end-user APIs, which allows us to
evolve these details without introducing breaking changes.

Closes #86480
  • Loading branch information
n1v0lg committed May 12, 2022
1 parent dd0aad4 commit 11886d7
Show file tree
Hide file tree
Showing 18 changed files with 422 additions and 98 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/86604.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 86604
summary: Relax restrictions for role names in roles API
area: Authorization
type: enhancement
issues:
- 86480
Original file line number Diff line number Diff line change
Expand Up @@ -828,10 +828,7 @@ public static RoleDescriptor kibanaSystemRoleDescriptor(String name) {
}

public static boolean isReserved(String role) {
return RESERVED_ROLES.containsKey(role)
|| UsernamesField.SYSTEM_ROLE.equals(role)
|| UsernamesField.XPACK_ROLE.equals(role)
|| UsernamesField.ASYNC_SEARCH_ROLE.equals(role);
return RESERVED_ROLES.containsKey(role);
}

public Map<String, Object> usageStats() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.xpack.core.security.user;

import org.elasticsearch.common.Strings;
import org.elasticsearch.xpack.core.XPackPlugin;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.support.MetadataUtils;
Expand All @@ -14,9 +15,8 @@ public class AsyncSearchUser extends User {

public static final String NAME = UsernamesField.ASYNC_SEARCH_NAME;
public static final AsyncSearchUser INSTANCE = new AsyncSearchUser();
public static final String ROLE_NAME = UsernamesField.ASYNC_SEARCH_ROLE;
public static final RoleDescriptor ROLE_DESCRIPTOR = new RoleDescriptor(
ROLE_NAME,
UsernamesField.ASYNC_SEARCH_ROLE,
new String[] { "cancel_task" },
new RoleDescriptor.IndicesPrivileges[] {
RoleDescriptor.IndicesPrivileges.builder()
Expand All @@ -32,11 +32,11 @@ public class AsyncSearchUser extends User {
);

private AsyncSearchUser() {
super(NAME, ROLE_NAME);
super(NAME, Strings.EMPTY_ARRAY);
// the following traits, and especially the run-as one, go with all the internal users
// TODO abstract in a base `InternalUser` class
assert enabled();
assert roles() != null && roles().length == 1;
assert roles() != null && roles().length == 0;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.xpack.core.security.user;

import org.elasticsearch.common.Strings;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.support.MetadataUtils;

Expand All @@ -18,9 +19,8 @@ public class SecurityProfileUser extends User {

public static final String NAME = UsernamesField.SECURITY_PROFILE_NAME;
public static final SecurityProfileUser INSTANCE = new SecurityProfileUser();
private static final String ROLE_NAME = UsernamesField.SECURITY_PROFILE_ROLE;
public static final RoleDescriptor ROLE_DESCRIPTOR = new RoleDescriptor(
ROLE_NAME,
UsernamesField.SECURITY_PROFILE_ROLE,
null,
new RoleDescriptor.IndicesPrivileges[] {
RoleDescriptor.IndicesPrivileges.builder()
Expand All @@ -36,11 +36,11 @@ public class SecurityProfileUser extends User {
);

private SecurityProfileUser() {
super(NAME, ROLE_NAME);
super(NAME, Strings.EMPTY_ARRAY);
// the following traits, and especially the run-as one, go with all the internal users
// TODO abstract in a base `InternalUser` class
assert enabled();
assert roles() != null && roles().length == 1;
assert roles() != null && roles().length == 0;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.xpack.core.security.user;

import org.elasticsearch.common.Strings;
import org.elasticsearch.xpack.core.security.authz.privilege.SystemPrivilege;

import java.util.function.Predicate;
Expand All @@ -23,11 +24,11 @@ public class SystemUser extends User {
private static final Predicate<String> PREDICATE = SystemPrivilege.INSTANCE.predicate();

private SystemUser() {
super(NAME, ROLE_NAME);
super(NAME, Strings.EMPTY_ARRAY);
// the following traits, and especially the run-as one, go with all the internal users
// TODO abstract in a base `InternalUser` class
assert enabled();
assert roles() != null && roles().length == 1;
assert roles() != null && roles().length == 0;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.xpack.core.security.user;

import org.elasticsearch.common.Strings;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.support.MetadataUtils;

Expand All @@ -18,9 +19,8 @@ public class XPackSecurityUser extends User {

public static final String NAME = UsernamesField.XPACK_SECURITY_NAME;
public static final XPackSecurityUser INSTANCE = new XPackSecurityUser();
private static final String ROLE_NAME = UsernamesField.XPACK_SECURITY_ROLE;
public static final RoleDescriptor ROLE_DESCRIPTOR = new RoleDescriptor(
ROLE_NAME,
UsernamesField.XPACK_SECURITY_ROLE,
new String[] { "all" },
new RoleDescriptor.IndicesPrivileges[] {
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").allowRestrictedIndices(true).build() },
Expand All @@ -32,11 +32,11 @@ public class XPackSecurityUser extends User {
);

private XPackSecurityUser() {
super(NAME, ROLE_NAME);
super(NAME, Strings.EMPTY_ARRAY);
// the following traits, and especially the run-as one, go with all the internal users
// TODO abstract in a base `InternalUser` class
assert enabled();
assert roles() != null && roles().length == 1;
assert roles() != null && roles().length == 0;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.xpack.core.security.user;

import org.elasticsearch.common.Strings;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.index.IndexAuditTrailField;
import org.elasticsearch.xpack.core.security.support.MetadataUtils;
Expand All @@ -16,9 +17,8 @@
public class XPackUser extends User {

public static final String NAME = UsernamesField.XPACK_NAME;
public static final String ROLE_NAME = UsernamesField.XPACK_ROLE;
public static final RoleDescriptor ROLE_DESCRIPTOR = new RoleDescriptor(
ROLE_NAME,
UsernamesField.XPACK_ROLE,
new String[] { "all" },
new RoleDescriptor.IndicesPrivileges[] {
RoleDescriptor.IndicesPrivileges.builder()
Expand All @@ -33,11 +33,11 @@ public class XPackUser extends User {
public static final XPackUser INSTANCE = new XPackUser();

private XPackUser() {
super(NAME, ROLE_NAME);
super(NAME, Strings.EMPTY_ARRAY);
// the following traits, and especially the run-as one, go with all the internal users
// TODO abstract in a base `InternalUser` class
assert enabled();
assert roles() != null && roles().length == 1;
assert roles() != null && roles().length == 0;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.xpack.core.security.user.SecurityProfileUser;
import org.elasticsearch.xpack.core.security.user.SystemUser;
import org.elasticsearch.xpack.core.security.user.User;
import org.elasticsearch.xpack.core.security.user.UsernamesField;
import org.elasticsearch.xpack.core.security.user.XPackSecurityUser;
import org.elasticsearch.xpack.core.security.user.XPackUser;

Expand Down Expand Up @@ -180,6 +181,16 @@ public static List<String> randomInternalUsernames() {
return ESTestCase.randomNonEmptySubsetOf(INTERNAL_USERS.stream().map(User::principal).toList());
}

public static String randomInternalRoleName() {
return ESTestCase.randomFrom(
UsernamesField.SYSTEM_ROLE,
UsernamesField.XPACK_ROLE,
UsernamesField.ASYNC_SEARCH_ROLE,
UsernamesField.XPACK_SECURITY_ROLE,
UsernamesField.SECURITY_PROFILE_ROLE
);
}

public static class AuthenticationTestBuilder {
private Version version;
private Authentication authenticatingAuthentication;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,11 @@
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices;
import org.elasticsearch.xpack.core.security.user.APMSystemUser;
import org.elasticsearch.xpack.core.security.user.AsyncSearchUser;
import org.elasticsearch.xpack.core.security.user.BeatsSystemUser;
import org.elasticsearch.xpack.core.security.user.LogstashSystemUser;
import org.elasticsearch.xpack.core.security.user.RemoteMonitoringUser;
import org.elasticsearch.xpack.core.security.user.SystemUser;
import org.elasticsearch.xpack.core.security.user.XPackUser;
import org.elasticsearch.xpack.core.security.user.UsernamesField;
import org.elasticsearch.xpack.core.textstructure.action.FindStructureAction;
import org.elasticsearch.xpack.core.transform.action.DeleteTransformAction;
import org.elasticsearch.xpack.core.transform.action.GetTransformAction;
Expand Down Expand Up @@ -239,7 +238,11 @@ public void testIsReserved() {
assertThat(ReservedRolesStore.isReserved("kibana_system"), is(true));
assertThat(ReservedRolesStore.isReserved("superuser"), is(true));
assertThat(ReservedRolesStore.isReserved("foobar"), is(false));
assertThat(ReservedRolesStore.isReserved(SystemUser.ROLE_NAME), is(true));
assertThat(ReservedRolesStore.isReserved(SystemUser.ROLE_NAME), is(false));
assertThat(ReservedRolesStore.isReserved(UsernamesField.ASYNC_SEARCH_ROLE), is(false));
assertThat(ReservedRolesStore.isReserved(UsernamesField.SECURITY_PROFILE_ROLE), is(false));
assertThat(ReservedRolesStore.isReserved(UsernamesField.XPACK_ROLE), is(false));
assertThat(ReservedRolesStore.isReserved(UsernamesField.XPACK_SECURITY_ROLE), is(false));
assertThat(ReservedRolesStore.isReserved("transport_client"), is(true));
assertThat(ReservedRolesStore.isReserved("kibana_admin"), is(true));
assertThat(ReservedRolesStore.isReserved("kibana_user"), is(true));
Expand All @@ -255,8 +258,6 @@ public void testIsReserved() {
assertThat(ReservedRolesStore.isReserved("watcher_user"), is(true));
assertThat(ReservedRolesStore.isReserved("watcher_admin"), is(true));
assertThat(ReservedRolesStore.isReserved("beats_admin"), is(true));
assertThat(ReservedRolesStore.isReserved(XPackUser.ROLE_NAME), is(true));
assertThat(ReservedRolesStore.isReserved(AsyncSearchUser.ROLE_NAME), is(true));
assertThat(ReservedRolesStore.isReserved(LogstashSystemUser.ROLE_NAME), is(true));
assertThat(ReservedRolesStore.isReserved(BeatsSystemUser.ROLE_NAME), is(true));
assertThat(ReservedRolesStore.isReserved(APMSystemUser.ROLE_NAME), is(true));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/*
* 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.integration;

import joptsimple.internal.Strings;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.SecuritySettingsSourceField;
import org.elasticsearch.xpack.core.XPackPlugin;
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
import org.elasticsearch.xpack.core.security.user.AsyncSearchUser;
import org.elasticsearch.xpack.core.security.user.SecurityProfileUser;
import org.elasticsearch.xpack.core.security.user.SystemUser;
import org.elasticsearch.xpack.core.security.user.UsernamesField;
import org.elasticsearch.xpack.core.security.user.XPackSecurityUser;
import org.elasticsearch.xpack.core.security.user.XPackUser;
import org.junit.AfterClass;
import org.junit.BeforeClass;

import java.nio.file.Path;

public class InternalUserAndRoleIntegTests extends AbstractPrivilegeTestCase {
private static final String[] INTERNAL_USERNAMES = new String[] {
SystemUser.NAME,
XPackUser.NAME,
XPackSecurityUser.NAME,
AsyncSearchUser.NAME,
SecurityProfileUser.NAME };

private static final String[] INTERNAL_ROLE_NAMES = new String[] {
UsernamesField.SYSTEM_ROLE,
UsernamesField.XPACK_ROLE,
UsernamesField.XPACK_SECURITY_ROLE,
UsernamesField.ASYNC_SEARCH_ROLE,
UsernamesField.SECURITY_PROFILE_ROLE };
public static final String NON_INTERNAL_USERNAME = "user";
public static final String NON_INTERNAL_ROLE_NAME = "role";

@Override
protected String configRoles() {
StringBuilder builder = new StringBuilder(super.configRoles());
for (String roleName : INTERNAL_ROLE_NAMES) {
builder.append(defaultRole(roleName));
}
builder.append(defaultRole(NON_INTERNAL_ROLE_NAME));
return builder.toString();
}

private String defaultRole(String roleName) {
return """
%s:
cluster: [ none ]
indices:
- names: 'a'
privileges: [ all ]
""".formatted(roleName);
}

@Override
protected String configUsers() {
final Hasher passwdHasher = getFastStoredHashAlgoForTests();
final String usersPasswdHashed = new String(passwdHasher.hash(SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING));
StringBuilder builder = new StringBuilder(super.configUsers());
for (String username : INTERNAL_USERNAMES) {
builder.append("%s:%s\n".formatted(username, usersPasswdHashed));
}
return builder + "user:" + usersPasswdHashed + "\n";
}

@Override
protected String configUsersRoles() {
StringBuilder builder = new StringBuilder(super.configUsersRoles());
// non-internal username maps to all internal role names
for (String roleName : INTERNAL_ROLE_NAMES) {
builder.append("%s:%s\n".formatted(roleName, NON_INTERNAL_USERNAME));
}
// all internal usernames are mapped to custom role
return builder + "%s:%s\n".formatted(NON_INTERNAL_ROLE_NAME, Strings.join(INTERNAL_USERNAMES, ","));
}

private static Path repositoryLocation;

@BeforeClass
public static void setupRepositoryPath() {
repositoryLocation = createTempDir();
}

@AfterClass
public static void cleanupRepositoryPath() {
repositoryLocation = null;
}

@Override
protected boolean addMockHttpTransport() {
return false; // enable http
}

@Override
protected Settings nodeSettings() {
return Settings.builder().put(super.nodeSettings()).put("path.repo", repositoryLocation).build();
}

public void testInternalRoleNamesDoNotResultInInternalUserPermissions() throws Exception {
assertAccessIsDenied(NON_INTERNAL_USERNAME, "GET", "/_cluster/health");
assertAccessIsDenied(NON_INTERNAL_USERNAME, "PUT", "/" + XPackPlugin.ASYNC_RESULTS_INDEX + "/_doc/1", "{}");
assertAccessIsDenied(NON_INTERNAL_USERNAME, "PUT", "/.security-profile/_doc/1", "{}");
assertAccessIsAllowed(NON_INTERNAL_USERNAME, "PUT", "/a/_doc/1", "{}");
}

public void testInternalUsernamesDoNotResultInInternalUserPermissions() throws Exception {
for (final var internalUsername : INTERNAL_USERNAMES) {
assertAccessIsDenied(internalUsername, "GET", "/_cluster/health");
assertAccessIsDenied(internalUsername, "PUT", "/" + XPackPlugin.ASYNC_RESULTS_INDEX + "/_doc/1", "{}");
assertAccessIsDenied(internalUsername, "PUT", "/.security-profile/_doc/1", "{}");
assertAccessIsAllowed(internalUsername, "PUT", "/a/_doc/1", "{}");
}
}
}

0 comments on commit 11886d7

Please sign in to comment.