Skip to content

Commit

Permalink
Remove system-index write-access from superuser role (#81400)
Browse files Browse the repository at this point in the history
This commit changes the superuser role (as used by the "elastic"
builtin user) so that it no longer has any sort of write access to
restricted indices (system indices).
This improves the safety and security of the cluster, as it means
that there are no out-of-the-box users or roles that can write to,
delete or close the security index.

Superusers can still read from (and monitor) system indices.

Other roles (and users) can still access system indices as specified
in their descriptor. These can be custom such as the
"_es_test_root" role used in the integration test suite, or builtin
roles such as kibana_system.
  • Loading branch information
tvernum committed Jan 17, 2022
1 parent 8f6f054 commit d61dda2
Show file tree
Hide file tree
Showing 48 changed files with 481 additions and 178 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ testClusters.register("runTask") {
systemProperty 'ingest.geoip.downloader.enabled.default', 'true'
setting 'xpack.security.enabled', 'true'
keystore 'bootstrap.password', 'password'
user username: 'elastic-admin', password: 'elastic-password', role: 'superuser'
user username: 'elastic-admin', password: 'elastic-password', role: '_es_test_root'
}
}

Expand Down
7 changes: 7 additions & 0 deletions build-tools-internal/src/main/resources/roles.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
_root:
cluster: [ "ALL" ]
indices:
- names: [ "*" ]
allow_restricted_indices: true
privileges: [ "ALL" ]
run_as: [ "*" ]
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,9 @@ private void configureSecurity() {
paramMap.entrySet().stream().flatMap(entry -> Stream.of(entry.getKey(), entry.getValue())).toArray(String[]::new)
)
);

// If we added users, then also add the standard test roles
rolesFile(getBuildPluginFile("/roles.yml"));
}
if (roleFiles.isEmpty() == false) {
logToProcessStdout("Setting up roles.yml");
Expand Down Expand Up @@ -751,10 +754,14 @@ public void user(Map<String, String> userSpec) {
Map<String, String> cred = new LinkedHashMap<>();
cred.put("useradd", userSpec.getOrDefault("username", "test_user"));
cred.put("-p", userSpec.getOrDefault("password", "x-pack-test-password"));
cred.put("-r", userSpec.getOrDefault("role", "superuser"));
cred.put("-r", userSpec.getOrDefault("role", "_es_test_root"));
credentials.add(cred);
}

private File getBuildPluginFile(String name) {
return project.getRootProject().file("build-tools/src/main/resources/" + name);
}

@Override
public void rolesFile(File rolesYml) {
roleFiles.add(rolesYml);
Expand Down
13 changes: 13 additions & 0 deletions build-tools/src/main/resources/roles.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

# The roles below are automatically defined by the "testcluster" setup
_es_test_root:
cluster: [ "ALL" ]
indices:
- names: [ "*" ]
allow_restricted_indices: true
privileges: [ "ALL" ]
run_as: [ "*" ]
applications:
- application: "*"
privileges: [ "*" ]
resources: [ "*" ]
2 changes: 1 addition & 1 deletion client/rest-high-level/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ testClusters.configureEach {
setting 'indices.lifecycle.poll_interval', '1000ms'
setting 'indices.lifecycle.history_index_enabled', 'false'
keystore 'xpack.security.transport.ssl.truststore.secure_password', 'testnode'
extraConfigFile 'roles.yml', file('roles.yml')
rolesFile file('roles.yml')
user username: clusterUserNameProvider.get(),
password: clusterPasswordProvider.get(),
role: providers.systemProperty('tests.rest.cluster.role').orElse('admin').forUseAtConfigurationTime().get()
Expand Down
2 changes: 1 addition & 1 deletion qa/system-indices/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ testClusters.configureEach {
testDistribution = 'DEFAULT'
setting 'xpack.security.enabled', 'true'
setting 'xpack.security.autoconfiguration.enabled', 'false'
user username: 'rest_user', password: 'rest-user-password', role: 'superuser'
user username: 'rest_user', password: 'rest-user-password'
}
2 changes: 1 addition & 1 deletion x-pack/docs/en/rest-api/security/authenticate.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@ The following example output provides information about the "rdeniro" user:
}
--------------------------------------------------
// TESTRESPONSE[s/"rdeniro"/"$body.username"/]
// TESTRESPONSE[s/"admin"/"superuser"/]
// TESTRESPONSE[s/"admin"/"_es_test_root"/]
3 changes: 3 additions & 0 deletions x-pack/docs/en/rest-api/security/get-tokens.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ seconds) that the token expires in, and the type:
}
--------------------------------------------------
// TESTRESPONSE[s/dGhpcyBpcyBub3QgYSByZWFsIHRva2VuIGJ1dCBpdCBpcyBvbmx5IHRlc3QgZGF0YS4gZG8gbm90IHRyeSB0byByZWFkIHRva2VuIQ==/$body.access_token/]
// TESTRESPONSE[s/superuser/_es_test_root/]

The token returned by this API can be used by sending a request with an
`Authorization` header with a value having the prefix "Bearer " followed
Expand Down Expand Up @@ -204,6 +205,7 @@ seconds) that the token expires in, the type, and the refresh token:
--------------------------------------------------
// TESTRESPONSE[s/dGhpcyBpcyBub3QgYSByZWFsIHRva2VuIGJ1dCBpdCBpcyBvbmx5IHRlc3QgZGF0YS4gZG8gbm90IHRyeSB0byByZWFkIHRva2VuIQ==/$body.access_token/]
// TESTRESPONSE[s/vLBPvmAB6KvwvJZr27cS/$body.refresh_token/]
// TESTRESPONSE[s/superuser/_es_test_root/]

[[security-api-refresh-token]]
To extend the life of an existing token obtained using the `password` grant type,
Expand Down Expand Up @@ -254,6 +256,7 @@ be used one time.
--------------------------------------------------
// TESTRESPONSE[s/dGhpcyBpcyBub3QgYSByZWFsIHRva2VuIGJ1dCBpdCBpcyBvbmx5IHRlc3QgZGF0YS4gZG8gbm90IHRyeSB0byByZWFkIHRva2VuIQ==/$body.access_token/]
// TESTRESPONSE[s/vLBPvmAB6KvwvJZr27cS/$body.refresh_token/]
// TESTRESPONSE[s/superuser/_es_test_root/]

The following example obtains a access token and refresh token using the `kerberos` grant type,
which simply creates a token in exchange for the base64 encoded kerberos ticket:
Expand Down
2 changes: 2 additions & 0 deletions x-pack/docs/en/rest-api/security/invalidate-tokens.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ The get token API returns the following information about the access token:
}
--------------------------------------------------
// TESTRESPONSE[s/dGhpcyBpcyBub3QgYSByZWFsIHRva2VuIGJ1dCBpdCBpcyBvbmx5IHRlc3QgZGF0YS4gZG8gbm90IHRyeSB0byByZWFkIHRva2VuIQ==/$body.access_token/]
// TESTRESPONSE[s/superuser/_es_test_root/]

This access token can now be immediately invalidated, as shown in the following
example:
Expand Down Expand Up @@ -165,6 +166,7 @@ The get token API returns the following information:
--------------------------------------------------
// TESTRESPONSE[s/dGhpcyBpcyBub3QgYSByZWFsIHRva2VuIGJ1dCBpdCBpcyBvbmx5IHRlc3QgZGF0YS4gZG8gbm90IHRyeSB0byByZWFkIHRva2VuIQ==/$body.access_token/]
// TESTRESPONSE[s/vLBPvmAB6KvwvJZr27cS/$body.refresh_token/]
// TESTRESPONSE[s/superuser/_es_test_root/]

The refresh token can now also be immediately invalidated as shown
in the following example:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,12 @@ public class ReservedRolesStore implements BiConsumer<Set<String>, ActionListene
"superuser",
new String[] { "all" },
new RoleDescriptor.IndicesPrivileges[] {
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").allowRestrictedIndices(true).build() },
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").allowRestrictedIndices(false).build(),
RoleDescriptor.IndicesPrivileges.builder()
.indices("*")
.privileges("monitor", "read", "view_index_metadata", "read_cross_cluster")
.allowRestrictedIndices(true)
.build() },
new RoleDescriptor.ApplicationResourcePrivileges[] {
RoleDescriptor.ApplicationResourcePrivileges.builder().application("*").privileges("*").resources("*").build() },
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.hash.MessageDigests;

import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -55,10 +54,13 @@ public String[] getRoleNames() {
public RoleKey id() {
if (roleNames.length == 0) {
return RoleKey.ROLE_KEY_EMPTY;
} else if (Arrays.asList(roleNames).contains(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName())) {
return RoleKey.ROLE_KEY_SUPERUSER;
} else {
return new RoleKey(Set.copyOf(new HashSet<>(List.of(roleNames))), RoleKey.ROLES_STORE_SOURCE);
final Set<String> distinctRoles = new HashSet<>(List.of(roleNames));
if (distinctRoles.size() == 1 && distinctRoles.contains(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName())) {
return RoleKey.ROLE_KEY_SUPERUSER;
} else {
return new RoleKey(Set.copyOf(distinctRoles), RoleKey.ROLES_STORE_SOURCE);
}
}
}

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

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

import java.util.Map;

/**
* internal user that manages xpack security. Has all cluster/indices permissions.
*/
Expand All @@ -14,6 +19,17 @@ 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,
new String[] { "all" },
new RoleDescriptor.IndicesPrivileges[] {
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").allowRestrictedIndices(true).build() },
null,
null,
new String[] { "*" },
MetadataUtils.DEFAULT_RESERVED_METADATA,
Map.of()
);

private XPackSecurityUser() {
super(NAME, ROLE_NAME);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,18 @@ public final class SecuritySettingsSourceField {
public static final String TEST_PASSWORD = "x-pack-test-password";
public static final String TEST_INVALID_PASSWORD = "invalid-test-password";

public static final String ES_TEST_ROOT_ROLE = "_es_test_root";
public static final String ES_TEST_ROOT_ROLE_YML = """
_es_test_root:
cluster: [ "ALL" ]
indices:
- names: [ "*" ]
allow_restricted_indices: true
privileges: [ "ALL" ]
run_as: [ "*" ]
# The _es_test_root role doesn't have any application privileges because that would require loading data (Application Privileges)
# from the security index, which can causes problems if the index is not available
""";

private SecuritySettingsSourceField() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1572,32 +1572,61 @@ public void testSuperuserRole() {
iac = superuserRole.indices().authorize(UpdateSettingsAction.NAME, Sets.newHashSet("aaaaaa", "ba"), lookup, fieldPermissionsCache);
assertThat(iac.getIndexPermissions("aaaaaa").isGranted(), is(true));
assertThat(iac.getIndexPermissions("b").isGranted(), is(true));

// Read security indices => allowed
iac = superuserRole.indices()
.authorize(
randomFrom(SearchAction.NAME, GetIndexAction.NAME),
Sets.newHashSet(RestrictedIndicesNames.SECURITY_MAIN_ALIAS),
lookup,
fieldPermissionsCache
);
assertThat("For " + iac, iac.getIndexPermissions(RestrictedIndicesNames.SECURITY_MAIN_ALIAS).isGranted(), is(true));
assertThat("For " + iac, iac.getIndexPermissions(internalSecurityIndex).isGranted(), is(true));

// Write security indices => denied
iac = superuserRole.indices()
.authorize(
randomFrom(IndexAction.NAME, DeleteIndexAction.NAME, SearchAction.NAME),
randomFrom(IndexAction.NAME, DeleteIndexAction.NAME),
Sets.newHashSet(RestrictedIndicesNames.SECURITY_MAIN_ALIAS),
lookup,
fieldPermissionsCache
);
assertThat(iac.getIndexPermissions(RestrictedIndicesNames.SECURITY_MAIN_ALIAS).isGranted(), is(true));
assertThat(iac.getIndexPermissions(internalSecurityIndex).isGranted(), is(true));
assertThat("For " + iac, iac.getIndexPermissions(RestrictedIndicesNames.SECURITY_MAIN_ALIAS).isGranted(), is(false));
assertThat("For " + iac, iac.getIndexPermissions(internalSecurityIndex).isGranted(), is(false));

assertTrue(superuserRole.indices().check(SearchAction.NAME));
assertFalse(superuserRole.indices().check("unknown"));

assertThat(superuserRole.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(true));

// Read security indices => allowed
assertThat(
superuserRole.indices()
.allowedIndicesMatcher(randomFrom(IndexAction.NAME, DeleteIndexAction.NAME, SearchAction.NAME))
.allowedIndicesMatcher(randomFrom(GetAction.NAME, IndicesStatsAction.NAME))
.test(mockIndexAbstraction(RestrictedIndicesNames.SECURITY_MAIN_ALIAS)),
is(true)
);
assertThat(
superuserRole.indices()
.allowedIndicesMatcher(randomFrom(IndexAction.NAME, DeleteIndexAction.NAME, SearchAction.NAME))
.allowedIndicesMatcher(randomFrom(GetAction.NAME, IndicesStatsAction.NAME))
.test(mockIndexAbstraction(internalSecurityIndex)),
is(true)
);

// Write security indices => denied
assertThat(
superuserRole.indices()
.allowedIndicesMatcher(randomFrom(IndexAction.NAME, DeleteIndexAction.NAME))
.test(mockIndexAbstraction(RestrictedIndicesNames.SECURITY_MAIN_ALIAS)),
is(false)
);
assertThat(
superuserRole.indices()
.allowedIndicesMatcher(randomFrom(IndexAction.NAME, DeleteIndexAction.NAME))
.test(mockIndexAbstraction(internalSecurityIndex)),
is(false)
);
}

public void testLogstashSystemRole() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,26 @@
public class RoleReferenceTests extends ESTestCase {

public void testNamedRoleReference() {
final String[] roleNames = randomArray(0, 2, String[]::new, () -> randomAlphaOfLength(8));
final String[] roleNames = randomArray(0, 2, String[]::new, () -> randomAlphaOfLengthBetween(4, 8));

final boolean hasSuperUserRole = roleNames.length > 0 && randomBoolean();
if (hasSuperUserRole) {
roleNames[randomIntBetween(0, roleNames.length - 1)] = "superuser";
final RoleReference.NamedRoleReference namedRoleReference = new RoleReference.NamedRoleReference(roleNames);

if (roleNames.length == 0) {
assertThat(namedRoleReference.id(), is(RoleKey.ROLE_KEY_EMPTY));
} else {
final RoleKey roleKey = namedRoleReference.id();
assertThat(roleKey.getNames(), equalTo(Set.of(roleNames)));
assertThat(roleKey.getSource(), equalTo(RoleKey.ROLES_STORE_SOURCE));
}
}

public void testSuperuserRoleReference() {
final String[] roleNames = randomArray(1, 3, String[]::new, () -> randomAlphaOfLengthBetween(4, 12));
roleNames[randomIntBetween(0, roleNames.length - 1)] = "superuser";
final RoleReference.NamedRoleReference namedRoleReference = new RoleReference.NamedRoleReference(roleNames);

if (hasSuperUserRole) {
if (roleNames.length == 1) {
assertThat(namedRoleReference.id(), is(RoleKey.ROLE_KEY_SUPERUSER));
} else if (roleNames.length == 0) {
assertThat(namedRoleReference.id(), is(RoleKey.ROLE_KEY_EMPTY));
} else {
final RoleKey roleKey = namedRoleReference.id();
assertThat(roleKey.getNames(), equalTo(Set.of(roleNames)));
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/fleet/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ testClusters.configureEach {
testDistribution = 'DEFAULT'
setting 'xpack.security.enabled', 'true'
setting 'xpack.security.autoconfiguration.enabled', 'false'
user username: 'x_pack_rest_user', password: 'x-pack-test-password', role: 'superuser'
user username: 'x_pack_rest_user', password: 'x-pack-test-password'
}
2 changes: 1 addition & 1 deletion x-pack/plugin/graph/qa/with-security/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ testClusters.configureEach {
setting 'xpack.security.enabled', 'true'
setting 'xpack.license.self_generated.type', 'trial'

extraConfigFile 'roles.yml', file('roles.yml')
rolesFile file('roles.yml')
user username: 'test_admin', password: 'x-pack-test-password'
user username: 'graph_explorer', password: 'x-pack-test-password', role: 'graph_explorer'
user username: 'no_graph_explorer', password: 'x-pack-test-password', role: 'no_graph_explorer'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ testClusters.configureEach {
setting 'xpack.security.authc.realms.saml.cloud-saml.attributes.name', 'https://idp.test.es.elasticsearch.org/attribute/name'
setting 'logger.org.elasticsearch.xpack.security.authc.saml', 'TRACE'
setting 'logger.org.elasticsearch.xpack.idp', 'TRACE'
extraConfigFile 'roles.yml', file('src/javaRestTest/resources/roles.yml')
rolesFile file('src/javaRestTest/resources/roles.yml')
extraConfigFile 'idp-sign.crt', file('src/javaRestTest/resources/idp-sign.crt')
extraConfigFile 'idp-sign.key', file('src/javaRestTest/resources/idp-sign.key')
extraConfigFile 'wildcard_services.json', file('src/javaRestTest/resources/wildcard_services.json')
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/ilm/qa/with-security/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ testClusters.configureEach {
setting 'xpack.watcher.enabled', 'false'
setting 'xpack.ml.enabled', 'false'
setting 'xpack.license.self_generated.type', 'trial'
extraConfigFile 'roles.yml', file('roles.yml')
rolesFile file('roles.yml')
user username: "test_ilm", password: "x-pack-test-password", role: "ilm"
}
2 changes: 1 addition & 1 deletion x-pack/plugin/logstash/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ dependencies {
testClusters.configureEach {
testDistribution = 'DEFAULT'
setting 'xpack.security.enabled', 'true'
user username: 'x_pack_rest_user', password: 'x-pack-test-password', role: 'superuser'
user username: 'x_pack_rest_user', password: 'x-pack-test-password'
}
2 changes: 1 addition & 1 deletion x-pack/plugin/ml/qa/ml-with-security/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ tasks.named("yamlRestTest").configure {

testClusters.configureEach {
testDistribution = 'DEFAULT'
extraConfigFile 'roles.yml', file('roles.yml')
rolesFile file('roles.yml')
user username: "x_pack_rest_user", password: "x-pack-test-password"
user username: "ml_admin", password: "x-pack-test-password", role: "minimal,machine_learning_admin,ingest_admin"
user username: "ml_user", password: "x-pack-test-password", role: "minimal,machine_learning_user"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ protected Settings externalClusterClientSettings() {
throw new UncheckedIOException(e);
}
writeFile(xpackConf, "users", "x_pack_rest_user" + ":" + SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING + "\n");
writeFile(xpackConf, "users_roles", "superuser:x_pack_rest_user\n");
writeFile(xpackConf, "users_roles", SecuritySettingsSourceField.ES_TEST_ROOT_ROLE + ":x_pack_rest_user\n");
writeFile(xpackConf, "roles.yml", SecuritySettingsSourceField.ES_TEST_ROOT_ROLE_YML);

Path key;
Path certificate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ tasks.register("javaRestTestWithSecurityEnabled", StandaloneRestIntegTestTask) {
extraConfigFile 'transport.key', file('src/javaRestTest/resources/ssl/transport.key')
extraConfigFile 'transport.crt', file('src/javaRestTest/resources/ssl/transport.crt')
extraConfigFile 'ca.crt', file('src/javaRestTest/resources/ssl/ca.crt')
extraConfigFile 'roles.yml', file('src/javaRestTest/resources/roles.yml')
rolesFile file('src/javaRestTest/resources/roles.yml')

user username: "admin_user", password: "admin-password"
user username: "security_test_user", password: "security-test-password", role: "security_test_role"
Expand Down
4 changes: 1 addition & 3 deletions x-pack/plugin/security/qa/profile/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@ testClusters.matching { it.name == 'javaRestTest' }.configureEach {
testDistribution = 'DEFAULT'
numberOfNodes = 2

extraConfigFile 'roles.yml', file('src/javaRestTest/resources/roles.yml')

setting 'xpack.ml.enabled', 'false'
setting 'xpack.license.self_generated.type', 'trial'

setting 'xpack.security.enabled', 'true'
setting 'xpack.security.authc.token.enabled', 'true'
setting 'xpack.security.authc.api_key.enabled', 'true'

user username: "test_admin", password: 'x-pack-test-password', role: "superuser"
user username: "test_admin", password: 'x-pack-test-password'
}
Empty file.

0 comments on commit d61dda2

Please sign in to comment.