This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit db1d4516317ddbb7f21d1d293f0a465a46939332 Author: Alex Heneveld <[email protected]> AuthorDate: Fri Sep 15 09:46:11 2023 +0100 add permission for shell scripts. identical to groovy scripts for the moment. --- .../core/mgmt/entitlement/EntitlementManagerAdapter.java | 6 ++++++ .../brooklyn/core/mgmt/entitlement/Entitlements.java | 16 ++++++++++++---- .../brooklyn/core/server/BrooklynServerConfig.java | 3 +++ .../core/workflow/steps/external/ShellWorkflowStep.java | 11 +++++++++++ .../brooklyn/core/mgmt/entitlement/EntitlementsTest.java | 1 + .../apache/brooklyn/rest/resources/ScriptResource.java | 8 ++++++-- 6 files changed, 39 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementManagerAdapter.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementManagerAdapter.java index 5dd6e24f50..e41ffe8a51 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementManagerAdapter.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementManagerAdapter.java @@ -110,6 +110,11 @@ public abstract class EntitlementManagerAdapter implements EntitlementManager { return isEntitledToExecuteGroovyScripts(context); } + @Override + public Boolean handleExecuteScript() { + return isEntitledToExecuteScripts(context); + } + @Override public Boolean handleRoot() { return isEntitledToRoot(context); @@ -145,6 +150,7 @@ public abstract class EntitlementManagerAdapter implements EntitlementManager { protected abstract boolean isEntitledToSeeAllServerInfo(EntitlementContext context); protected abstract boolean isEntitledToSeeServerStatus(EntitlementContext context); protected abstract boolean isEntitledToExecuteGroovyScripts(EntitlementContext context); + protected abstract boolean isEntitledToExecuteScripts(EntitlementContext context); protected abstract boolean isEntitledToRoot(EntitlementContext context); } diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/Entitlements.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/Entitlements.java index 3eac82bf75..5aac7359d6 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/Entitlements.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/Entitlements.java @@ -131,10 +131,15 @@ public class Entitlements { public static EntitlementClass<Void> LOGBOOK_LOG_STORE_QUERY = new BasicEntitlementClassDefinition<Void>("logbook.query", Void.class); /** - * Permission to execute groovy scripts + * Permission to execute groovy scripts. Since 1.1 prefer {@link #EXECUTE_SCRIPT}. */ public static EntitlementClass<Void> EXECUTE_GROOVY_SCRIPT = new BasicEntitlementClassDefinition<Void>("groovy_script.execute", Void.class); + /** + * Permission to run local script commands, eg 'shell' workflow step, groovy scripts, etc + */ + public static EntitlementClass<Void> EXECUTE_SCRIPT = new BasicEntitlementClassDefinition<Void>("script.execute", Void.class); + @SuppressWarnings("unchecked") public enum EntitlementClassesEnum { ENTITLEMENT_SEE_CATALOG_ITEM(SEE_CATALOG_ITEM) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleSeeCatalogItem((String)argument); } }, @@ -154,6 +159,7 @@ public class Entitlements { ENTITLEMENT_SERVER_STATUS(SERVER_STATUS) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleSeeServerStatus(); } }, ENTITLEMENT_ROOT(ROOT) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleRoot(); } }, ENTITLEMENT_EXECUTE_GROOVY_SCRIPT(EXECUTE_GROOVY_SCRIPT) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleExecuteGroovyScript(); } }, + ENTITLEMENT_EXECUTE_SCRIPT(EXECUTE_SCRIPT) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleExecuteScript(); } }, /* NOTE, 'ROOT' USER ONLY IS ALLOWED TO SEE THE LOGS. */ ENTITLEMENT_LOGBOOK_QUERY(LOGBOOK_LOG_STORE_QUERY) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleRoot(); } }, @@ -191,6 +197,7 @@ public class Entitlements { public T handleAddJava(Object app); public T handleSeeAllServerInfo(); public T handleExecuteGroovyScript(); + public T handleExecuteScript(); public T handleRoot(); } @@ -267,13 +274,13 @@ public class Entitlements { } /** - * @return An entitlement manager allowing everything but {@link #EXECUTE_GROOVY_SCRIPT}. + * @return An entitlement manager allowing everything but {@link #EXECUTE_SCRIPT}, {@link #EXECUTE_GROOVY_SCRIPT}. */ public static EntitlementManager powerUser() { return new EntitlementManager() { @Override public <T> boolean isEntitled(EntitlementContext context, EntitlementClass<T> permission, T entitlementClassArgument) { - return !EXECUTE_GROOVY_SCRIPT.equals(permission); + return !EXECUTE_GROOVY_SCRIPT.equals(permission) && !EXECUTE_SCRIPT.equals(permission); } @Override public String toString() { @@ -284,7 +291,7 @@ public class Entitlements { /** * @return An entitlement manager allowing everything but {@link #ROOT}, {@link #LOGBOOK_LOG_STORE_QUERY}, {@link #SEE_ALL_SERVER_INFO}, - * {@link #EXECUTE_GROOVY_SCRIPT}, {@link #MODIFY_ENTITY}, and {@link #HA_ADMIN}. + * {@link #EXECUTE_GROOVY_SCRIPT}, {@link #EXECUTE_SCRIPT}, {@link #MODIFY_ENTITY}, and {@link #HA_ADMIN}. */ public static EntitlementManager user() { return new EntitlementManager() { @@ -295,6 +302,7 @@ public class Entitlements { !ROOT.equals(permission) && !LOGBOOK_LOG_STORE_QUERY.equals(permission) && !EXECUTE_GROOVY_SCRIPT.equals(permission) && + !EXECUTE_SCRIPT.equals(permission) && !MODIFY_ENTITY.equals(permission) && !HA_ADMIN.equals(permission); } diff --git a/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java b/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java index fc6979eb21..b1d7331455 100644 --- a/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java +++ b/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java @@ -178,4 +178,7 @@ public class BrooklynServerConfig { public static final ConfigKey<List<String>> SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "brooklyn.security.sensitive.fields.ext.blocked.phrases", "Extended blocking settings when plaintext values are blocked, allowing also to block DSL and complex values which contain any of the phrases supplied in this config key (comma-separated list)"); + public static final ConfigKey<Boolean> SHELL_WORKFLOW_STEP_DISABLED = ConfigKeys.newBooleanConfigKey("brooklyn.security.shell.workflow_step.disabled", + "Whether the ShellWorkflowStep is disabled for security; default false"); + } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/ShellWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/ShellWorkflowStep.java index 20a61d5516..961ae5c19a 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/ShellWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/ShellWorkflowStep.java @@ -27,6 +27,10 @@ import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.config.MapConfigKey; import org.apache.brooklyn.core.location.Locations; import org.apache.brooklyn.core.mgmt.BrooklynTags; +import org.apache.brooklyn.core.mgmt.entitlement.Entitlements; +import org.apache.brooklyn.core.mgmt.entitlement.Entitlements.EntityAndItem; +import org.apache.brooklyn.core.mgmt.entitlement.Entitlements.StringAndArgument; +import org.apache.brooklyn.core.mgmt.entitlement.WebEntitlementContext; import org.apache.brooklyn.core.workflow.WorkflowStepDefinition; import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext; import org.apache.brooklyn.core.workflow.steps.variables.SetVariableWorkflowStep; @@ -44,6 +48,8 @@ import org.apache.brooklyn.util.core.task.system.internal.SystemProcessTaskFacto import org.apache.brooklyn.util.core.text.TemplateProcessor; import org.apache.brooklyn.util.text.Strings; +import static org.apache.brooklyn.core.server.BrooklynServerConfig.SHELL_WORKFLOW_STEP_DISABLED; + public class ShellWorkflowStep extends WorkflowStepDefinition { public static final String SHORTHAND = "${command...}"; @@ -64,6 +70,11 @@ public class ShellWorkflowStep extends WorkflowStepDefinition { @Override protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) { + if (Boolean.TRUE.equals(context.getManagementContext().getConfig().getConfig(SHELL_WORKFLOW_STEP_DISABLED))) { + throw new IllegalStateException("The 'shell' workflow step is disabled in this intance of Cloudsoft AMP."); + } + Entitlements.checkEntitled(context.getManagementContext().getEntitlementManager(), Entitlements.EXECUTE_SCRIPT, null); + String command = new SetVariableWorkflowStep.ConfigurableInterpolationEvaluation<>(context, TypeToken.of(String.class), getInput().get(COMMAND.getName()), context.getInputOrDefault(INTERPOLATION_MODE), context.getInputOrDefault(INTERPOLATION_ERRORS)).evaluate(); diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementsTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementsTest.java index 34690bbc5e..5f7802d097 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementsTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementsTest.java @@ -92,6 +92,7 @@ public class EntitlementsTest extends BrooklynAppUnitTestSupport { assertFalse(allowSeeEntity.isEntitled(null, Entitlements.DEPLOY_APPLICATION, null)); assertTrue(allowSeeEntity.isEntitled(null, Entitlements.SEE_ALL_SERVER_INFO, null)); assertFalse(allowSeeEntity.isEntitled(null, Entitlements.EXECUTE_GROOVY_SCRIPT, null)); + assertFalse(allowSeeEntity.isEntitled(null, Entitlements.EXECUTE_SCRIPT, null)); } // nonSecret diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ScriptResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ScriptResource.java index fa1aa093ef..83356a584a 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ScriptResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ScriptResource.java @@ -46,8 +46,12 @@ public class ScriptResource extends AbstractBrooklynRestResource implements Scri @SuppressWarnings("rawtypes") @Override public ScriptExecutionSummary groovy(HttpServletRequest request, String script) { - if (!Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.EXECUTE_GROOVY_SCRIPT, null)) { - throw WebResourceUtils.forbidden("User '%s' is not authorized to perform this operation", Entitlements.getEntitlementContext().user()); + boolean entitledGS = Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.EXECUTE_GROOVY_SCRIPT, null); + boolean entitledS = Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.EXECUTE_SCRIPT, null); + + if (!entitledS || !entitledGS) { + if (entitledGS) log.warn("User '"+Entitlements.getEntitlementContext().user()+"' is entitled to run groovy scripts but not scripts. The two permissions should be equivalent and the former may be removed, blocking access. Either grant permission to execute scripts or remove permission to execute groovy scripts."); + else throw WebResourceUtils.forbidden("User '%s' is not authorized to perform this operation", Entitlements.getEntitlementContext().user()); } log.info("Web REST executing user-supplied script");
