From d0464893f6ad30440cc5162277538663702d7a7d Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 24 Mar 2020 18:25:43 +1100 Subject: [PATCH] Validate role templates before saving role mapping (#52636) Role names are now compiled from role templates before role mapping is saved. This serves as validation for role templates to prevent malformed and invalid scripts to be persisted, which could later break authentication. Resolves: #48773 --- .../security/create-role-mappings.asciidoc | 54 ++++---- .../support/mapper/TemplateRoleName.java | 12 ++ .../support/mapper/TemplateRoleNameTests.java | 117 ++++++++++++++++++ .../mapper/NativeRoleMappingStore.java | 5 + .../mapper/NativeRoleMappingStoreTests.java | 15 +++ 5 files changed, 176 insertions(+), 27 deletions(-) diff --git a/x-pack/docs/en/rest-api/security/create-role-mappings.asciidoc b/x-pack/docs/en/rest-api/security/create-role-mappings.asciidoc index e80ff662c8305..51ccb39a69eca 100644 --- a/x-pack/docs/en/rest-api/security/create-role-mappings.asciidoc +++ b/x-pack/docs/en/rest-api/security/create-role-mappings.asciidoc @@ -23,7 +23,7 @@ Creates and updates role mappings. [[security-api-put-role-mapping-desc]] ==== {api-description-title} -Role mappings define which roles are assigned to each user. Each mapping has +Role mappings define which roles are assigned to each user. Each mapping has _rules_ that identify users and a list of _roles_ that are granted to those users. The role mapping APIs are generally the preferred way to manage role mappings @@ -37,6 +37,31 @@ roles API>> or <>. For more information, see <>. +[[_role_templates]] +===== Role templates + +The most common use for role mappings is to create a mapping from a known value +on the user to a fixed role name. For example, all users in the +`cn=admin,dc=example,dc=com` LDAP group should be given the `superuser` role in +{es}. The `roles` field is used for this purpose. + +For more complex needs, it is possible to use Mustache templates to dynamically +determine the names of the roles that should be granted to the user. The +`role_templates` field is used for this purpose. + +NOTE: To use role templates successfully, the relevant scripting feature must be +enabled. Otherwise, all attempts to create a role mapping with role templates +fail. See <>. + +All of the <> that are available in the +role mapping `rules` are also available in the role templates. Thus it is possible +to assign a user to a role that reflects their `username`, their `groups`, or the +name of the `realm` to which they authenticated. + +By default a template is evaluated to produce a single string that is the name +of the role which should be assigned to the user. If the `format` of the template +is set to `"json"` then the template is expected to produce a JSON string or an +array of JSON strings for the role names. [[security-api-put-role-mapping-path-params]] ==== {api-path-parms-title} @@ -77,31 +102,7 @@ _Exactly one of `roles` or `role_templates` must be specified_. `rules`:: (Required, object) The rules that determine which users should be matched by the mapping. A rule is a logical condition that is expressed by using a JSON DSL. -See <>. - -==== Role Templates - -The most common use for role mappings is to create a mapping from a known value -on the user to a fixed role name. -For example, all users in the `cn=admin,dc=example,dc=com` LDAP group should be -given the `superuser` role in {es}. -The `roles` field is used for this purpose. - -For more complex needs it is possible to use Mustache templates to dynamically -determine the names of the roles that should be granted to the user. -The `role_templates` field is used for this purpose. - -All of the <> that are available in the -role mapping `rules` are also available in the role templates. Thus it is possible -to assign a user to a role that reflects their `username`, their `groups` or the -name of the `realm` to which they authenticated. - -By default a template is evaluated to produce a single string that is the name -of the role which should be assigned to the user. If the `format` of the template -is set to `"json"` then the template is expected to produce a JSON string, or an -array of JSON strings for the role name(s). - -The Examples section below demonstrates the use of templated role names. +See <>. [[security-api-put-role-mapping-example]] ==== {api-examples-title} @@ -339,4 +340,3 @@ POST /_security/role_mapping/mapping9 <1> Because it is not possible to specify both `roles` and `role_templates` in the same role mapping, we can apply a "fixed name" role by using a template that has no substitutions. - diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/mapper/TemplateRoleName.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/mapper/TemplateRoleName.java index 59f9eafec1c00..e5a039b89a1ad 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/mapper/TemplateRoleName.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/mapper/TemplateRoleName.java @@ -97,6 +97,18 @@ public List getRoleNames(ScriptService scriptService, ExpressionModel mo } } + public void validate(ScriptService scriptService) { + try { + parseTemplate(scriptService, Collections.emptyMap()); + } catch (IllegalArgumentException e) { + throw e; + } catch (IOException e) { + throw new UncheckedIOException(e); + } catch (Exception e) { + throw new IllegalArgumentException(e); + } + } + private List convertJsonToList(String evaluation) throws IOException { final XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, evaluation); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/support/mapper/TemplateRoleNameTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/support/mapper/TemplateRoleNameTests.java index cab10ca728323..19433d132f1ed 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/support/mapper/TemplateRoleNameTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/support/mapper/TemplateRoleNameTests.java @@ -6,6 +6,9 @@ package org.elasticsearch.xpack.core.security.authc.support.mapper; +import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; @@ -17,8 +20,11 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.script.ScriptException; +import org.elasticsearch.script.ScriptMetaData; import org.elasticsearch.script.ScriptModule; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.StoredScriptSource; import org.elasticsearch.script.mustache.MustacheScriptEngine; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.EqualsHashCodeTestUtils; @@ -31,8 +37,11 @@ import java.util.Collections; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class TemplateRoleNameTests extends ESTestCase { @@ -116,4 +125,112 @@ public void tryEquals(TemplateRoleName original) { }; EqualsHashCodeTestUtils.checkEqualsAndHashCode(original, copy, mutate); } + + public void testValidate() { + final ScriptService scriptService = new ScriptService(Settings.EMPTY, + Collections.singletonMap(MustacheScriptEngine.NAME, new MustacheScriptEngine()), ScriptModule.CORE_CONTEXTS); + + final TemplateRoleName plainString = new TemplateRoleName(new BytesArray("{ \"source\":\"heroes\" }"), Format.STRING); + plainString.validate(scriptService); + + final TemplateRoleName user = new TemplateRoleName(new BytesArray("{ \"source\":\"_user_{{username}}\" }"), Format.STRING); + user.validate(scriptService); + + final TemplateRoleName groups = new TemplateRoleName(new BytesArray("{ \"source\":\"{{#tojson}}groups{{/tojson}}\" }"), + Format.JSON); + groups.validate(scriptService); + + final TemplateRoleName notObject = new TemplateRoleName(new BytesArray("heroes"), Format.STRING); + expectThrows(IllegalArgumentException.class, () -> notObject.validate(scriptService)); + + final TemplateRoleName invalidField = new TemplateRoleName(new BytesArray("{ \"foo\":\"heroes\" }"), Format.STRING); + expectThrows(IllegalArgumentException.class, () -> invalidField.validate(scriptService)); + } + + public void testValidateWillPassWithEmptyContext() { + final ScriptService scriptService = new ScriptService(Settings.EMPTY, + Collections.singletonMap(MustacheScriptEngine.NAME, new MustacheScriptEngine()), ScriptModule.CORE_CONTEXTS); + + final BytesReference template = new BytesArray("{ \"source\":\"" + + "{{username}}/{{dn}}/{{realm}}/{{metadata}}" + + "{{#realm}}" + + " {{name}}/{{type}}" + + "{{/realm}}" + + "{{#toJson}}groups{{/toJson}}" + + "{{^groups}}{{.}}{{/groups}}" + + "{{#metadata}}" + + " {{#first}}" + + "
  • {{name}}
    • " + + " {{/first}}" + + " {{#link}}" + + "
  • {{name}}
    • " + + " {{/link}}" + + " {{#toJson}}subgroups{{/toJson}}" + + " {{something-else}}" + + "{{/metadata}}\" }"); + final TemplateRoleName templateRoleName = new TemplateRoleName(template, Format.STRING); + templateRoleName.validate(scriptService); + } + + public void testValidateWillFailForSyntaxError() { + final ScriptService scriptService = new ScriptService(Settings.EMPTY, + Collections.singletonMap(MustacheScriptEngine.NAME, new MustacheScriptEngine()), ScriptModule.CORE_CONTEXTS); + + final BytesReference template = new BytesArray("{ \"source\":\" {{#not-closed}} {{other-variable}} \" }"); + + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new TemplateRoleName(template, Format.STRING).validate(scriptService)); + assertTrue(e.getCause() instanceof ScriptException); + } + + public void testValidationWillFailWhenInlineScriptIsNotEnabled() { + final Settings settings = Settings.builder().put("script.allowed_types", ScriptService.ALLOW_NONE).build(); + final ScriptService scriptService = new ScriptService(settings, + Collections.singletonMap(MustacheScriptEngine.NAME, new MustacheScriptEngine()), ScriptModule.CORE_CONTEXTS); + final BytesReference inlineScript = new BytesArray("{ \"source\":\"\" }"); + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new TemplateRoleName(inlineScript, Format.STRING).validate(scriptService)); + assertThat(e.getMessage(), containsString("[inline]")); + } + + public void testValidateWillFailWhenStoredScriptIsNotEnabled() { + final Settings settings = Settings.builder().put("script.allowed_types", ScriptService.ALLOW_NONE).build(); + final ScriptService scriptService = new ScriptService(settings, + Collections.singletonMap(MustacheScriptEngine.NAME, new MustacheScriptEngine()), ScriptModule.CORE_CONTEXTS); + final ClusterChangedEvent clusterChangedEvent = mock(ClusterChangedEvent.class); + final ClusterState clusterState = mock(ClusterState.class); + final MetaData metaData = mock(MetaData.class); + final StoredScriptSource storedScriptSource = mock(StoredScriptSource.class); + final ScriptMetaData scriptMetaData = new ScriptMetaData.Builder(null).storeScript("foo", storedScriptSource).build(); + when(clusterChangedEvent.state()).thenReturn(clusterState); + when(clusterState.metaData()).thenReturn(metaData); + when(metaData.custom(ScriptMetaData.TYPE)).thenReturn(scriptMetaData); + when(storedScriptSource.getLang()).thenReturn("mustache"); + when(storedScriptSource.getSource()).thenReturn(""); + when(storedScriptSource.getOptions()).thenReturn(Collections.emptyMap()); + scriptService.applyClusterState(clusterChangedEvent); + + final BytesReference storedScript = new BytesArray("{ \"id\":\"foo\" }"); + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new TemplateRoleName(storedScript, Format.STRING).validate(scriptService)); + assertThat(e.getMessage(), containsString("[stored]")); + } + + public void testValidateWillFailWhenStoredScriptIsNotFound() { + final ScriptService scriptService = new ScriptService(Settings.EMPTY, + Collections.singletonMap(MustacheScriptEngine.NAME, new MustacheScriptEngine()), ScriptModule.CORE_CONTEXTS); + final ClusterChangedEvent clusterChangedEvent = mock(ClusterChangedEvent.class); + final ClusterState clusterState = mock(ClusterState.class); + final MetaData metaData = mock(MetaData.class); + final ScriptMetaData scriptMetaData = new ScriptMetaData.Builder(null).build(); + when(clusterChangedEvent.state()).thenReturn(clusterState); + when(clusterState.metaData()).thenReturn(metaData); + when(metaData.custom(ScriptMetaData.TYPE)).thenReturn(scriptMetaData); + scriptService.applyClusterState(clusterChangedEvent); + + final BytesReference storedScript = new BytesArray("{ \"id\":\"foo\" }"); + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new TemplateRoleName(storedScript, Format.STRING).validate(scriptService)); + assertThat(e.getMessage(), containsString("unable to find script")); + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java index 2bd02e6decb1f..9586ddb47ab20 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java @@ -34,6 +34,7 @@ import org.elasticsearch.xpack.core.security.action.rolemapping.DeleteRoleMappingRequest; import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingRequest; import org.elasticsearch.xpack.core.security.authc.support.mapper.ExpressionRoleMapping; +import org.elasticsearch.xpack.core.security.authc.support.mapper.TemplateRoleName; import org.elasticsearch.xpack.core.security.authc.support.mapper.expressiondsl.ExpressionModel; import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames; import org.elasticsearch.xpack.core.security.authc.support.CachingRealm; @@ -165,6 +166,10 @@ protected ExpressionRoleMapping buildMapping(String id, BytesReference source) { * Stores (create or update) a single mapping in the index */ public void putRoleMapping(PutRoleMappingRequest request, ActionListener listener) { + // Validate all templates before storing the role mapping + for (TemplateRoleName templateRoleName : request.getRoleTemplates()) { + templateRoleName.validate(scriptService); + } modifyMapping(request.getName(), this::innerPutMapping, request, listener); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java index 9d96f1e115869..37490d0ddec11 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java @@ -24,6 +24,7 @@ import org.elasticsearch.xpack.core.security.action.realm.ClearRealmCacheAction; import org.elasticsearch.xpack.core.security.action.realm.ClearRealmCacheRequest; import org.elasticsearch.xpack.core.security.action.realm.ClearRealmCacheResponse; +import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingRequest; import org.elasticsearch.xpack.core.security.authc.AuthenticationResult; import org.elasticsearch.xpack.core.security.authc.RealmConfig; import org.elasticsearch.xpack.core.security.authc.RealmSettings; @@ -210,6 +211,20 @@ public void testCacheIsNotClearedIfNoRealmsAreAttached() { assertEquals(0, numInvalidation.get()); } + public void testPutRoleMappingWillValidateTemplateRoleNamesBeforeSave() { + final PutRoleMappingRequest putRoleMappingRequest = mock(PutRoleMappingRequest.class); + final TemplateRoleName templateRoleName = mock(TemplateRoleName.class); + final ScriptService scriptService = mock(ScriptService.class); + when(putRoleMappingRequest.getRoleTemplates()).thenReturn(Collections.singletonList(templateRoleName)); + doAnswer(invocationOnMock -> { + throw new IllegalArgumentException(); + }).when(templateRoleName).validate(scriptService); + + final NativeRoleMappingStore nativeRoleMappingStore = + new NativeRoleMappingStore(Settings.EMPTY, mock(Client.class), mock(SecurityIndexManager.class), scriptService); + expectThrows(IllegalArgumentException.class, () -> nativeRoleMappingStore.putRoleMapping(putRoleMappingRequest, null)); + } + private NativeRoleMappingStore buildRoleMappingStoreForInvalidationTesting(AtomicInteger invalidationCounter, boolean attachRealm) { final Settings settings = Settings.builder().put("path.home", createTempDir()).build();