You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by we...@apache.org on 2025/10/16 07:49:55 UTC
(cloudstack) 02/02: server,utils: improve js interpretation functionality
This is an automated email from the ASF dual-hosted git repository.
weizhou pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 03a4b9f4fd33d69717e2a21e5963e5f26612df90
Author: Abhishek Kumar <ab...@gmail.com>
AuthorDate: Wed Sep 17 01:20:38 2025 +0530
server,utils: improve js interpretation functionality
Make JS interpretation functionalities configurable via a hidden config
- js.interpretation.enabled
Default value is false, making such functionalities disabled, ie, new
heuristic rules cannot be added or updated.
For JsInterpretor, use --no-java --no-syntax-extensions args and a deny-all ClassFilter.
Replace string-spliced vars with ENGINE_SCOPE Bindings, use a fresh ScriptContext per run, and compile before eval.
Use a named daemon worker with hard timeouts and capture stdout.
Signed-off-by: Abhishek Kumar <ab...@gmail.com>
---
.../java/com/cloud/server/ManagementService.java | 13 +-
.../apache/cloudstack/quota/QuotaManagerImpl.java | 4 +-
.../cloudstack/quota/QuotaManagerImplTest.java | 4 +-
.../api/response/QuotaResponseBuilderImpl.java | 23 ++-
.../org/apache/cloudstack/quota/QuotaService.java | 12 +-
.../apache/cloudstack/quota/QuotaServiceImpl.java | 9 ++
.../com/cloud/resource/ResourceManagerImpl.java | 6 +
.../com/cloud/server/ManagementServerImpl.java | 22 ++-
.../java/com/cloud/storage/StorageManagerImpl.java | 9 ++
.../utils/jsinterpreter/JsInterpreter.java | 169 ++++++++++++++-------
.../utils/jsinterpreter/TagAsRuleHelper.java | 7 +-
.../utils/jsinterpreter/JsInterpreterTest.java | 81 ++++++----
12 files changed, 252 insertions(+), 107 deletions(-)
diff --git a/api/src/main/java/com/cloud/server/ManagementService.java b/api/src/main/java/com/cloud/server/ManagementService.java
index 18f3e901cd9..d6d7f7a59c6 100644
--- a/api/src/main/java/com/cloud/server/ManagementService.java
+++ b/api/src/main/java/com/cloud/server/ManagementService.java
@@ -20,7 +20,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import com.cloud.user.UserData;
import org.apache.cloudstack.api.command.admin.cluster.ListClustersCmd;
import org.apache.cloudstack.api.command.admin.config.ListCfgGroupsByCmd;
import org.apache.cloudstack.api.command.admin.config.ListCfgsByCmd;
@@ -66,6 +65,7 @@ import org.apache.cloudstack.api.command.user.vm.GetVMPasswordCmd;
import org.apache.cloudstack.api.command.user.vmgroup.UpdateVMGroupCmd;
import org.apache.cloudstack.config.Configuration;
import org.apache.cloudstack.config.ConfigurationGroup;
+import org.apache.cloudstack.framework.config.ConfigKey;
import com.cloud.alert.Alert;
import com.cloud.capacity.Capacity;
@@ -85,6 +85,7 @@ import com.cloud.storage.GuestOSHypervisor;
import com.cloud.storage.GuestOsCategory;
import com.cloud.storage.StoragePool;
import com.cloud.user.SSHKeyPair;
+import com.cloud.user.UserData;
import com.cloud.utils.Pair;
import com.cloud.utils.Ternary;
import com.cloud.vm.InstanceGroup;
@@ -98,6 +99,14 @@ import com.cloud.vm.VirtualMachine.Type;
public interface ManagementService {
static final String Name = "management-server";
+ ConfigKey<Boolean> JsInterpretationEnabled = new ConfigKey<>("Hidden"
+ , Boolean.class
+ , "js.interpretation.enabled"
+ , "false"
+ , "Enable/Disable all JavaScript interpretation related functionalities to create or update Javascript rules."
+ , false
+ , ConfigKey.Scope.Global);
+
/**
* returns the a map of the names/values in the configuration table
*
@@ -481,4 +490,6 @@ public interface ManagementService {
Pair<Boolean, String> patchSystemVM(PatchSystemVMCmd cmd);
+ void checkJsInterpretationAllowedIfNeededForParameterValue(String paramName, boolean paramValue);
+
}
diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java
index c9254814f46..99181f80c29 100644
--- a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java
+++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java
@@ -32,7 +32,6 @@ import java.util.stream.Collectors;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
-import com.cloud.user.Account;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.quota.activationrule.presetvariables.GenericPresetVariable;
import org.apache.cloudstack.quota.activationrule.presetvariables.PresetVariableHelper;
@@ -62,6 +61,7 @@ import org.springframework.stereotype.Component;
import com.cloud.usage.UsageVO;
import com.cloud.usage.dao.UsageDao;
+import com.cloud.user.Account;
import com.cloud.user.AccountVO;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.DateUtil;
@@ -467,7 +467,7 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager {
}
- jsInterpreter.injectStringVariable("resourceType", presetVariables.getResourceType());
+ jsInterpreter.injectVariable("resourceType", presetVariables.getResourceType());
jsInterpreter.injectVariable("value", presetVariables.getValue().toString());
jsInterpreter.injectVariable("zone", presetVariables.getZone().toString());
}
diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java
index 5dfc12f7ef8..3b2ea54e86d 100644
--- a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java
+++ b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java
@@ -270,7 +270,7 @@ public class QuotaManagerImplTest {
Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString());
Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString());
Mockito.verify(jsInterpreterMock, Mockito.never()).injectVariable(Mockito.eq("project"), Mockito.anyString());
- Mockito.verify(jsInterpreterMock).injectStringVariable(Mockito.eq("resourceType"), Mockito.anyString());
+ Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString());
Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString());
Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString());
}
@@ -291,7 +291,7 @@ public class QuotaManagerImplTest {
Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString());
Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString());
Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("project"), Mockito.anyString());
- Mockito.verify(jsInterpreterMock).injectStringVariable(Mockito.eq("resourceType"), Mockito.anyString());
+ Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString());
Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString());
Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString());
}
diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java
index 1c486759e43..00110bb0c0a 100644
--- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java
+++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java
@@ -39,7 +39,6 @@ import java.util.stream.Collectors;
import javax.inject.Inject;
-import com.cloud.utils.DateUtil;
import org.apache.cloudstack.api.ApiErrorCode;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.command.QuotaBalanceCmd;
@@ -70,8 +69,8 @@ import org.apache.cloudstack.quota.dao.QuotaCreditsDao;
import org.apache.cloudstack.quota.dao.QuotaEmailConfigurationDao;
import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
import org.apache.cloudstack.quota.dao.QuotaTariffDao;
-import org.apache.cloudstack.quota.vo.QuotaAccountVO;
import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
import org.apache.cloudstack.quota.vo.QuotaCreditsVO;
import org.apache.cloudstack.quota.vo.QuotaEmailConfigurationVO;
@@ -79,26 +78,28 @@ import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
import org.apache.cloudstack.quota.vo.QuotaTariffVO;
import org.apache.cloudstack.quota.vo.QuotaUsageVO;
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.reflect.FieldUtils;
-import org.apache.commons.lang3.ObjectUtils;
-import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
import org.springframework.stereotype.Component;
import com.cloud.domain.DomainVO;
import com.cloud.domain.dao.DomainDao;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.PermissionDeniedException;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.user.AccountVO;
import com.cloud.user.User;
import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.UserDao;
+import com.cloud.utils.DateUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.db.Filter;
-import com.cloud.event.ActionEvent;
-import com.cloud.event.EventTypes;
@Component
public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
@@ -139,6 +140,12 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
@Inject
private ApiDiscoveryService apiDiscoveryService;
+ protected void checkActivationRulesAllowed(String activationRule) {
+ if (!_quotaService.isJsInterpretationEnabled() && StringUtils.isNotEmpty(activationRule)) {
+ throw new PermissionDeniedException("Quota Tariff Activation Rule cannot be set, as Javascript interpretation is disabled in the configuration.");
+ }
+ }
+
@Override
public QuotaTariffResponse createQuotaTariffResponse(QuotaTariffVO tariff, boolean returnActivationRule) {
final QuotaTariffResponse response = new QuotaTariffResponse();
@@ -440,6 +447,8 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
throw new InvalidParameterValueException(String.format("There is no quota tariffs with name [%s].", name));
}
+ checkActivationRulesAllowed(activationRule);
+
Date currentQuotaTariffStartDate = currentQuotaTariff.getEffectiveOn();
currentQuotaTariff.setRemoved(now);
@@ -696,6 +705,8 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
throw new InvalidParameterValueException(String.format("A quota tariff with name [%s] already exist.", name));
}
+ checkActivationRulesAllowed(activationRule);
+
if (startDate.compareTo(now) < 0) {
throw new InvalidParameterValueException(String.format("The value passed as Quota tariff's start date is in the past: [%s]. " +
"Please, inform a date in the future or do not pass the parameter to use the current date and time.", startDate));
diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaService.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaService.java
index 8f3c34982c0..f6a34e01be8 100644
--- a/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaService.java
+++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaService.java
@@ -16,15 +16,15 @@
//under the License.
package org.apache.cloudstack.quota;
-import com.cloud.user.AccountVO;
-import com.cloud.utils.component.PluggableService;
+import java.math.BigDecimal;
+import java.util.Date;
+import java.util.List;
import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
import org.apache.cloudstack.quota.vo.QuotaUsageVO;
-import java.math.BigDecimal;
-import java.util.Date;
-import java.util.List;
+import com.cloud.user.AccountVO;
+import com.cloud.utils.component.PluggableService;
public interface QuotaService extends PluggableService {
@@ -40,4 +40,6 @@ public interface QuotaService extends PluggableService {
boolean saveQuotaAccount(AccountVO account, BigDecimal aggrUsage, Date endDate);
+ boolean isJsInterpretationEnabled();
+
}
diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaServiceImpl.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaServiceImpl.java
index 17fa7bd8425..73a8bc30486 100644
--- a/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaServiceImpl.java
+++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaServiceImpl.java
@@ -60,6 +60,7 @@ import com.cloud.configuration.Config;
import com.cloud.domain.dao.DomainDao;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.PermissionDeniedException;
+import com.cloud.server.ManagementService;
import com.cloud.user.Account;
import com.cloud.user.AccountVO;
import com.cloud.user.dao.AccountDao;
@@ -86,6 +87,8 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
private TimeZone _usageTimezone;
+ private boolean jsInterpretationEnabled = false;
+
public QuotaServiceImpl() {
super();
}
@@ -97,6 +100,8 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
String timeZoneStr = ObjectUtils.defaultIfNull(_configDao.getValue(Config.UsageAggregationTimezone.toString()), "GMT");
_usageTimezone = TimeZone.getTimeZone(timeZoneStr);
+ jsInterpretationEnabled = ManagementService.JsInterpretationEnabled.value();
+
return true;
}
@@ -284,4 +289,8 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
}
}
+ @Override
+ public boolean isJsInterpretationEnabled() {
+ return jsInterpretationEnabled;
+ }
}
diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
index 2ef7f4b6637..24f89548490 100755
--- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
+++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
@@ -154,6 +154,7 @@ import com.cloud.org.Cluster;
import com.cloud.org.Grouping;
import com.cloud.org.Managed;
import com.cloud.serializer.GsonHelper;
+import com.cloud.server.ManagementService;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.service.dao.ServiceOfferingDetailsDao;
@@ -271,6 +272,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
private ServiceOfferingDetailsDao _serviceOfferingDetailsDao;
@Inject
private UserVmManager userVmManager;
+ @Inject
+ ManagementService managementService;
private List<? extends Discoverer> _discoverers;
@@ -1936,6 +1939,9 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
@Override
public Host updateHost(final UpdateHostCmd cmd) throws NoTransitionException {
+ managementService.checkJsInterpretationAllowedIfNeededForParameterValue(ApiConstants.IS_TAG_A_RULE,
+ Boolean.TRUE.equals(cmd.getIsTagARule()));
+
return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(),
cmd.getAllocationState(), cmd.getUrl(), cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false);
}
diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
index 56e8a56f2e2..58ac6891e5f 100644
--- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
@@ -1040,6 +1040,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
protected List<DeploymentPlanner> _planners;
+ private boolean jsInterpretationEnabled = false;
+
private final List<HypervisorType> supportedHypervisors = new ArrayList<>();
public List<DeploymentPlanner> getPlanners() {
@@ -1126,6 +1128,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
supportedHypervisors.add(HypervisorType.KVM);
supportedHypervisors.add(HypervisorType.XenServer);
+ jsInterpretationEnabled = JsInterpretationEnabled.value();
+
return true;
}
@@ -4022,8 +4026,10 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
cmdList.add(ListGuestVlansCmd.class);
cmdList.add(AssignVolumeCmd.class);
cmdList.add(ListSecondaryStorageSelectorsCmd.class);
- cmdList.add(CreateSecondaryStorageSelectorCmd.class);
- cmdList.add(UpdateSecondaryStorageSelectorCmd.class);
+ if (jsInterpretationEnabled) {
+ cmdList.add(CreateSecondaryStorageSelectorCmd.class);
+ cmdList.add(UpdateSecondaryStorageSelectorCmd.class);
+ }
cmdList.add(RemoveSecondaryStorageSelectorCmd.class);
cmdList.add(ListAffectedVmsForStorageScopeChangeCmd.class);
@@ -4066,7 +4072,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
@Override
public ConfigKey<?>[] getConfigKeys() {
- return new ConfigKey<?>[] {vmPasswordLength, sshKeyLength, humanReadableSizes, customCsIdentifier};
+ return new ConfigKey<?>[] {vmPasswordLength, sshKeyLength, humanReadableSizes, customCsIdentifier,
+ JsInterpretationEnabled};
}
protected class EventPurgeTask extends ManagedContextRunnable {
@@ -5523,4 +5530,13 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
_lockControllerListener = lockControllerListener;
}
+ @Override
+ public void checkJsInterpretationAllowedIfNeededForParameterValue(String paramName, boolean paramValue) {
+ if (!paramValue || jsInterpretationEnabled) {
+ return;
+ }
+ throw new InvalidParameterValueException(String.format(
+ "The parameter %s cannot be set to true as JS interpretation is disabled",
+ paramName));
+ }
}
diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
index 323c0eb3ee4..5a66ad502f2 100644
--- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
@@ -213,6 +213,7 @@ import com.cloud.org.Grouping.AllocationState;
import com.cloud.resource.ResourceState;
import com.cloud.server.ConfigurationServer;
import com.cloud.server.ManagementServer;
+import com.cloud.server.ManagementService;
import com.cloud.server.StatsCollector;
import com.cloud.service.dao.ServiceOfferingDetailsDao;
import com.cloud.storage.Storage.ImageFormat;
@@ -398,6 +399,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
ConfigurationDao configurationDao;
@Inject
private ImageStoreDetailsUtil imageStoreDetailsUtil;
+ @Inject
+ ManagementService managementService;
protected List<StoragePoolDiscoverer> _discoverers;
@@ -1015,6 +1018,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone is currently disabled: %s", zone));
}
+ managementService.checkJsInterpretationAllowedIfNeededForParameterValue(ApiConstants.IS_TAG_A_RULE,
+ Boolean.TRUE.equals(cmd.isTagARule()));
+
Map<String, Object> params = new HashMap<>();
params.put("zoneId", zone.getId());
params.put("clusterId", clusterId);
@@ -1197,6 +1203,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
// Input validation
Long id = cmd.getId();
+ managementService.checkJsInterpretationAllowedIfNeededForParameterValue(ApiConstants.IS_TAG_A_RULE,
+ Boolean.TRUE.equals(cmd.isTagARule()));
+
StoragePoolVO pool = _storagePoolDao.findById(id);
if (pool == null) {
throw new IllegalArgumentException("Unable to find storage pool with ID: " + id);
diff --git a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java
index 1a3d9d843c3..3126da50bca 100644
--- a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java
+++ b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java
@@ -19,31 +19,52 @@ package org.apache.cloudstack.utils.jsinterpreter;
import java.io.Closeable;
import java.io.IOException;
+import java.io.StringWriter;
+import java.util.Arrays;
import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
import java.util.concurrent.Future;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
+import javax.script.Bindings;
+import javax.script.Compilable;
+import javax.script.CompiledScript;
+import javax.script.ScriptContext;
+import javax.script.ScriptEngine;
+import javax.script.ScriptException;
+import javax.script.SimpleBindings;
+import javax.script.SimpleScriptContext;
+
import org.apache.commons.collections.MapUtils;
-import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
-
-import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.logging.log4j.Logger;
+import org.openjdk.nashorn.api.scripting.ClassFilter;
import org.openjdk.nashorn.api.scripting.NashornScriptEngineFactory;
-import javax.script.ScriptEngine;
+import com.cloud.utils.exception.CloudRuntimeException;
/**
- * A class to execute JavaScript scripts, with the possibility to inject context to the scripts.
+ * Executes JavaScript with strong restrictions to mitigate RCE risks.
+ * - Disables Java interop via --no-java AND a deny-all ClassFilter
+ * - Disables Nashorn syntax extensions
+ * - Uses Bindings instead of string-splicing variables
+ * - Fresh ScriptContext per execution, with timeout on a daemon worker
*/
public class JsInterpreter implements Closeable {
protected Logger logger = LogManager.getLogger(JsInterpreter.class);
+ protected static final List<String> RESTRICTED_TOKENS = Arrays.asList( "engine", "context", "factory",
+ "Java", "java", "Packages"," javax", "load", "loadWithNewGlobal", "print", "factory", "getClass",
+ "runCommand", "Runtime", "exec", "ProcessBuilder", "Thread", "thread", "Threads", "Class", "class");
+
protected ScriptEngine interpreter;
protected String interpreterName;
private final String injectingLogMessage = "Injecting variable [%s] with value [%s] into the JS interpreter.";
@@ -51,21 +72,40 @@ public class JsInterpreter implements Closeable {
private TimeUnit defaultTimeUnit = TimeUnit.MILLISECONDS;
private long timeout;
private String timeoutDefaultMessage;
- protected Map<String, String> variables = new LinkedHashMap<>();
+
+ // Store variables as Objects; they go into Bindings (no code splicing)
+ protected Map<String, Object> variables = new LinkedHashMap<>();
+
+ /** Deny-all filter: no Java class is visible from scripts. */
+ static final class DenyAllClassFilter implements ClassFilter {
+ @Override public boolean exposeToScripts(String className) { return false; }
+ }
/**
* Constructor created exclusively for unit testing.
*/
- protected JsInterpreter() {
- }
+ protected JsInterpreter() { }
public JsInterpreter(long timeout) {
this.timeout = timeout;
- this.timeoutDefaultMessage = String.format("Timeout (in milliseconds) defined in the global setting [quota.activationrule.timeout]: [%s].", this.timeout);
+ this.timeoutDefaultMessage = String.format(
+ "Timeout (in milliseconds) defined in the global setting [quota.activationrule.timeout]: [%s].", this.timeout);
- executor = Executors.newSingleThreadExecutor();
- NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
+ if (System.getProperty("nashorn.args") == null) {
+ System.setProperty("nashorn.args", "--no-java --no-syntax-extensions");
+ }
+
+ this.executor = new ThreadPoolExecutor(
+ 1, 1, 60L, TimeUnit.SECONDS,
+ new LinkedBlockingQueue<>(),
+ r -> {
+ Thread t = new Thread(r, "JsInterpreter-worker");
+ t.setDaemon(true);
+ return t;
+ }
+ );
+ NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
this.interpreterName = factory.getEngineName();
logger.trace(String.format("Initiating JS interpreter: %s.", interpreterName));
@@ -73,49 +113,53 @@ public class JsInterpreter implements Closeable {
}
protected void setScriptEngineDisablingJavaLanguage(NashornScriptEngineFactory factory) {
- interpreter = factory.getScriptEngine("--no-java");
+ String[] opts = new String[] {
+ "--no-java",
+ "--no-syntax-extensions",
+ };
+ interpreter = factory.getScriptEngine(
+ opts,
+ JsInterpreter.class.getClassLoader(),
+ new DenyAllClassFilter()
+ );
}
- /**
- * Discards the current variables map and create a new one.
- */
+ /** Discards the current variables map and create a new one. */
public void discardCurrentVariables() {
logger.trace("Discarding current variables map and creating a new one.");
variables = new LinkedHashMap<>();
}
/**
- * Adds the parameters to a Map that will be converted to JS variables right before executing the script.
- * @param key The name of the variable.
- * @param value The value of the variable.
+ * Adds a variable that will be exposed via ENGINE_SCOPE bindings.
+ * Safe against code injection (no string concatenation).
*/
- public void injectVariable(String key, String value) {
- logger.trace(String.format(injectingLogMessage, key, value));
+ public void injectVariable(String key, Object value) {
+ if (key == null) return;
+ logger.trace(String.format(injectingLogMessage, key, String.valueOf(value)));
variables.put(key, value);
}
/**
- * Adds the parameter, surrounded by double quotes, to a Map that will be converted to a JS variable right before executing the script.
- * @param key The name of the variable.
- * @param value The value of the variable.
+ * @deprecated Not needed when using Bindings; kept for source compatibility.
+ * Prefer {@link #injectVariable(String, Object)}.
*/
+ @Deprecated
public void injectStringVariable(String key, String value) {
if (value == null) {
logger.trace(String.format("Not injecting [%s] because its value is null.", key));
return;
}
- value = String.format("\"%s\"", value);
- logger.trace(String.format(injectingLogMessage, key, value));
- variables.put(key, value);
+ injectVariable(key, value);
}
/**
- * Injects the variables to the script and execute it.
+ * Injects the variables via Bindings and executes the script with a fresh context.
* @param script Code to be executed.
* @return The result of the executed script.
*/
public Object executeScript(String script) {
- script = addVariablesToScript(script);
+ Objects.requireNonNull(script, "script");
logger.debug(String.format("Executing script [%s].", script));
@@ -126,43 +170,60 @@ public class JsInterpreter implements Closeable {
}
protected Object executeScriptInThread(String script) {
- Callable<Object> task = () -> interpreter.eval(script);
+ final Callable<Object> task = () -> {
+ final SimpleScriptContext ctx = new SimpleScriptContext();
- Future<Object> future = executor.submit(task);
+ final Bindings engineBindings = new SimpleBindings();
+ if (MapUtils.isNotEmpty(variables)) {
+ engineBindings.putAll(variables);
+ }
+ for (String token : RESTRICTED_TOKENS) {
+ engineBindings.put(token, null);
+ }
+ ctx.setBindings(engineBindings, ScriptContext.ENGINE_SCOPE);
+
+ final StringWriter out = new StringWriter();
+ ctx.setWriter(out);
+
+ try {
+ final CompiledScript compiled = ((Compilable) interpreter).compile(script);
+ Object result = compiled.eval(ctx);
+ if (out.getBuffer().length() > 0) {
+ logger.info("Script produced output on stdout: [{}]", out);
+ }
+ return result;
+ } catch (ScriptException se) {
+ String msg = se.getMessage() == null ? "Script error" : se.getMessage();
+ throw new ScriptException("Script error: " + msg, se.getFileName(), se.getLineNumber(), se.getColumnNumber());
+ }
+ };
+
+ final Future<Object> future = executor.submit(task);
try {
return future.get(this.timeout, defaultTimeUnit);
- } catch (TimeoutException | InterruptedException | ExecutionException e) {
- String message = String.format("Unable to execute script [%s] due to [%s]", script, e.getMessage());
-
- if (e instanceof TimeoutException) {
- message = String.format("Execution of script [%s] took too long and timed out. %s", script, timeoutDefaultMessage);
- }
-
+ } catch (TimeoutException e) {
+ String message = String.format(
+ "Execution of script [%s] took too long and timed out. %s", script, timeoutDefaultMessage);
logger.error(message, e);
throw new CloudRuntimeException(message, e);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ String message = String.format("Execution of script [%s] was interrupted.", script);
+ logger.error(message, e);
+ throw new CloudRuntimeException(message, e);
+ } catch (ExecutionException e) {
+ Throwable cause = e.getCause() == null ? e : e.getCause();
+ String message = String.format("Unable to execute script [%s] due to [%s]", script, cause.getMessage());
+ logger.error(message, cause);
+ throw new CloudRuntimeException(message, cause);
} finally {
future.cancel(true);
}
}
- protected String addVariablesToScript(String script) {
- if (MapUtils.isEmpty(variables)) {
- logger.trace(String.format("There is no variables to add to script [%s]. Returning.", script));
- return script;
- }
-
- String variablesToString = "";
- for (Map.Entry<String, String> variable : variables.entrySet()) {
- variablesToString = String.format("%s %s = %s;", variablesToString, variable.getKey(), variable.getValue());
- }
-
- return String.format("%s %s", variablesToString, script);
- }
-
-
@Override
public void close() throws IOException {
- executor.shutdown();
+ executor.shutdownNow();
}
}
diff --git a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java
index 8cf9c13fc19..da3da612a22 100644
--- a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java
+++ b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java
@@ -16,12 +16,12 @@
//under the License.
package org.apache.cloudstack.utils.jsinterpreter;
-import com.cloud.utils.exception.CloudRuntimeException;
-import org.apache.commons.lang3.StringEscapeUtils;
+import java.io.IOException;
+
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
-import java.io.IOException;
+import com.cloud.utils.exception.CloudRuntimeException;
public class TagAsRuleHelper {
@@ -32,7 +32,6 @@ public class TagAsRuleHelper {
public static boolean interpretTagAsRule(String rule, String tags, long timeout) {
String script = PARSE_TAGS + rule;
- tags = String.format("'%s'", StringEscapeUtils.escapeEcmaScript(tags));
try (JsInterpreter jsInterpreter = new JsInterpreter(timeout)) {
jsInterpreter.injectVariable("tags", tags);
Object scriptReturn = jsInterpreter.executeScript(script);
diff --git a/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java b/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java
index ea8a0247f9c..a78ee7b908e 100644
--- a/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java
+++ b/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java
@@ -20,6 +20,7 @@ package org.apache.cloudstack.utils.jsinterpreter;
import java.io.IOException;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.UUID;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
@@ -27,7 +28,8 @@ import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeoutException;
-import com.cloud.utils.exception.CloudRuntimeException;
+import javax.script.ScriptEngine;
+
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -36,9 +38,10 @@ import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
+import org.openjdk.nashorn.api.scripting.ClassFilter;
import org.openjdk.nashorn.api.scripting.NashornScriptEngineFactory;
-import javax.script.ScriptEngine;
+import com.cloud.utils.exception.CloudRuntimeException;
@RunWith(MockitoJUnitRunner.class)
public class JsInterpreterTest {
@@ -61,30 +64,6 @@ public class JsInterpreterTest {
Assert.assertTrue(executor.isShutdown());
}
- @Test
- public void addVariablesToScriptTestVariablesMapIsEmptyReturnScript() {
- String script = "a + b";
- jsInterpreterSpy.variables = new LinkedHashMap<>();
-
- String result = jsInterpreterSpy.addVariablesToScript(script);
-
- Assert.assertEquals(script, result);
- }
-
- @Test
- public void addVariablesToScriptTestVariablesMapIsNotEmptyInjectVariableToScript() {
- String script = "a + b";
- String var1 = "{test: \"test\"}";
- jsInterpreterSpy.injectVariable("var1", var1);
- jsInterpreterSpy.injectVariable("var2", var1);
-
- String expected = String.format(" var1 = %s; var2 = %s; %s", var1, var1, script);
-
- String result = jsInterpreterSpy.addVariablesToScript(script);
-
- Assert.assertEquals(expected, result);
- }
-
@Test
public void executeScriptTestReturnResultOfScriptExecution() {
String script = "5";
@@ -154,7 +133,7 @@ public class JsInterpreterTest {
@Test
public void discardCurrentVariablesTestInstantiateNewMap() {
- Map<String, String> variables = new LinkedHashMap<>();
+ Map<String, Object> variables = new LinkedHashMap<>();
variables.put("a", "b");
variables.put("b", null);
@@ -170,12 +149,14 @@ public class JsInterpreterTest {
NashornScriptEngineFactory nashornScriptEngineFactoryMock = Mockito.spy(NashornScriptEngineFactory.class);
ScriptEngine scriptEngineMock = Mockito.mock(ScriptEngine.class);
- Mockito.doReturn(scriptEngineMock).when(nashornScriptEngineFactoryMock).getScriptEngine(Mockito.anyString());
+ Mockito.doReturn(scriptEngineMock).when(nashornScriptEngineFactoryMock).getScriptEngine(Mockito.any(),
+ Mockito.any(ClassLoader.class), Mockito.any(ClassFilter.class));
jsInterpreterSpy.setScriptEngineDisablingJavaLanguage(nashornScriptEngineFactoryMock);
Assert.assertEquals(scriptEngineMock, jsInterpreterSpy.interpreter);
- Mockito.verify(nashornScriptEngineFactoryMock).getScriptEngine("--no-java");
+ Mockito.verify(nashornScriptEngineFactoryMock).getScriptEngine(Mockito.any(),
+ Mockito.any(ClassLoader.class), Mockito.any(ClassFilter.class));
}
@Test
@@ -193,6 +174,46 @@ public class JsInterpreterTest {
jsInterpreterSpy.injectStringVariable("a", "b");
- Assert.assertEquals(jsInterpreterSpy.variables.get("a"), "\"b\"");
+ Assert.assertEquals(jsInterpreterSpy.variables.get("a"), "b");
+ }
+
+ @Test
+ public void executeScriptTestValidScriptShouldPassWithMixedVariables() {
+ try (JsInterpreter jsInterpreter = new JsInterpreter(1000)) {
+ jsInterpreter.injectVariable("x", 10);
+ jsInterpreter.injectVariable("y", "hello");
+ jsInterpreter.injectVariable("z", true);
+ String validScript = "var result = x + (z ? 1 : 0); y + '-' + result;";
+ Object result = jsInterpreter.executeScript(validScript);
+ Assert.assertEquals("hello-11", result);
+ } catch (IOException exception) {
+ Assert.fail("IOException not expected here");
+ }
+ }
+
+ private void runMaliciousScriptFileTest(String script, String filename) {
+ try (JsInterpreter jsInterpreter = new JsInterpreter(1000)) {
+ jsInterpreter.executeScript(script);
+ } catch (CloudRuntimeException ex) {
+ Assert.assertTrue(ex.getMessage().contains("Unable to execute script"));
+ java.io.File f = new java.io.File(filename);
+ Assert.assertFalse(f.exists());
+ } catch (IOException exception) {
+ Assert.fail("IOException not expected here");
+ }
+ }
+
+ @Test
+ public void executeScriptTestMaliciousScriptShouldThrowException1() {
+ String filename = "/tmp/hack1-" + UUID.randomUUID();
+ String maliciousScript = "Java.type('java.lang.Runtime').getRuntime().exec('touch " + filename + "')";
+ runMaliciousScriptFileTest(maliciousScript, filename);
+ }
+
+ @Test
+ public void executeScriptTestMaliciousScriptShouldThrowException2() {
+ String filename = "/tmp/hack2-" + UUID.randomUUID();
+ String maliciousScript = "var e=this.engine.getFactory().getScriptEngine('-Dnashorn.args=--no-java=False'); e.eval(\"java.lang.Runtime.getRuntime().exec(['/bin/bash','-c','touch " + filename + "']);\");";
+ runMaliciousScriptFileTest(maliciousScript, filename);
}
}