From 309b444205a0323f9608d85105f8b30c66613884 Mon Sep 17 00:00:00 2001 From: dahn Date: Thu, 9 Oct 2025 08:39:45 +0200 Subject: [PATCH 1/9] pom.xml: update jetty version (#11793) * update jetty * Rollback jetty-maven-plugin version in pom.xml Co-authored-by: Rohit Yadav --------- Co-authored-by: Daan Hoogland Co-authored-by: Wei Zhou Co-authored-by: Rohit Yadav --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 17385ab5a8c..b620a648e36 100644 --- a/pom.xml +++ b/pom.xml @@ -158,7 +158,7 @@ 2.3.3 2.3.7 2.26 - 9.4.51.v20230217 + 9.4.58.v20250814 9.4.27.v20200227 5.5.0 2.12.5 From 4d95f08a3abc181ae050018f5dd7741ab34f8ca1 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Thu, 9 Oct 2025 13:11:18 +0530 Subject: [PATCH 2/9] Delete template from storage pool instantly if no volume is using it (#11782) --- .../com/cloud/template/TemplateManager.java | 9 +++ .../cloud/storage/dao/VMTemplatePoolDao.java | 2 + .../storage/dao/VMTemplatePoolDaoImpl.java | 10 ++++ .../datastore/db/PrimaryDataStoreDao.java | 2 + .../datastore/db/PrimaryDataStoreDaoImpl.java | 14 +++++ .../template/HypervisorTemplateAdapter.java | 11 +++- .../cloud/template/TemplateManagerImpl.java | 55 +++++++++++++------ 7 files changed, 85 insertions(+), 18 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java index b8912526fdf..a0f91882c4d 100644 --- a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java +++ b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java @@ -56,6 +56,13 @@ public interface TemplateManager { + "will validate if the provided URL is resolvable during the register of templates/ISOs before persisting them in the database.", true); + ConfigKey TemplateDeleteFromPrimaryStorage = new ConfigKey("Advanced", + Boolean.class, + "template.delete.from.primary.storage", "true", + "Template when deleted will be instantly deleted from the Primary Storage", + true, + ConfigKey.Scope.Global); + static final String VMWARE_TOOLS_ISO = "vmware-tools.iso"; static final String XS_TOOLS_ISO = "xs-tools.iso"; @@ -103,6 +110,8 @@ public interface TemplateManager { */ List getUnusedTemplatesInPool(StoragePoolVO pool); + void evictTemplateFromStoragePoolsForZones(Long templateId, List zoneId); + /** * Deletes a template in the specified storage pool. * diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDao.java index a3ce03a74c3..a208590e23a 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDao.java @@ -35,6 +35,8 @@ public interface VMTemplatePoolDao extends GenericDao listByPoolIdAndState(long poolId, ObjectInDataStoreStateMachine.State state); + List listByPoolIdsAndTemplate(List poolIds, Long templateId); + List listByTemplateStatus(long templateId, VMTemplateStoragePoolVO.Status downloadState); List listByTemplateStatus(long templateId, VMTemplateStoragePoolVO.Status downloadState, long poolId); diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDaoImpl.java index 5a2ec1163fb..5dfc138d8e1 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDaoImpl.java @@ -150,6 +150,16 @@ public class VMTemplatePoolDaoImpl extends GenericDaoBase listByPoolIdsAndTemplate(List poolIds, Long templateId) { + SearchCriteria sc = PoolTemplateSearch.create(); + if (CollectionUtils.isNotEmpty(poolIds)) { + sc.setParameters("pool_id", poolIds.toArray()); + } + sc.setParameters("template_id", templateId); + return listBy(sc); + } + @Override public List listByTemplateStatus(long templateId, VMTemplateStoragePoolVO.Status downloadState) { SearchCriteria sc = TemplateStatusSearch.create(); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java index f0c235e842c..102df6848f8 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java @@ -154,4 +154,6 @@ public interface PrimaryDataStoreDao extends GenericDao { String keyword, Filter searchFilter); List listByIds(List ids); + + List listByDataCenterIds(List dataCenterIds); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java index cb7313954dc..0d901c82306 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java @@ -63,6 +63,7 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase private final GenericSearchBuilder StatusCountSearch; private final SearchBuilder ClustersSearch; private final SearchBuilder IdsSearch; + private final SearchBuilder DcsSearch; @Inject private StoragePoolDetailsDao _detailsDao; @@ -155,6 +156,9 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase IdsSearch.and("ids", IdsSearch.entity().getId(), SearchCriteria.Op.IN); IdsSearch.done(); + DcsSearch = createSearchBuilder(); + DcsSearch.and("dataCenterId", DcsSearch.entity().getDataCenterId(), SearchCriteria.Op.IN); + DcsSearch.done(); } @Override @@ -733,6 +737,16 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase return listBy(sc); } + @Override + public List listByDataCenterIds(List dataCenterIds) { + if (CollectionUtils.isEmpty(dataCenterIds)) { + return Collections.emptyList(); + } + SearchCriteria sc = DcsSearch.create(); + sc.setParameters("dataCenterId", dataCenterIds.toArray()); + return listBy(sc); + } + private SearchCriteria createStoragePoolSearchCriteria(Long storagePoolId, String storagePoolName, Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, String keyword) { diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index f7eb654141d..8d38aba0e7e 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -627,7 +627,7 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { boolean dataDiskDeletetionResult = true; List dataDiskTemplates = templateDao.listByParentTemplatetId(template.getId()); - if (dataDiskTemplates != null && dataDiskTemplates.size() > 0) { + if (CollectionUtils.isNotEmpty(dataDiskTemplates)) { logger.info("Template: {} has Datadisk template(s) associated with it. Delete Datadisk templates before deleting the template", template); for (VMTemplateVO dataDiskTemplate : dataDiskTemplates) { logger.info("Delete Datadisk template: {} from image store: {}", dataDiskTemplate, imageStore); @@ -693,6 +693,9 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { if (success) { if ((imageStores != null && imageStores.size() > 1) && (profile.getZoneIdList() != null)) { //if template is stored in more than one image stores, and the zone id is not null, then don't delete other templates. + if (templateMgr.TemplateDeleteFromPrimaryStorage.value()) { + templateMgr.evictTemplateFromStoragePoolsForZones(template.getId(), profile.getZoneIdList()); + } return cleanupTemplate(template, success); } @@ -705,7 +708,7 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { // find all eligible image stores for this template List iStores = templateMgr.getImageStoreByTemplate(template.getId(), null); - if (iStores == null || iStores.size() == 0) { + if (CollectionUtils.isEmpty(iStores)) { // remove any references from template_zone_ref List templateZones = templateZoneDao.listByTemplateId(template.getId()); if (templateZones != null) { @@ -726,6 +729,10 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { } + if (templateMgr.TemplateDeleteFromPrimaryStorage.value()) { + templateMgr.evictTemplateFromStoragePoolsForZones(template.getId(), profile.getZoneIdList()); + } + // remove its related ACL permission Pair, Long> templateClassForId = new Pair<>(VirtualMachineTemplate.class, template.getId()); _messageBus.publish(_name, EntityManager.MESSAGE_REMOVE_ENTITY_EVENT, PublishScope.LOCAL, templateClassForId); diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index cf88ccec919..5aa31eff170 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1024,33 +1024,53 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, return adapter.delete(new TemplateProfile(userId, template, zoneId)); } + private Boolean templateIsUnusedInPool(VMTemplateStoragePoolVO templatePoolVO) { + VMTemplateVO template = _tmpltDao.findByIdIncludingRemoved(templatePoolVO.getTemplateId()); + + // If this is a routing template, consider it in use + if (template.getTemplateType() == TemplateType.SYSTEM) { + return false; + } + + // If the template is not yet downloaded to the pool, consider it in use + if (templatePoolVO.getDownloadState() != Status.DOWNLOADED) { + return false; + } + + if (template.getFormat() == ImageFormat.ISO || _volumeDao.isAnyVolumeActivelyUsingTemplateOnPool(template.getId(), templatePoolVO.getPoolId())) { + return false; + } + return true; + } + @Override public List getUnusedTemplatesInPool(StoragePoolVO pool) { List unusedTemplatesInPool = new ArrayList(); List allTemplatesInPool = _tmpltPoolDao.listByPoolId(pool.getId()); for (VMTemplateStoragePoolVO templatePoolVO : allTemplatesInPool) { - VMTemplateVO template = _tmpltDao.findByIdIncludingRemoved(templatePoolVO.getTemplateId()); - - // If this is a routing template, consider it in use - if (template.getTemplateType() == TemplateType.SYSTEM) { - continue; - } - - // If the template is not yet downloaded to the pool, consider it in - // use - if (templatePoolVO.getDownloadState() != Status.DOWNLOADED) { - continue; - } - - if (template.getFormat() != ImageFormat.ISO && !_volumeDao.isAnyVolumeActivelyUsingTemplateOnPool(template.getId(), pool.getId())) { + if (templateIsUnusedInPool(templatePoolVO)) { unusedTemplatesInPool.add(templatePoolVO); } } - return unusedTemplatesInPool; } + @Override + public void evictTemplateFromStoragePoolsForZones(Long templateId, List zoneIds) { + List poolIds = new ArrayList<>(); + if (CollectionUtils.isNotEmpty(zoneIds)) { + List pools = _poolDao.listByDataCenterIds(zoneIds); + poolIds = pools.stream().map(StoragePoolVO::getId).collect(Collectors.toList()); + } + List templateStoragePoolVOS = _tmpltPoolDao.listByPoolIdsAndTemplate(poolIds, templateId); + for (VMTemplateStoragePoolVO templateStoragePoolVO: templateStoragePoolVOS) { + if (templateIsUnusedInPool(templateStoragePoolVO)) { + evictTemplateFromStoragePool(templateStoragePoolVO); + } + } + } + @Override @DB public void evictTemplateFromStoragePool(VMTemplateStoragePoolVO templatePoolVO) { @@ -2368,7 +2388,10 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {AllowPublicUserTemplates, TemplatePreloaderPoolSize, ValidateUrlIsResolvableBeforeRegisteringTemplate}; + return new ConfigKey[] {AllowPublicUserTemplates, + TemplatePreloaderPoolSize, + ValidateUrlIsResolvableBeforeRegisteringTemplate, + TemplateDeleteFromPrimaryStorage}; } public List getTemplateAdapters() { From 86cad79c156298e66dcabf668a05dc79fd46a129 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 13 Oct 2025 08:16:46 +0200 Subject: [PATCH 3/9] importvm: fix IP address allocation on Shared networks (#11811) --- .../orchestration/NetworkOrchestrator.java | 20 +++++++++++++++---- .../NetworkOrchestratorTest.java | 2 +- .../vm/UnmanagedVMsManagerImpl.java | 9 ++------- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index a9c15afc2cf..4c7641fb79b 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -4772,6 +4772,18 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } }); + if (selectedIp != null && GuestType.Shared.equals(network.getGuestType())) { + IPAddressVO ipAddressVO = _ipAddressDao.findByIpAndSourceNetworkId(network.getId(), selectedIp); + if (ipAddressVO != null && IpAddress.State.Free.equals(ipAddressVO.getState())) { + ipAddressVO.setState(IPAddressVO.State.Allocated); + ipAddressVO.setAllocatedTime(new Date()); + Account account = _accountDao.findById(vm.getAccountId()); + ipAddressVO.setAllocatedInDomainId(account.getDomainId()); + ipAddressVO.setAllocatedToAccountId(account.getId()); + _ipAddressDao.update(ipAddressVO.getId(), ipAddressVO); + } + } + final Integer networkRate = _networkModel.getNetworkRate(network.getId(), vm.getId()); final NicProfile vmNic = new NicProfile(vo, network, vo.getBroadcastUri(), vo.getIsolationUri(), networkRate, _networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag(vm.getHypervisorType(), network)); @@ -4783,15 +4795,15 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra if (network.getGuestType() == GuestType.L2) { return null; } - return dataCenter.getNetworkType() == NetworkType.Basic ? - getSelectedIpForNicImportOnBasicZone(ipAddresses.getIp4Address(), network, dataCenter): + return GuestType.Shared.equals(network.getGuestType()) ? + getSelectedIpForNicImportOnSharedNetwork(ipAddresses.getIp4Address(), network, dataCenter): _ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address()); } - protected String getSelectedIpForNicImportOnBasicZone(String requestedIp, Network network, DataCenter dataCenter) { + protected String getSelectedIpForNicImportOnSharedNetwork(String requestedIp, Network network, DataCenter dataCenter) { IPAddressVO ipAddressVO = StringUtils.isBlank(requestedIp) ? _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free): - _ipAddressDao.findByIp(requestedIp); + _ipAddressDao.findByIpAndSourceNetworkId(network.getId(), requestedIp); if (ipAddressVO == null || ipAddressVO.getState() != IpAddress.State.Free) { String msg = String.format("Cannot find a free IP to assign to VM NIC on network %s", network.getName()); logger.error(msg); diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java index 064ca44c69a..a95f0e36475 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java @@ -822,7 +822,7 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(network.getId()).thenReturn(networkId); Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); Mockito.when(ipAddresses.getIp4Address()).thenReturn(requestedIp); - Mockito.when(testOrchestrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); + Mockito.when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(networkId, requestedIp)).thenReturn(ipAddressVO); String ipAddress = testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); Assert.assertEquals(requestedIp, ipAddress); } diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index e5eb29a798d..0cf921f36be 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -71,7 +71,6 @@ import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.hypervisor.HypervisorGuru; import com.cloud.hypervisor.HypervisorGuruManager; -import com.cloud.network.IpAddressManager; import com.cloud.network.Network; import com.cloud.network.NetworkModel; import com.cloud.network.Networks; @@ -271,8 +270,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { @Inject private PhysicalNetworkDao physicalNetworkDao; @Inject - private IpAddressManager ipAddressManager; - @Inject private StoragePoolHostDao storagePoolHostDao; @Inject private HypervisorGuruManager hypervisorGuruManager; @@ -2613,10 +2610,8 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { } String macAddress = networkModel.getNextAvailableMacAddressInNetwork(networkId); - String ipAddress = null; - if (network.getGuestType() != Network.GuestType.L2) { - ipAddress = ipAddressManager.acquireGuestIpAddress(network, null); - } + + String ipAddress = network.getGuestType() != Network.GuestType.L2 ? "auto" : null; Network.IpAddresses requestedIpPair = new Network.IpAddresses(ipAddress, null, macAddress); From 6f931dbd005c26f0df9da7cf88e5bde707442d85 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 14 Oct 2025 14:23:45 +0530 Subject: [PATCH 4/9] agent: increase timeout for host arch retrieval (#11254) (#11822) Cherry-picked from 44f80648a9ea818e34997416aabbcd95cb03f847 Signed-off-by: Abhishek Kumar Co-authored-by: Abhishek Kumar --- agent/src/main/java/com/cloud/agent/Agent.java | 7 +++---- .../org/apache/cloudstack/utils/linux/KVMHostInfo.java | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/Agent.java b/agent/src/main/java/com/cloud/agent/Agent.java index 2e7b61fbd51..23b5e790eb9 100644 --- a/agent/src/main/java/com/cloud/agent/Agent.java +++ b/agent/src/main/java/com/cloud/agent/Agent.java @@ -93,7 +93,6 @@ import com.cloud.utils.nio.Link; import com.cloud.utils.nio.NioClient; import com.cloud.utils.nio.NioConnection; import com.cloud.utils.nio.Task; -import com.cloud.utils.script.OutputInterpreter; import com.cloud.utils.script.Script; /** @@ -598,9 +597,9 @@ public class Agent implements HandlerFactory, IAgentControl, AgentStatusUpdater } protected String getAgentArch() { - final Script command = new Script("/usr/bin/arch", 500, logger); - final OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser(); - return command.execute(parser); + String arch = Script.runSimpleBashScript(Script.getExecutableAbsolutePath("arch"), 2000); + logger.debug("Arch for agent: {} found: {}", _name, arch); + return arch; } @Override diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java index cee07421314..0792d09d975 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java @@ -58,7 +58,7 @@ public class KVMHostInfo { private long reservedMemory; private long overCommitMemory; private List capabilities = new ArrayList<>(); - private static String cpuArchCommand = "/usr/bin/arch"; + private static String cpuArchRetrieveExecutable = "arch"; private static List cpuInfoFreqFileNames = List.of("/sys/devices/system/cpu/cpu0/cpufreq/base_frequency","/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq"); public KVMHostInfo(long reservedMemory, long overCommitMemory, long manualSpeed, int reservedCpus) { @@ -248,6 +248,6 @@ public class KVMHostInfo { private String getCPUArchFromCommand() { LOGGER.info("Fetching host CPU arch"); - return Script.runSimpleBashScript(cpuArchCommand); + return Script.runSimpleBashScript(Script.getExecutableAbsolutePath(cpuArchRetrieveExecutable)); } } From 43278710362ec6c72f90fdc27e2b2be5b30a05f2 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Tue, 14 Oct 2025 13:00:33 +0200 Subject: [PATCH 5/9] Routed: fix create network exception when auto-allocation is disabled (#11624) * Routed: fix create network exception when auto-allocation is disabled for regular users * routed: throw InvalidParameterValueException instead of CloudRuntimeException which gives vague message to regular users --- .../network/RoutedIpv4ManagerImpl.java | 42 ++++++++++++------- .../network/RoutedIpv4ManagerImplTest.java | 4 +- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java index 50ec8a827b4..5174a5d7688 100644 --- a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java @@ -607,10 +607,20 @@ public class RoutedIpv4ManagerImpl extends ComponentLifecycleBase implements Rou } protected Ipv4GuestSubnetNetworkMap getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(Integer cidrSize, Long ownerDomainId, Long ownerAccountId, Long zoneId) { - validateNetworkCidrSize(ownerAccountId, cidrSize); + validateNetworkCidrSize(cidrSize); List subnets = getZoneSubnetsForAccount(ownerDomainId, ownerAccountId, zoneId); for (DataCenterIpv4GuestSubnetVO subnet : subnets) { - Ipv4GuestSubnetNetworkMap result = getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(cidrSize, subnet); + Ipv4GuestSubnetNetworkMap result = getIpv4SubnetForGuestNetworkOrVpcInternal(cidrSize, subnet); + if (result != null) { + return result; + } + } + Boolean isAutoAllocationEnabled = RoutedIPv4NetworkCidrAutoAllocationEnabled.valueIn(ownerAccountId); + if (!Boolean.TRUE.equals(isAutoAllocationEnabled)) { + throw new InvalidParameterValueException("CIDR auto-allocation is disabled for this account"); + } + for (DataCenterIpv4GuestSubnetVO subnet : subnets) { + Ipv4GuestSubnetNetworkMap result = createIpv4SubnetForGuestNetworkOrVpcInternal(cidrSize, subnet); if (result != null) { return result; } @@ -618,11 +628,11 @@ public class RoutedIpv4ManagerImpl extends ComponentLifecycleBase implements Rou return null; } - protected Ipv4GuestSubnetNetworkMap getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(Integer cidrSize, DataCenterIpv4GuestSubnetVO subnet) { - Ipv4GuestSubnetNetworkMap map = ipv4GuestSubnetNetworkMapDao.findFirstAvailable(subnet.getId(), cidrSize); - if (map != null) { - return map; - } + protected Ipv4GuestSubnetNetworkMap getIpv4SubnetForGuestNetworkOrVpcInternal(Integer cidrSize, DataCenterIpv4GuestSubnetVO subnet) { + return ipv4GuestSubnetNetworkMapDao.findFirstAvailable(subnet.getId(), cidrSize); + } + + protected Ipv4GuestSubnetNetworkMap createIpv4SubnetForGuestNetworkOrVpcInternal(Integer cidrSize, DataCenterIpv4GuestSubnetVO subnet) { try { return createIpv4SubnetFromParentSubnet(subnet, cidrSize); } catch (Exception ex) { @@ -631,6 +641,14 @@ public class RoutedIpv4ManagerImpl extends ComponentLifecycleBase implements Rou return null; } + protected Ipv4GuestSubnetNetworkMap getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(Integer cidrSize, DataCenterIpv4GuestSubnetVO subnet) { + Ipv4GuestSubnetNetworkMap map = getIpv4SubnetForGuestNetworkOrVpcInternal(cidrSize, subnet); + if (map != null) { + return map; + } + return createIpv4SubnetForGuestNetworkOrVpcInternal(cidrSize, subnet); + } + protected void getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(String networkCidr, Long ownerDomainId, Long ownerAccountId, Long zoneId) { Ipv4GuestSubnetNetworkMapVO subnetMap = ipv4GuestSubnetNetworkMapDao.findBySubnet(networkCidr); if (subnetMap != null) { @@ -693,13 +711,9 @@ public class RoutedIpv4ManagerImpl extends ComponentLifecycleBase implements Rou } } - private void validateNetworkCidrSize(long accountId, Integer networkCidrSize) { + private void validateNetworkCidrSize(Integer networkCidrSize) { if (networkCidrSize == null) { - throw new CloudRuntimeException("network/vpc CidrSize is null"); - } - Boolean isAutoAllocationEnabled = RoutedIPv4NetworkCidrAutoAllocationEnabled.valueIn(accountId); - if (!Boolean.TRUE.equals(isAutoAllocationEnabled)) { - throw new CloudRuntimeException("CIDR auto-allocation is disabled for this account"); + throw new InvalidParameterValueException("network/vpc CidrSize is null"); } } @@ -755,7 +769,7 @@ public class RoutedIpv4ManagerImpl extends ComponentLifecycleBase implements Rou // Allocate a subnet automatically String networkCidr = getFreeNetworkCidr(subnetsInFreeIpRanges, networkCidrSize); if (networkCidr == null) { - throw new CloudRuntimeException("Failed to automatically allocate a subnet with specified cidrsize"); + throw new InvalidParameterValueException("Failed to automatically allocate a subnet with specified cidrsize"); } return networkCidr; } diff --git a/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java index 5351abe2e2d..81110dd3f53 100644 --- a/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java @@ -545,12 +545,12 @@ public class RoutedIpv4ManagerImplTest { DataCenterIpv4GuestSubnetVO subnet3 = Mockito.mock(DataCenterIpv4GuestSubnetVO.class); when(dataCenterIpv4GuestSubnetDao.listNonDedicatedByDataCenterId(zoneId)).thenReturn(Arrays.asList(subnet3)); - doReturn(null).doReturn(null).doReturn(ipv4GuestSubnetNetworkMap).when(routedIpv4Manager).getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(eq(cidrSize), any()); + doReturn(null).doReturn(null).doReturn(ipv4GuestSubnetNetworkMap).when(routedIpv4Manager).getIpv4SubnetForGuestNetworkOrVpcInternal(eq(cidrSize), any()); Ipv4GuestSubnetNetworkMap result = routedIpv4Manager.getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(cidrSize, domainId, accountId, zoneId); Assert.assertEquals(ipv4GuestSubnetNetworkMap, result); - verify(routedIpv4Manager, times(3)).getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(eq(cidrSize), any()); + verify(routedIpv4Manager, times(3)).getIpv4SubnetForGuestNetworkOrVpcInternal(eq(cidrSize), any()); } @Test From b82369c2415dc8fb05c95ba0f1975788b9c143ef Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 15 Oct 2025 11:42:24 +0200 Subject: [PATCH 6/9] systemvm: fix duplicated "en_US.UTF-8 UTF-8" in /etc/locale.gen (#11823) --- tools/appliance/systemvmtemplate/scripts/configure_locale.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/appliance/systemvmtemplate/scripts/configure_locale.sh b/tools/appliance/systemvmtemplate/scripts/configure_locale.sh index 8db7e4e5598..f5477506cfd 100644 --- a/tools/appliance/systemvmtemplate/scripts/configure_locale.sh +++ b/tools/appliance/systemvmtemplate/scripts/configure_locale.sh @@ -22,13 +22,15 @@ set -x function configure_locale() { grep LANG=en_US.UTF-8 /etc/default/locale && \ grep LC_ALL=en_US.UTF-8 /etc/default/locale && \ - grep "en_US.UTF-8 UTF-8" /etc/locale.gen && + grep "^en_US.UTF-8 UTF-8" /etc/locale.gen && return cat >> /etc/default/locale << EOF LANG=en_US.UTF-8 LC_ALL=en_US.UTF-8 EOF + + grep "^en_US.UTF-8 UTF-8" /etc/locale.gen || \ cat >> /etc/locale.gen << EOF en_US.UTF-8 UTF-8 EOF From eee43e534fa0ad033a0ad79664cd23418086ee52 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 16 Oct 2025 00:01:00 +0530 Subject: [PATCH 7/9] cloudutils: fix warning, error during kvm agent installation (#11318) * cloudutils: fix warning, error during kvm agent installation Fixes #10379 Signed-off-by: Abhishek Kumar * fix Signed-off-by: Abhishek Kumar * Update utilities.py --------- Signed-off-by: Abhishek Kumar --- python/lib/cloudutils/configFileOps.py | 8 ++++---- python/lib/cloudutils/networkConfig.py | 13 ++++++++----- python/lib/cloudutils/utilities.py | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/python/lib/cloudutils/configFileOps.py b/python/lib/cloudutils/configFileOps.py index c061071b96e..f4d704af783 100644 --- a/python/lib/cloudutils/configFileOps.py +++ b/python/lib/cloudutils/configFileOps.py @@ -68,14 +68,14 @@ class configFileOps: for entry in self.entries: if entry.op == "add": if entry.separator == "=": - matchString = "^\ *" + entry.name + ".*" + matchString = r"^\ *" + entry.name + ".*" elif entry.separator == " ": - matchString = "^\ *" + entry.name + "\ *" + entry.value + matchString = r"^\ *" + entry.name + r"\ *" + entry.value else: if entry.separator == "=": - matchString = "^\ *" + entry.name + "\ *=\ *" + entry.value + matchString = r"^\ *" + entry.name + r"\ *=\ *" + entry.value else: - matchString = "^\ *" + entry.name + "\ *" + entry.value + matchString = r"^\ *" + entry.name + r"\ *" + entry.value match = re.match(matchString, line) if match is not None: diff --git a/python/lib/cloudutils/networkConfig.py b/python/lib/cloudutils/networkConfig.py index ac51f92aa58..827664bbc73 100644 --- a/python/lib/cloudutils/networkConfig.py +++ b/python/lib/cloudutils/networkConfig.py @@ -45,8 +45,11 @@ class networkConfig: if not cmd.isSuccess(): logging.debug("Failed to get default route") raise CloudRuntimeException("Failed to get default route") - - result = cmd.getStdout().split(" ") + output = cmd.getStdout().strip() + result = output.split(" ") + if len(result) < 2: + logging.debug("Output for the default route incomplete: %s. It should have been ' '" % output) + raise CloudRuntimeException("Output for the default route incomplete") gateway = result[0] dev = result[1] @@ -150,10 +153,10 @@ class networkConfig: if line.find("HWaddr") != -1: macAddr = line.split("HWaddr ")[1].strip(" ") elif line.find("inet ") != -1: - m = re.search("addr:(.*)\ *Bcast:(.*)\ *Mask:(.*)", line) + m = re.search(r"addr:([^\s]+)\s*Bcast:([^\s]+)\s*Mask:([^\s]+)", line) if m is not None: - ipAddr = m.group(1).rstrip(" ") - netmask = m.group(3).rstrip(" ") + ipAddr = m.group(1).strip() + netmask = m.group(3).strip() if networkConfig.isBridgePort(dev): type = "brport" diff --git a/python/lib/cloudutils/utilities.py b/python/lib/cloudutils/utilities.py index ce50516193e..95044de5558 100755 --- a/python/lib/cloudutils/utilities.py +++ b/python/lib/cloudutils/utilities.py @@ -63,7 +63,7 @@ class bash: return self.stdout.decode('utf-8').strip('\n') def getLines(self): - return self.stdout.decode('utf-8').strip('\n') + return self.stdout.decode('utf-8').strip('\n').split('\n') def getStderr(self): return self.stderr.decode('utf-8').strip('\n') From c8d44d92a76efe6dd197128dab8faf5f0f8ac85b Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 17 Sep 2025 02:34:31 +0530 Subject: [PATCH 8/9] api,server: fix entity access Added access check for: - createNetworkACL - listNetworkACLs - listResourceDetails - listVirtualMachinesUsageHistory - listVolumesUsageHistory Signed-off-by: Abhishek Kumar --- .../com/cloud/server/ResourceManagerUtil.java | 1 + .../metrics/MetricsServiceImpl.java | 76 ++++++++- .../metrics/MetricsServiceImplTest.java | 155 +++++++++++++++--- .../com/cloud/api/query/QueryManagerImpl.java | 2 +- .../network/vpc/NetworkACLServiceImpl.java | 3 + .../cloud/tags/ResourceManagerUtilImpl.java | 11 ++ 6 files changed, 217 insertions(+), 31 deletions(-) diff --git a/api/src/main/java/com/cloud/server/ResourceManagerUtil.java b/api/src/main/java/com/cloud/server/ResourceManagerUtil.java index 9a3b51a70d5..f5081cbe307 100644 --- a/api/src/main/java/com/cloud/server/ResourceManagerUtil.java +++ b/api/src/main/java/com/cloud/server/ResourceManagerUtil.java @@ -18,6 +18,7 @@ package com.cloud.server; public interface ResourceManagerUtil { long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType); + long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType, boolean checkAccess); String getUuid(String resourceId, ResourceTag.ResourceObjectType resourceType); ResourceTag.ResourceObjectType getResourceType(String resourceTypeStr); void checkResourceAccessible(Long accountId, Long domainId, String exceptionMessage); diff --git a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java index 0ef094d3d4e..c305d9312ba 100644 --- a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java +++ b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java @@ -22,6 +22,7 @@ import static com.cloud.utils.NumbersUtil.toReadableSize; import java.lang.reflect.InvocationTargetException; import java.text.DecimalFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -32,8 +33,6 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.dc.ClusterVO; -import com.cloud.utils.Ternary; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ListClustersMetricsCmd; import org.apache.cloudstack.api.ListDbMetricsCmd; @@ -100,6 +99,7 @@ import com.cloud.capacity.CapacityManager; import com.cloud.capacity.dao.CapacityDao; import com.cloud.capacity.dao.CapacityDaoImpl; import com.cloud.cluster.dao.ManagementServerHostDao; +import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenter; import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; @@ -112,6 +112,7 @@ import com.cloud.host.Status; import com.cloud.host.dao.HostDao; import com.cloud.network.router.VirtualRouter; import com.cloud.org.Cluster; +import com.cloud.projects.Project; import com.cloud.server.DbStatsCollection; import com.cloud.server.ManagementServerHostStats; import com.cloud.server.StatsCollector; @@ -124,6 +125,7 @@ import com.cloud.usage.dao.UsageJobDao; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; import com.cloud.utils.db.DbProperties; import com.cloud.utils.db.DbUtil; import com.cloud.utils.db.Filter; @@ -184,6 +186,10 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements private static Gson gson = new Gson(); + private final List AccountTypesWithRecursiveUsageAccess = Arrays.asList( + Account.Type.ADMIN, Account.Type.DOMAIN_ADMIN, Account.Type.READ_ONLY_ADMIN + ); + protected MetricsServiceImpl() { super(); } @@ -245,17 +251,30 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements * @return the list of VMs. */ protected Pair, Integer> searchForUserVmsInternal(ListVMsUsageHistoryCmd cmd) { + final Long id = cmd.getId(); + Account caller = CallContext.current().getCallingAccount(); + List permittedAccounts = new ArrayList<>(); + boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType()); + Ternary domainIdRecursiveListProject = new Ternary<>(null, recursive, null); + accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false); + Long domainId = domainIdRecursiveListProject.first(); + Boolean isRecursive = domainIdRecursiveListProject.second(); + Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + Filter searchFilter = new Filter(UserVmVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); List ids = getIdsListFromCmd(cmd.getId(), cmd.getIds()); String name = cmd.getName(); String keyword = cmd.getKeyword(); SearchBuilder sb = userVmDao.createSearchBuilder(); + sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); // select distinct + accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN); sb.and("displayName", sb.entity().getDisplayName(), SearchCriteria.Op.LIKE); sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); SearchCriteria sc = sb.create(); + accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); if (CollectionUtils.isNotEmpty(ids)) { sc.setParameters("idIN", ids.toArray()); } @@ -269,7 +288,14 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements sc.addAnd("displayName", SearchCriteria.Op.SC, ssc); } - return userVmDao.searchAndCount(sc, searchFilter); + Pair, Integer> uniqueVmPair = userVmDao.searchAndCount(sc, searchFilter); + Integer count = uniqueVmPair.second(); + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + List vmIds = uniqueVmPair.first().stream().map(UserVmVO::getId).collect(Collectors.toList()); + List vms = userVmDao.listByIds(vmIds); + return new Pair<>(vms, count); } /** @@ -331,17 +357,49 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements * @return the list of VMs. */ protected Pair, Integer> searchForVolumesInternal(ListVolumesUsageHistoryCmd cmd) { + final Long id = cmd.getId(); + Account caller = CallContext.current().getCallingAccount(); + List permittedAccounts = new ArrayList<>(); + boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType()); + Ternary domainIdRecursiveListProject = new Ternary<>(null, recursive, null); + accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false); + Long domainId = domainIdRecursiveListProject.first(); + Boolean isRecursive = domainIdRecursiveListProject.second(); + Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + Filter searchFilter = new Filter(VolumeVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); List ids = getIdsListFromCmd(cmd.getId(), cmd.getIds()); String name = cmd.getName(); String keyword = cmd.getKeyword(); SearchBuilder sb = volumeDao.createSearchBuilder(); + sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); // select distinct + accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN); sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ); sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); + boolean shouldListSystemVmVolumes = accountMgr.isRootAdmin(CallContext.current().getCallingAccountId()); + List vmIds = new ArrayList<>(); + if (!shouldListSystemVmVolumes) { + SearchBuilder vmSearch = userVmDao.createSearchBuilder(); + vmSearch.select(null, SearchCriteria.Func.DISTINCT, vmSearch.entity().getId()); + accountMgr.buildACLSearchBuilder(vmSearch, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + SearchCriteria vmSc = vmSearch.create(); + accountMgr.buildACLSearchCriteria(vmSc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + List vms = userVmDao.search(vmSc, null); + vmIds = vms.stream().map(UserVmVO::getId).collect(Collectors.toList()); + if (vmIds.isEmpty()) { + sb.and("instanceIdNull", sb.entity().getInstanceId(), SearchCriteria.Op.NULL); + } else { + sb.and().op("instanceIdNull", sb.entity().getInstanceId(), SearchCriteria.Op.NULL); + sb.or("instanceIds", sb.entity().getInstanceId(), SearchCriteria.Op.IN); + sb.cp(); + } + } + SearchCriteria sc = sb.create(); + accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); if (CollectionUtils.isNotEmpty(ids)) { sc.setParameters("idIN", ids.toArray()); } @@ -354,8 +412,18 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements ssc.addOr("state", SearchCriteria.Op.EQ, keyword); sc.addAnd("name", SearchCriteria.Op.SC, ssc); } + if (!shouldListSystemVmVolumes && CollectionUtils.isNotEmpty(vmIds)) { + sc.setParameters("instanceIds", vmIds.toArray()); + } - return volumeDao.searchAndCount(sc, searchFilter); + Pair, Integer> uniqueVolumePair = volumeDao.searchAndCount(sc, searchFilter); + Integer count = uniqueVolumePair.second(); + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + List volumeIds = uniqueVolumePair.first().stream().map(VolumeVO::getId).collect(Collectors.toList()); + List volumes = volumeDao.listByIds(volumeIds); + return new Pair<>(volumes, count); } /** diff --git a/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java b/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java index b37be68b3e4..9184d744410 100644 --- a/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java +++ b/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java @@ -19,13 +19,16 @@ package org.apache.cloudstack.metrics; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.cloudstack.api.ListVMsUsageHistoryCmd; +import org.apache.cloudstack.api.ListVolumesUsageHistoryCmd; import org.apache.cloudstack.api.response.ListResponse; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.response.VmMetricsStatsResponse; import org.apache.commons.lang3.time.DateUtils; import org.junit.Assert; @@ -35,12 +38,18 @@ import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; import com.cloud.utils.Pair; +import com.cloud.utils.db.Filter; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.vm.UserVmVO; @@ -75,6 +84,12 @@ public class MetricsServiceImplTest { @Mock VmStatsDao vmStatsDaoMock; + @Mock + AccountManager accountManager; + + @Mock + VolumeDao volumeDao; + @Captor ArgumentCaptor stringCaptor1, stringCaptor2; @@ -95,12 +110,26 @@ public class MetricsServiceImplTest { @Mock VmStatsVO vmStatsVOMock; + @Mock + Account mockAccount; + @Mock + ListVolumesUsageHistoryCmd listVolumesUsageHistoryCmdMock; + @Mock + VolumeVO volumeVOMock; + @Mock + SearchBuilder volumeSearchBuilderMock; + @Mock + SearchCriteria volumeSearchCriteriaMock; + @Mock + Filter filterMock; + private void prepareSearchCriteriaWhenUseSetParameters() { Mockito.doNothing().when(scMock).setParameters(Mockito.anyString(), Mockito.any()); } private void preparesearchForUserVmsInternalTest() { + Mockito.when(mockAccount.getType()).thenReturn(Account.Type.NORMAL); expectedVmListAndCounter = new Pair<>(Arrays.asList(userVmVOMock), 1); Mockito.doReturn(1L).when(listVMsUsageHistoryCmdMock).getStartIndex(); @@ -111,8 +140,10 @@ public class MetricsServiceImplTest { Mockito.doReturn(userVmVOMock).when(sbMock).entity(); Mockito.doReturn(scMock).when(sbMock).create(); - Mockito.doReturn(new Pair, Integer>(Arrays.asList(userVmVOMock), 1)) + Mockito.doReturn(expectedVmListAndCounter) .when(userVmDaoMock).searchAndCount(Mockito.any(), Mockito.any()); + Mockito.doReturn(expectedVmListAndCounter.first()) + .when(userVmDaoMock).listByIds(Mockito.anyList()); } @Test @@ -124,12 +155,17 @@ public class MetricsServiceImplTest { Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getName(); Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getKeyword(); - Pair, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); + try (MockedStatic callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); - Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); - Assert.assertEquals("idIN", stringCaptor1.getValue()); - Assert.assertEquals(fakeVmId1, objectArrayCaptor.getAllValues().get(0)[0]); - Assert.assertEquals(expectedVmListAndCounter, result); + Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); + Assert.assertEquals("idIN", stringCaptor1.getValue()); + Assert.assertEquals(fakeVmId1, objectArrayCaptor.getAllValues().get(0)[0]); + Assert.assertEquals(expectedVmListAndCounter, result); + } } @Test @@ -141,13 +177,17 @@ public class MetricsServiceImplTest { Mockito.doReturn(expected).when(listVMsUsageHistoryCmdMock).getIds(); Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getName(); Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getKeyword(); + try (MockedStatic callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); - Pair, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); - - Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); - Assert.assertEquals("idIN", stringCaptor1.getValue()); - Assert.assertArrayEquals(expected.toArray(), objectArrayCaptor.getAllValues().get(0)); - Assert.assertEquals(expectedVmListAndCounter, result); + Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); + Assert.assertEquals("idIN", stringCaptor1.getValue()); + Assert.assertArrayEquals(expected.toArray(), objectArrayCaptor.getAllValues().get(0)); + Assert.assertEquals(expectedVmListAndCounter, result); + } } @Test @@ -159,12 +199,17 @@ public class MetricsServiceImplTest { Mockito.doReturn("fakeName").when(listVMsUsageHistoryCmdMock).getName(); Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getKeyword(); - Pair, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); + try (MockedStatic callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); - Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); - Assert.assertEquals("displayName", stringCaptor1.getValue()); - Assert.assertEquals("%fakeName%", objectArrayCaptor.getValue()[0]); - Assert.assertEquals(expectedVmListAndCounter, result); + Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture()); + Assert.assertEquals("displayName", stringCaptor1.getValue()); + Assert.assertEquals("%fakeName%", objectArrayCaptor.getValue()[0]); + Assert.assertEquals(expectedVmListAndCounter, result); + } } @Test @@ -177,16 +222,21 @@ public class MetricsServiceImplTest { Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getName(); Mockito.doReturn("fakeKeyword").when(listVMsUsageHistoryCmdMock).getKeyword(); - Pair, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); + try (MockedStatic callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock); - Mockito.verify(scMock, Mockito.times(2)).addOr(stringCaptor1.capture(), opCaptor.capture(), objectArrayCaptor.capture()); - List conditions = stringCaptor1.getAllValues(); - List params = objectArrayCaptor.getAllValues(); - Assert.assertEquals("displayName", conditions.get(0)); - Assert.assertEquals("state", conditions.get(1)); - Assert.assertEquals("%fakeKeyword%", params.get(0)[0]); - Assert.assertEquals("fakeKeyword", params.get(1)[0]); - Assert.assertEquals(expectedVmListAndCounter, result); + Mockito.verify(scMock, Mockito.times(2)).addOr(stringCaptor1.capture(), opCaptor.capture(), objectArrayCaptor.capture()); + List conditions = stringCaptor1.getAllValues(); + List params = objectArrayCaptor.getAllValues(); + Assert.assertEquals("displayName", conditions.get(0)); + Assert.assertEquals("state", conditions.get(1)); + Assert.assertEquals("%fakeKeyword%", params.get(0)[0]); + Assert.assertEquals("fakeKeyword", params.get(1)[0]); + Assert.assertEquals(expectedVmListAndCounter, result); + } } @Test @@ -317,4 +367,57 @@ public class MetricsServiceImplTest { spy.createStatsResponse(Arrays.asList(vmStatsVOMock)); } + + @Test + public void searchForVolumesInternalWithValidParameters() { + Mockito.doReturn(null).when(listVolumesUsageHistoryCmdMock).getId(); + Mockito.doReturn(Arrays.asList(1L, 2L)).when(listVolumesUsageHistoryCmdMock).getIds(); + Mockito.doReturn("volumeName").when(listVolumesUsageHistoryCmdMock).getName(); + Mockito.doReturn("keyword").when(listVolumesUsageHistoryCmdMock).getKeyword(); + Mockito.doReturn(volumeSearchBuilderMock).when(volumeDao).createSearchBuilder(); + Mockito.doReturn(volumeVOMock).when(volumeSearchBuilderMock).entity(); + SearchBuilder vmSearchBuilderMock = Mockito.mock(SearchBuilder.class); + Mockito.doReturn(vmSearchBuilderMock).when(userVmDaoMock).createSearchBuilder(); + Mockito.doReturn(userVmVOMock).when(vmSearchBuilderMock).entity(); + Mockito.doReturn(volumeSearchCriteriaMock).when(volumeSearchBuilderMock).create(); + Mockito.doReturn(volumeSearchCriteriaMock).when(volumeDao).createSearchCriteria(); + Mockito.doReturn(new Pair<>(Arrays.asList(volumeVOMock), 1)).when(volumeDao).searchAndCount(Mockito.any(), Mockito.any()); + Mockito.doReturn(Arrays.asList(volumeVOMock)).when(volumeDao).listByIds(Mockito.anyList()); + + + try (MockedStatic callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Pair, Integer> result = spy.searchForVolumesInternal(listVolumesUsageHistoryCmdMock); + + Assert.assertNotNull(result); + Assert.assertEquals(1, result.second().intValue()); + Assert.assertEquals(volumeVOMock, result.first().get(0)); + } + } + + @Test + public void searchForVolumesInternalWithValidParametersNoItem() { + Mockito.doReturn(1L).when(listVolumesUsageHistoryCmdMock).getId(); + Mockito.doReturn(volumeSearchBuilderMock).when(volumeDao).createSearchBuilder(); + Mockito.doReturn(volumeVOMock).when(volumeSearchBuilderMock).entity(); + Mockito.doReturn(volumeSearchCriteriaMock).when(volumeSearchBuilderMock).create(); + Mockito.doReturn(new Pair<>(Collections.emptyList(), 0)).when(volumeDao).searchAndCount(Mockito.any(), Mockito.any()); + + + try (MockedStatic callContextMocked = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount); + Mockito.when(callContextMock.getCallingAccountId()).thenReturn(1L); + Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(true); + Mockito.when(mockAccount.getType()).thenReturn(Account.Type.ADMIN); + Pair, Integer> result = spy.searchForVolumesInternal(listVolumesUsageHistoryCmdMock); + + Assert.assertNotNull(result); + Assert.assertEquals(0, result.second().intValue()); + } + } + } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 6fb9ab515cb..6ed2fed9cb6 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -5335,7 +5335,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q //Validation - 1.3 if (resourceIdStr != null) { - resourceId = resourceManagerUtil.getResourceId(resourceIdStr, resourceType); + resourceId = resourceManagerUtil.getResourceId(resourceIdStr, resourceType, true); } List detailList = new ArrayList<>(); diff --git a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java index 0791ca7ecd3..f99ea6b9b8c 100644 --- a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java @@ -483,6 +483,8 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ throw new InvalidParameterValueException("Cannot create Network ACL Item. ACL Id or network Id is required"); } Network network = networkModel.getNetwork(createNetworkACLCmd.getNetworkId()); + Account caller = CallContext.current().getCallingAccount(); + _accountMgr.checkAccess(caller, null, true, network); if (network.getVpcId() == null) { throw new InvalidParameterValueException("Network: " + network.getUuid() + " does not belong to VPC"); } @@ -744,6 +746,7 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ if (networkId != null) { final Network network = _networkDao.findById(networkId); + _accountMgr.checkAccess(caller, null, true, network); aclId = network.getNetworkACLId(); if (aclId == null) { // No aclId associated with the network. diff --git a/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java b/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java index c02f4136863..82d69992cdf 100644 --- a/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java +++ b/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java @@ -22,6 +22,7 @@ import java.util.Objects; import javax.inject.Inject; +import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.api.Identity; import org.apache.cloudstack.api.InternalIdentity; import org.apache.cloudstack.context.CallContext; @@ -128,6 +129,11 @@ public class ResourceManagerUtilImpl implements ResourceManagerUtil { @Override public long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType) { + return getResourceId(resourceId, resourceType, false); + } + + @Override + public long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType, boolean checkAccess) { Class clazz = s_typeMap.get(resourceType); Object entity = entityMgr.findByUuid(clazz, resourceId); if (entity != null) { @@ -138,6 +144,11 @@ public class ResourceManagerUtilImpl implements ResourceManagerUtil { } entity = entityMgr.findById(clazz, resourceId); if (entity != null) { + if (checkAccess && entity instanceof ControlledEntity) { + ControlledEntity controlledEntity = (ControlledEntity)entity; + Account caller = CallContext.current().getCallingAccount(); + accountMgr.checkAccess(caller, null, false, controlledEntity); + } return ((InternalIdentity)entity).getId(); } throw new InvalidParameterValueException("Unable to find resource by id " + resourceId + " and type " + resourceType); From 03a4b9f4fd33d69717e2a21e5963e5f26612df90 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 17 Sep 2025 01:20:38 +0530 Subject: [PATCH 9/9] 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 --- .../com/cloud/server/ManagementService.java | 13 +- .../cloudstack/quota/QuotaManagerImpl.java | 4 +- .../quota/QuotaManagerImplTest.java | 4 +- .../response/QuotaResponseBuilderImpl.java | 23 ++- .../apache/cloudstack/quota/QuotaService.java | 12 +- .../cloudstack/quota/QuotaServiceImpl.java | 9 + .../cloud/resource/ResourceManagerImpl.java | 6 + .../cloud/server/ManagementServerImpl.java | 22 ++- .../com/cloud/storage/StorageManagerImpl.java | 9 + .../utils/jsinterpreter/JsInterpreter.java | 171 ++++++++++++------ .../utils/jsinterpreter/TagAsRuleHelper.java | 7 +- .../jsinterpreter/JsInterpreterTest.java | 81 ++++++--- 12 files changed, 253 insertions(+), 108 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 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 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 _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 _planners; + private boolean jsInterpretationEnabled = false; + private final List supportedHypervisors = new ArrayList<>(); public List 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 _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 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 org.apache.commons.collections.MapUtils; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.LogManager; +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 com.cloud.utils.exception.CloudRuntimeException; +import org.apache.commons.collections.MapUtils; +import org.apache.logging.log4j.LogManager; +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 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 variables = new LinkedHashMap<>(); + + // Store variables as Objects; they go into Bindings (no code splicing) + protected Map 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); + + 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; + } + ); - executor = Executors.newSingleThreadExecutor(); 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 task = () -> interpreter.eval(script); + final Callable task = () -> { + final SimpleScriptContext ctx = new SimpleScriptContext(); - Future 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 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 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 variables = new LinkedHashMap<>(); + Map 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); } }