Skip to content

Commit

Permalink
Remove system-index write-access from superuser role (#82649)
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.

Backport of: #81400
  • Loading branch information
tvernum committed Jan 17, 2022
1 parent 9850819 commit 78382cb
Show file tree
Hide file tree
Showing 43 changed files with 447 additions and 166 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ testClusters.register("runTask") {
}
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 @@ -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 @@ -1588,32 +1588,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
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 @@ -235,7 +235,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
2 changes: 1 addition & 1 deletion x-pack/plugin/security/qa/security-trial/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ testClusters.matching { it.name == 'javaRestTest' }.configureEach {
setting 'xpack.security.authc.token.enabled', 'true'
setting 'xpack.security.authc.api_key.enabled', 'true'

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"
user username: "x_pack_rest_user", password: "x-pack-test-password"
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/security/qa/service-account/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ testClusters.matching { it.name == 'javaRestTest' }.configureEach {
testDistribution = 'DEFAULT'
numberOfNodes = 2

extraConfigFile 'roles.yml', file('src/javaRestTest/resources/roles.yml')
extraConfigFile 'node.key', file('src/javaRestTest/resources/ssl/node.key')
extraConfigFile 'node.crt', file('src/javaRestTest/resources/ssl/node.crt')
extraConfigFile 'ca.crt', file('src/javaRestTest/resources/ssl/ca.crt')
Expand Down Expand Up @@ -38,7 +37,8 @@ testClusters.matching { it.name == 'javaRestTest' }.configureEach {
keystore 'xpack.security.transport.ssl.secure_key_passphrase', 'node-password'
keystore 'xpack.security.http.ssl.secure_key_passphrase', 'node-password'

user username: "test_admin", password: 'x-pack-test-password', role: "superuser"
rolesFile file('src/javaRestTest/resources/roles.yml')
user username: "test_admin", password: 'x-pack-test-password'
user username: "elastic/fleet-server", password: 'x-pack-test-password', role: "superuser"
user username: "service_account_manager", password: 'x-pack-test-password', role: "service_account_manager"
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ testClusters.matching { it.name == 'javaRestTest' }.configureEach {
setting 'xpack.security.authc.realms.oidc.openid7.claims.principal', 'sub'
keystore 'xpack.security.authc.realms.oidc.openid7.rp.client_secret', 'this-is-my-secret'

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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ protected boolean addMockHttpTransport() {

@Override
protected String configRoles() {
return SecuritySettingsSource.CONFIG_ROLE_ALLOW_ALL + "\n" + "r1:\n" + " cluster: all\n";
return super.configRoles() + "\n" + "r1:\n" + " cluster: all\n";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ protected boolean addMockHttpTransport() {

@Override
protected String configRoles() {
// The definition of TEST_ROLE here is intentionally different than the definition in the superclass.
return """
%s:
cluster: [ all ]
Expand Down Expand Up @@ -113,7 +114,7 @@ protected String configRoles() {
indices:
- names: 'b'
privileges: [all]
""".formatted(SecuritySettingsSource.TEST_ROLE);
""".formatted(SecuritySettingsSource.TEST_ROLE) + '\n' + SecuritySettingsSourceField.ES_TEST_ROOT_ROLE_YML;
}

@Override
Expand Down Expand Up @@ -168,7 +169,7 @@ public void testSingleRole() throws Exception {

try {
client.prepareSearch("test", "test2").setQuery(matchAllQuery()).get();
fail("expected an authorization exception when one of mulitple indices is forbidden");
fail("expected an authorization exception when one of multiple indices is forbidden");
} catch (ElasticsearchSecurityException e) {
// expected
assertThat(e.status(), is(RestStatus.FORBIDDEN));
Expand Down

0 comments on commit 78382cb

Please sign in to comment.