diff --git a/api/src/main/java/com/cloud/user/AccountService.java b/api/src/main/java/com/cloud/user/AccountService.java index 63f5455cfd0..60db7abb734 100644 --- a/api/src/main/java/com/cloud/user/AccountService.java +++ b/api/src/main/java/com/cloud/user/AccountService.java @@ -116,6 +116,8 @@ public interface AccountService { void checkAccess(Account account, AccessType accessType, boolean sameOwner, String apiName, ControlledEntity... entities) throws PermissionDeniedException; + void validateAccountHasAccessToResource(Account account, AccessType accessType, Object resource); + Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly); /** diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupScheduleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupScheduleCmd.java index a7610717435..fa6e3ea5d45 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupScheduleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupScheduleCmd.java @@ -82,7 +82,7 @@ public class ListBackupScheduleCmd extends BaseCmd { List schedules = backupManager.listBackupSchedule(getVmId()); ListResponse response = new ListResponse<>(); List scheduleResponses = new ArrayList<>(); - if (CollectionUtils.isNullOrEmpty(schedules)) { + if (!CollectionUtils.isNullOrEmpty(schedules)) { for (BackupSchedule schedule : schedules) { scheduleResponses.add(_responseGenerator.createBackupScheduleResponse(schedule)); } diff --git a/debian/control b/debian/control index ed4f95d8e49..c0cb95af035 100644 --- a/debian/control +++ b/debian/control @@ -24,7 +24,7 @@ Description: CloudStack server library Package: cloudstack-agent Architecture: all -Depends: ${python:Depends}, ${python3:Depends}, openjdk-17-jre-headless | java17-runtime-headless | java17-runtime | zulu-17, cloudstack-common (= ${source:Version}), lsb-base (>= 9), openssh-client, qemu-kvm (>= 2.5) | qemu-system-x86 (>= 5.2), libvirt-bin (>= 1.3) | libvirt-daemon-system (>= 3.0), iproute2, ebtables, vlan, ipset, python3-libvirt, ethtool, iptables, cryptsetup, rng-tools, rsync, lsb-release, ufw, apparmor, cpu-checker +Depends: ${python:Depends}, ${python3:Depends}, openjdk-17-jre-headless | java17-runtime-headless | java17-runtime | zulu-17, cloudstack-common (= ${source:Version}), lsb-base (>= 9), openssh-client, qemu-kvm (>= 2.5) | qemu-system-x86 (>= 5.2), libvirt-bin (>= 1.3) | libvirt-daemon-system (>= 3.0), iproute2, ebtables, vlan, ipset, python3-libvirt, ethtool, iptables, cryptsetup, rng-tools, rsync, lsb-release, ufw, apparmor, cpu-checker, libvirt-daemon-driver-storage-rbd Recommends: init-system-helpers Conflicts: cloud-agent, cloud-agent-libs, cloud-agent-deps, cloud-agent-scripts Description: CloudStack agent diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDaoImpl.java index d1149e47408..e4c19fd1666 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDaoImpl.java @@ -126,14 +126,7 @@ public class VolumeStatsDaoImpl extends GenericDaoBase impl logger.debug(String.format("Starting to remove all volume_stats rows older than [%s].", limitDate)); - long totalRemoved = 0; - long removed; - - do { - removed = expunge(sc, limitPerQuery); - totalRemoved += removed; - logger.trace(String.format("Removed [%s] volume_stats rows on the last update and a sum of [%s] volume_stats rows older than [%s] until now.", removed, totalRemoved, limitDate)); - } while (limitPerQuery > 0 && removed >= limitPerQuery); + long totalRemoved = batchExpunge(sc, limitPerQuery); logger.info(String.format("Removed a total of [%s] volume_stats rows older than [%s].", totalRemoved, limitDate)); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java index aa58e489364..327acec0c17 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java @@ -123,14 +123,7 @@ public class VmStatsDaoImpl extends GenericDaoBase implements V logger.debug(String.format("Starting to remove all vm_stats rows older than [%s].", limitDate)); - long totalRemoved = 0; - long removed; - - do { - removed = expunge(sc, limitPerQuery); - totalRemoved += removed; - logger.trace(String.format("Removed [%s] vm_stats rows on the last update and a sum of [%s] vm_stats rows older than [%s] until now.", removed, totalRemoved, limitDate)); - } while (limitPerQuery > 0 && removed >= limitPerQuery); + long totalRemoved = batchExpunge(sc, limitPerQuery); logger.info(String.format("Removed a total of [%s] vm_stats rows older than [%s].", totalRemoved, limitDate)); } diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index 07b53842640..9755dcbb0df 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java @@ -1958,25 +1958,26 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { * - Full clones (no backing file): Take snapshot of the VM prior disk creation * Return this information */ - protected void setVolumeMigrationOptions(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo, - VirtualMachineTO vmTO, Host srcHost, StoragePoolVO destStoragePool) { - if (!destStoragePool.isManaged()) { - String srcVolumeBackingFile = getVolumeBackingFile(srcVolumeInfo); - - String srcPoolUuid = srcVolumeInfo.getDataStore().getUuid(); - StoragePoolVO srcPool = _storagePoolDao.findById(srcVolumeInfo.getPoolId()); - Storage.StoragePoolType srcPoolType = srcPool.getPoolType(); - - MigrationOptions migrationOptions; - if (StringUtils.isNotBlank(srcVolumeBackingFile)) { - migrationOptions = createLinkedCloneMigrationOptions(srcVolumeInfo, destVolumeInfo, - srcVolumeBackingFile, srcPoolUuid, srcPoolType); - } else { - migrationOptions = createFullCloneMigrationOptions(srcVolumeInfo, vmTO, srcHost, srcPoolUuid, srcPoolType); - } - migrationOptions.setTimeout(StorageManager.KvmStorageOnlineMigrationWait.value()); - destVolumeInfo.setMigrationOptions(migrationOptions); + protected void setVolumeMigrationOptions(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo, VirtualMachineTO vmTO, Host srcHost, StoragePoolVO destStoragePool, + MigrationOptions.Type migrationType) { + if (destStoragePool.isManaged()) { + return; } + + String srcVolumeBackingFile = getVolumeBackingFile(srcVolumeInfo); + + String srcPoolUuid = srcVolumeInfo.getDataStore().getUuid(); + StoragePoolVO srcPool = _storagePoolDao.findById(srcVolumeInfo.getPoolId()); + Storage.StoragePoolType srcPoolType = srcPool.getPoolType(); + + MigrationOptions migrationOptions; + if (MigrationOptions.Type.LinkedClone.equals(migrationType)) { + migrationOptions = createLinkedCloneMigrationOptions(srcVolumeInfo, destVolumeInfo, srcVolumeBackingFile, srcPoolUuid, srcPoolType); + } else { + migrationOptions = createFullCloneMigrationOptions(srcVolumeInfo, vmTO, srcHost, srcPoolUuid, srcPoolType); + } + migrationOptions.setTimeout(StorageManager.KvmStorageOnlineMigrationWait.value()); + destVolumeInfo.setMigrationOptions(migrationOptions); } /** @@ -2007,6 +2008,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { Map srcVolumeInfoToDestVolumeInfo = new HashMap<>(); boolean managedStorageDestination = false; + boolean migrateNonSharedInc = false; for (Map.Entry entry : volumeDataStoreMap.entrySet()) { VolumeInfo srcVolumeInfo = entry.getKey(); DataStore destDataStore = entry.getValue(); @@ -2024,15 +2026,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { continue; } - VMTemplateVO vmTemplate = _vmTemplateDao.findById(vmInstance.getTemplateId()); - if (srcVolumeInfo.getTemplateId() != null && - Objects.nonNull(vmTemplate) && - !Arrays.asList(KVM_VM_IMPORT_DEFAULT_TEMPLATE_NAME, VM_IMPORT_DEFAULT_TEMPLATE_NAME).contains(vmTemplate.getName())) { - logger.debug(String.format("Copying template [%s] of volume [%s] from source storage pool [%s] to target storage pool [%s].", srcVolumeInfo.getTemplateId(), srcVolumeInfo.getId(), sourceStoragePool.getId(), destStoragePool.getId())); - copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, sourceStoragePool, destDataStore, destStoragePool, destHost); - } else { - logger.debug(String.format("Skipping copy template from source storage pool [%s] to target storage pool [%s] before migration due to volume [%s] does not have a template.", sourceStoragePool.getId(), destStoragePool.getId(), srcVolumeInfo.getId())); - } + MigrationOptions.Type migrationType = decideMigrationTypeAndCopyTemplateIfNeeded(destHost, vmInstance, srcVolumeInfo, sourceStoragePool, destStoragePool, destDataStore); + migrateNonSharedInc = migrateNonSharedInc || MigrationOptions.Type.LinkedClone.equals(migrationType); VolumeVO destVolume = duplicateVolumeOnAnotherStorage(srcVolume, destStoragePool); VolumeInfo destVolumeInfo = _volumeDataFactory.getVolume(destVolume.getId(), destDataStore); @@ -2044,7 +2039,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { // move the volume from Ready to Migrating destVolumeInfo.processEvent(Event.MigrationRequested); - setVolumeMigrationOptions(srcVolumeInfo, destVolumeInfo, vmTO, srcHost, destStoragePool); + setVolumeMigrationOptions(srcVolumeInfo, destVolumeInfo, vmTO, srcHost, destStoragePool, migrationType); // create a volume on the destination storage destDataStore.getDriver().createAsync(destDataStore, destVolumeInfo, null); @@ -2059,7 +2054,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { _volumeDao.update(destVolume.getId(), destVolume); - postVolumeCreationActions(srcVolumeInfo, destVolumeInfo, vmTO, srcHost); + postVolumeCreationActions(srcVolumeInfo, destVolumeInfo); destVolumeInfo = _volumeDataFactory.getVolume(destVolume.getId(), destDataStore); @@ -2083,8 +2078,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { migrateDiskInfo = configureMigrateDiskInfo(srcVolumeInfo, destPath, backingPath); migrateDiskInfo.setSourceDiskOnStorageFileSystem(isStoragePoolTypeOfFile(sourceStoragePool)); migrateDiskInfoList.add(migrateDiskInfo); - prepareDiskWithSecretConsumerDetail(vmTO, srcVolumeInfo, destVolumeInfo.getPath()); } + prepareDiskWithSecretConsumerDetail(vmTO, srcVolumeInfo, destVolumeInfo.getPath()); migrateStorage.put(srcVolumeInfo.getPath(), migrateDiskInfo); @@ -2110,8 +2105,6 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { VMInstanceVO vm = _vmDao.findById(vmTO.getId()); boolean isWindows = _guestOsCategoryDao.findById(_guestOsDao.findById(vm.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows"); - boolean migrateNonSharedInc = isSourceAndDestinationPoolTypeOfNfs(volumeDataStoreMap); - MigrateCommand migrateCommand = new MigrateCommand(vmTO.getName(), destHost.getPrivateIpAddress(), isWindows, vmTO, true); migrateCommand.setWait(StorageManager.KvmStorageOnlineMigrationWait.value()); migrateCommand.setMigrateStorage(migrateStorage); @@ -2161,6 +2154,22 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { } } + private MigrationOptions.Type decideMigrationTypeAndCopyTemplateIfNeeded(Host destHost, VMInstanceVO vmInstance, VolumeInfo srcVolumeInfo, StoragePoolVO sourceStoragePool, StoragePoolVO destStoragePool, DataStore destDataStore) { + VMTemplateVO vmTemplate = _vmTemplateDao.findById(vmInstance.getTemplateId()); + String srcVolumeBackingFile = getVolumeBackingFile(srcVolumeInfo); + if (StringUtils.isNotBlank(srcVolumeBackingFile) && supportStoragePoolType(destStoragePool.getPoolType(), StoragePoolType.Filesystem) && + srcVolumeInfo.getTemplateId() != null && + Objects.nonNull(vmTemplate) && + !Arrays.asList(KVM_VM_IMPORT_DEFAULT_TEMPLATE_NAME, VM_IMPORT_DEFAULT_TEMPLATE_NAME).contains(vmTemplate.getName())) { + logger.debug(String.format("Copying template [%s] of volume [%s] from source storage pool [%s] to target storage pool [%s].", srcVolumeInfo.getTemplateId(), srcVolumeInfo.getId(), sourceStoragePool.getId(), destStoragePool.getId())); + copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, sourceStoragePool, destDataStore, destStoragePool, destHost); + return MigrationOptions.Type.LinkedClone; + } + logger.debug(String.format("Skipping copy template from source storage pool [%s] to target storage pool [%s] before migration due to volume [%s] does not have a " + + "template or we are doing full clone migration.", sourceStoragePool.getId(), destStoragePool.getId(), srcVolumeInfo.getId())); + return MigrationOptions.Type.FullClone; + } + protected String formatMigrationElementsAsJsonToDisplayOnLog(String objectName, Object object, Object from, Object to){ return String.format("{%s: \"%s\", from: \"%s\", to:\"%s\"}", objectName, object, from, to); } @@ -2422,7 +2431,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { /** * Handle post destination volume creation actions depending on the migrating volume type: full clone or linked clone */ - protected void postVolumeCreationActions(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo, VirtualMachineTO vmTO, Host srcHost) { + protected void postVolumeCreationActions(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo) { MigrationOptions migrationOptions = destVolumeInfo.getMigrationOptions(); if (migrationOptions != null) { if (migrationOptions.getType() == MigrationOptions.Type.LinkedClone && migrationOptions.isCopySrcTemplate()) { @@ -2470,7 +2479,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { throw new CloudRuntimeException("Destination storage pool with ID " + dataStore.getId() + " was not located."); } - if (srcStoragePoolVO.isManaged() && srcStoragePoolVO.getId() != destStoragePoolVO.getId()) { + boolean isSrcAndDestPoolPowerFlexStorage = srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex) && destStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex); + if (srcStoragePoolVO.isManaged() && !isSrcAndDestPoolPowerFlexStorage && srcStoragePoolVO.getId() != destStoragePoolVO.getId()) { throw new CloudRuntimeException("Migrating a volume online with KVM from managed storage is not currently supported."); } diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java index 84750c2068c..de8838b0999 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java @@ -247,14 +247,6 @@ public interface GenericDao { int expungeList(List ids); - /** - * Delete the entity beans specified by the search criteria with a given limit - * @param sc Search criteria - * @param limit Maximum number of rows that will be affected - * @return Number of rows deleted - */ - int expunge(SearchCriteria sc, long limit); - /** * expunge the removed rows. */ diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index 52a6b204ee8..82fea9749ff 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -1244,13 +1244,6 @@ public abstract class GenericDaoBase extends Compone } } - // FIXME: Does not work for joins. - @Override - public int expunge(final SearchCriteria sc, long limit) { - Filter filter = new Filter(limit); - return expunge(sc, filter); - } - @Override public int expunge(final SearchCriteria sc, final Filter filter) { if (sc == null) { @@ -1400,22 +1393,39 @@ public abstract class GenericDaoBase extends Compone onClause.append("?"); joinAttrList.add(join.getFirstAttributes()[i]); } else { - onClause.append(joinedTableNames.getOrDefault(join.getFirstAttributes()[i].table, join.getFirstAttributes()[i].table)) - .append(".") - .append(join.getFirstAttributes()[i].columnName); - } - onClause.append("="); - if (join.getSecondAttribute()[i].getValue() != null) { - onClause.append("?"); - joinAttrList.add(join.getSecondAttribute()[i]); - } else { - if(!joinTableAlias.equals(joinTableName)) { - onClause.append(joinTableAlias); + if ((join.getFirstAttributes()[i].table == null && join.getFirstAttributes()[i].value == null) || + (join.getSecondAttribute()[i].table == null && join.getSecondAttribute()[i].value == null)) { + onClause.append(joinedTableNames.getOrDefault(join.getSecondAttribute()[i].table, join.getFirstAttributes()[i].table)) + .append("."); + if (join.getFirstAttributes()[i].table == null && join.getFirstAttributes()[i].value == null) { + onClause.append(join.getSecondAttribute()[i].columnName); + } else { + onClause.append(join.getFirstAttributes()[i].columnName); + } + } else { - onClause.append(joinTableName); + onClause.append(joinedTableNames.getOrDefault(join.getFirstAttributes()[i].table, join.getFirstAttributes()[i].table)) + .append(".") + .append(join.getFirstAttributes()[i].columnName); + } + } + if ((join.getFirstAttributes()[i].table == null && join.getFirstAttributes()[i].value == null) || + (join.getSecondAttribute()[i].table == null && join.getSecondAttribute()[i].value == null)) { + onClause.append(" IS NULL"); + } else { + onClause.append("="); + if (join.getSecondAttribute()[i].getValue() != null) { + onClause.append("?"); + joinAttrList.add(join.getSecondAttribute()[i]); + } else { + if (!joinTableAlias.equals(joinTableName)) { + onClause.append(joinTableAlias); + } else { + onClause.append(joinTableName); + } + onClause.append(".") + .append(join.getSecondAttribute()[i].columnName); } - onClause.append(".") - .append(join.getSecondAttribute()[i].columnName); } } onClause.append(" "); diff --git a/packaging/el8/cloud.spec b/packaging/el8/cloud.spec index a88d4b1cbbf..f9d6fd1ce87 100644 --- a/packaging/el8/cloud.spec +++ b/packaging/el8/cloud.spec @@ -102,6 +102,7 @@ Requires: java-17-openjdk Requires: tzdata-java Requires: %{name}-common = %{_ver} Requires: libvirt +Requires: libvirt-daemon-driver-storage-rbd Requires: ebtables Requires: iptables Requires: ethtool diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaBalanceCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaBalanceCmd.java index 218e3c2b2f9..628eca56c2d 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaBalanceCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaBalanceCmd.java @@ -21,6 +21,9 @@ import java.util.List; import javax.inject.Inject; +import com.cloud.user.Account; + +import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseCmd; @@ -40,6 +43,7 @@ public class QuotaBalanceCmd extends BaseCmd { @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, required = true, description = "Account Id for which statement needs to be generated") private String accountName; + @ACL @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = true, entityType = DomainResponse.class, description = "If domain Id is given and the caller is domain admin then the statement is generated for domain.") private Long domainId; @@ -51,6 +55,7 @@ public class QuotaBalanceCmd extends BaseCmd { ApiConstants.PARAMETER_DESCRIPTION_START_DATE_POSSIBLE_FORMATS) private Date startDate; + @ACL @Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, description = "List usage records for the specified account") private Long accountId; @@ -104,7 +109,14 @@ public class QuotaBalanceCmd extends BaseCmd { @Override public long getEntityOwnerId() { - return _accountService.getActiveAccountByName(accountName, domainId).getAccountId(); + if (accountId != null) { + return accountId; + } + Account account = _accountService.getActiveAccountByName(accountName, domainId); + if (account != null) { + return account.getAccountId(); + } + return Account.ACCOUNT_ID_SYSTEM; } @Override diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaCreditsCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaCreditsCmd.java index 8ca29f275dd..cfa391291c8 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaCreditsCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaCreditsCmd.java @@ -18,6 +18,7 @@ package org.apache.cloudstack.api.command; import com.cloud.user.Account; +import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -46,6 +47,7 @@ public class QuotaCreditsCmd extends BaseCmd { @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, required = true, description = "Account Id for which quota credits need to be added") private String accountName; + @ACL @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = true, entityType = DomainResponse.class, description = "Domain for which quota credits need to be added") private Long domainId; @@ -130,6 +132,10 @@ public class QuotaCreditsCmd extends BaseCmd { @Override public long getEntityOwnerId() { + Account account = _accountService.getActiveAccountByName(accountName, domainId); + if (account != null) { + return account.getAccountId(); + } return Account.ACCOUNT_ID_SYSTEM; } diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaStatementCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaStatementCmd.java index 4fb33f79672..78fa0f7df84 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaStatementCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaStatementCmd.java @@ -21,6 +21,7 @@ import java.util.List; import javax.inject.Inject; +import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseCmd; @@ -42,6 +43,7 @@ public class QuotaStatementCmd extends BaseCmd { @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, required = true, description = "Optional, Account Id for which statement needs to be generated") private String accountName; + @ACL @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = true, entityType = DomainResponse.class, description = "Optional, If domain Id is given and the caller is domain admin then the statement is generated for domain.") private Long domainId; @@ -56,6 +58,7 @@ public class QuotaStatementCmd extends BaseCmd { @Parameter(name = ApiConstants.TYPE, type = CommandType.INTEGER, description = "List quota usage records for the specified usage type") private Integer usageType; + @ACL @Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, description = "List usage records for the specified account") private Long accountId; @@ -112,6 +115,9 @@ public class QuotaStatementCmd extends BaseCmd { @Override public long getEntityOwnerId() { + if (accountId != null) { + return accountId; + } Account activeAccountByName = _accountService.getActiveAccountByName(accountName, domainId); if (activeAccountByName != null) { return activeAccountByName.getAccountId(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index abd6e24a532..0b0416a8049 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -381,6 +381,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv protected static final String DEFAULT_TUNGSTEN_VIF_DRIVER_CLASS_NAME = "com.cloud.hypervisor.kvm.resource.VRouterVifDriver"; private final static long HYPERVISOR_LIBVIRT_VERSION_SUPPORTS_IO_URING = 6003000; private final static long HYPERVISOR_QEMU_VERSION_SUPPORTS_IO_URING = 5000000; + private final static long HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED = 7000000; protected HypervisorType hypervisorType; protected String hypervisorURI; @@ -3050,7 +3051,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } } else if (volume.getType() != Volume.Type.ISO) { final PrimaryDataStoreTO store = (PrimaryDataStoreTO)data.getDataStore(); - physicalDisk = storagePoolManager.getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath()); + physicalDisk = getStoragePoolMgr().getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath()); pool = physicalDisk.getPool(); } @@ -3152,6 +3153,9 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv else { disk.defBlockBasedDisk(physicalDisk.getPath(), devId, diskBusType); } + if (pool.getType() == StoragePoolType.Linstor && isQemuDiscardBugFree(diskBusType)) { + disk.setDiscard(DiscardType.UNMAP); + } } else { if (volume.getType() == Volume.Type.DATADISK && !(isWindowsTemplate && isUefiEnabled)) { disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2); @@ -3296,6 +3300,16 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return isUbuntuHost() || isIoUringSupportedByQemu(); } + /** + * Qemu has a bug with discard enabled on IDE bus devices if qemu version < 7.0. + * redhat bug entry + * @param diskBus used for the disk + * @return true if it is safe to enable discard, otherwise false. + */ + public boolean isQemuDiscardBugFree(DiskDef.DiskBus diskBus) { + return diskBus != DiskDef.DiskBus.IDE || getHypervisorQemuVersion() >= HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED; + } + public boolean isUbuntuHost() { Map versionString = getVersionStrings(); String hostKey = "Host.OS"; @@ -3498,6 +3512,9 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv diskdef.defFileBasedDisk(attachingDisk.getPath(), devId, busT, DiskDef.DiskFmtType.QCOW2); } else if (attachingDisk.getFormat() == PhysicalDiskFormat.RAW) { diskdef.defBlockBasedDisk(attachingDisk.getPath(), devId, busT); + if (attachingPool.getType() == StoragePoolType.Linstor) { + diskdef.setDiscard(DiscardType.UNMAP); + } } if (bytesReadRate != null && bytesReadRate > 0) { diskdef.setBytesReadRate(bytesReadRate); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java index ae8d06a1986..8f027e01ca4 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java @@ -123,8 +123,10 @@ public class MigrateKVMAsync implements Callable { if (migrateNonSharedInc) { flags |= VIR_MIGRATE_PERSIST_DEST; flags |= VIR_MIGRATE_NON_SHARED_INC; + logger.debug("Setting VIR_MIGRATE_NON_SHARED_INC for linked clone migration."); } else { flags |= VIR_MIGRATE_NON_SHARED_DISK; + logger.debug("Setting VIR_MIGRATE_NON_SHARED_DISK for full clone migration."); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsFetchInterfaceCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsFetchInterfaceCommandWrapper.java index 923c44f4828..9ecdfddc2ad 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsFetchInterfaceCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsFetchInterfaceCommandWrapper.java @@ -89,7 +89,7 @@ public final class LibvirtOvsFetchInterfaceCommandWrapper extends CommandWrapper @Override public Answer execute(final OvsFetchInterfaceCommand command, final LibvirtComputingResource libvirtComputingResource) { - final String label = "'" + command.getLabel() + "'"; + final String label = command.getLabel(); logger.debug("Will look for network with name-label:" + label); try { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 04662604382..391c7674552 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1447,6 +1447,9 @@ public class KVMStorageProcessor implements StorageProcessor { diskdef.defFileBasedDisk(attachingDisk.getPath(), devId, busT, DiskDef.DiskFmtType.QCOW2); } else if (attachingDisk.getFormat() == PhysicalDiskFormat.RAW) { diskdef.defBlockBasedDisk(attachingDisk.getPath(), devId, busT); + if (attachingPool.getType() == StoragePoolType.Linstor && resource.isQemuDiscardBugFree(busT)) { + diskdef.setDiscard(DiscardType.UNMAP); + } } if (encryptDetails != null) { diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 30d0b2ab163..15c2b46e077 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -58,9 +58,12 @@ import javax.xml.xpath.XPathFactory; import com.cloud.utils.net.NetUtils; +import com.cloud.vm.VmDetailConstants; import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy; import org.apache.cloudstack.storage.command.AttachAnswer; import org.apache.cloudstack.storage.command.AttachCommand; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; import org.apache.cloudstack.utils.linux.CPUStat; import org.apache.cloudstack.utils.linux.MemStat; @@ -6415,4 +6418,114 @@ public class LibvirtComputingResourceTest { Assert.assertEquals(newStats.getDiskReadKBs() - oldStats.getDiskReadKBs(), metrics.getDiskReadKBs(), 0); Assert.assertEquals(newStats.getDiskWriteKBs() - oldStats.getDiskWriteKBs(), metrics.getDiskWriteKBs(), 0); } + + @Test + public void createLinstorVdb() throws LibvirtException, InternalErrorException, URISyntaxException { + final Connect connect = Mockito.mock(Connect.class); + + final int id = random.nextInt(65534); + final String name = "test-instance-1"; + + final int cpus = 2; + final int speed = 1024; + final int minRam = 256 * 1024; + final int maxRam = 512 * 1024; + final String os = "Ubuntu"; + final String vncPassword = "mySuperSecretPassword"; + + final VirtualMachineTO to = new VirtualMachineTO(id, name, VirtualMachine.Type.User, cpus, speed, minRam, + maxRam, BootloaderType.HVM, os, false, false, vncPassword); + to.setVncAddr(""); + to.setArch("x86_64"); + to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9"); + to.setVcpuMaxLimit(cpus + 1); + final HashMap vmToDetails = new HashMap<>(); + to.setDetails(vmToDetails); + + String diskLinPath = "9ebe53c1-3d35-46e5-b7aa-6fc223ba0fcf"; + final DiskTO diskTO = new DiskTO(); + diskTO.setDiskSeq(1L); + diskTO.setType(Volume.Type.ROOT); + diskTO.setDetails(new HashMap<>()); + diskTO.setPath(diskLinPath); + + final PrimaryDataStoreTO primaryDataStoreTO = Mockito.mock(PrimaryDataStoreTO.class); + String pDSTOUUID = "9ebe53c1-3d35-46e5-b7aa-6fc223ac4fcf"; + when(primaryDataStoreTO.getPoolType()).thenReturn(StoragePoolType.Linstor); + when(primaryDataStoreTO.getUuid()).thenReturn(pDSTOUUID); + + VolumeObjectTO dataTO = new VolumeObjectTO(); + + dataTO.setUuid("12be53c1-3d35-46e5-b7aa-6fc223ba0f34"); + dataTO.setPath(diskTO.getPath()); + dataTO.setDataStore(primaryDataStoreTO); + diskTO.setData(dataTO); + to.setDisks(new DiskTO[]{diskTO}); + + String path = "/dev/drbd1020"; + final KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + final KVMStoragePool storagePool = Mockito.mock(KVMStoragePool.class); + final KVMPhysicalDisk vol = Mockito.mock(KVMPhysicalDisk.class); + + when(libvirtComputingResourceSpy.getStoragePoolMgr()).thenReturn(storagePoolMgr); + when(storagePool.getType()).thenReturn(StoragePoolType.Linstor); + when(storagePoolMgr.getPhysicalDisk(StoragePoolType.Linstor, pDSTOUUID, diskLinPath)).thenReturn(vol); + when(vol.getPath()).thenReturn(path); + when(vol.getPool()).thenReturn(storagePool); + when(vol.getFormat()).thenReturn(PhysicalDiskFormat.RAW); + + // 1. test Bus: IDE and broken qemu version -> NO discard + when(libvirtComputingResourceSpy.getHypervisorQemuVersion()).thenReturn(6000000L); + vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.IDE.name()); + { + LibvirtVMDef vm = new LibvirtVMDef(); + vm.addComp(new DevicesDef()); + libvirtComputingResourceSpy.createVbd(connect, to, name, vm); + + DiskDef rootDisk = vm.getDevices().getDisks().get(0); + assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType()); + assertEquals(DiskDef.DiskBus.IDE, rootDisk.getBusType()); + assertEquals(DiskDef.DiscardType.IGNORE, rootDisk.getDiscard()); + } + + // 2. test Bus: VIRTIO and broken qemu version -> discard unmap + vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.VIRTIO.name()); + { + LibvirtVMDef vm = new LibvirtVMDef(); + vm.addComp(new DevicesDef()); + libvirtComputingResourceSpy.createVbd(connect, to, name, vm); + + DiskDef rootDisk = vm.getDevices().getDisks().get(0); + assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType()); + assertEquals(DiskDef.DiskBus.VIRTIO, rootDisk.getBusType()); + assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard()); + } + + // 3. test Bus; IDE and "good" qemu version -> discard unmap + vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.IDE.name()); + when(libvirtComputingResourceSpy.getHypervisorQemuVersion()).thenReturn(7000000L); + { + LibvirtVMDef vm = new LibvirtVMDef(); + vm.addComp(new DevicesDef()); + libvirtComputingResourceSpy.createVbd(connect, to, name, vm); + + DiskDef rootDisk = vm.getDevices().getDisks().get(0); + assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType()); + assertEquals(DiskDef.DiskBus.IDE, rootDisk.getBusType()); + assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard()); + } + + // 4. test Bus: VIRTIO and "good" qemu version -> discard unmap + vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.VIRTIO.name()); + { + LibvirtVMDef vm = new LibvirtVMDef(); + vm.addComp(new DevicesDef()); + libvirtComputingResourceSpy.createVbd(connect, to, name, vm); + + DiskDef rootDisk = vm.getDevices().getDisks().get(0); + assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType()); + assertEquals(DiskDef.DiskBus.VIRTIO, rootDisk.getBusType()); + assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard()); + } + } } diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java index 6bb9752d764..3a5541654bb 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java @@ -454,6 +454,11 @@ public class MockAccountManager extends ManagerBase implements AccountManager { // TODO Auto-generated method stub } + @Override + public void validateAccountHasAccessToResource(Account account, AccessType accessType, Object resource) { + // TODO Auto-generated method stub + } + @Override public Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly) { // TODO Auto-generated method stub diff --git a/plugins/storage/object/ceph/src/main/java/org/apache/cloudstack/storage/datastore/driver/CephObjectStoreDriverImpl.java b/plugins/storage/object/ceph/src/main/java/org/apache/cloudstack/storage/datastore/driver/CephObjectStoreDriverImpl.java index 6fece40e6ac..b2e1d23917b 100644 --- a/plugins/storage/object/ceph/src/main/java/org/apache/cloudstack/storage/datastore/driver/CephObjectStoreDriverImpl.java +++ b/plugins/storage/object/ceph/src/main/java/org/apache/cloudstack/storage/datastore/driver/CephObjectStoreDriverImpl.java @@ -46,8 +46,6 @@ import org.apache.cloudstack.storage.datastore.db.ObjectStoreDetailsDao; import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO; import org.apache.cloudstack.storage.object.BaseObjectStoreDriverImpl; import org.apache.cloudstack.storage.object.BucketObject; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.twonote.rgwadmin4j.RgwAdmin; import org.twonote.rgwadmin4j.RgwAdminBuilder; import org.twonote.rgwadmin4j.model.BucketInfo; @@ -62,7 +60,6 @@ import java.util.Map; import java.util.HashMap; public class CephObjectStoreDriverImpl extends BaseObjectStoreDriverImpl { - private static final Logger s_logger = LogManager.getLogger(CephObjectStoreDriverImpl.class); @Inject AccountDao _accountDao; @@ -168,7 +165,7 @@ public class CephObjectStoreDriverImpl extends BaseObjectStoreDriverImpl { String policyConfig; if (policy.equalsIgnoreCase("public")) { - s_logger.debug("Setting public policy on bucket " + bucket.getName()); + logger.debug("Setting public policy on bucket " + bucket.getName()); StringBuilder builder = new StringBuilder(); builder.append("{\n"); builder.append(" \"Statement\": [\n"); @@ -192,7 +189,7 @@ public class CephObjectStoreDriverImpl extends BaseObjectStoreDriverImpl { builder.append("}\n"); policyConfig = builder.toString(); } else { - s_logger.debug("Setting private policy on bucket " + bucket.getName()); + logger.debug("Setting private policy on bucket " + bucket.getName()); policyConfig = "{\"Version\":\"2012-10-17\",\"Statement\":[]}"; } @@ -218,15 +215,15 @@ public class CephObjectStoreDriverImpl extends BaseObjectStoreDriverImpl { RgwAdmin rgwAdmin = getRgwAdminClient(storeId); String username = account.getUuid(); - s_logger.debug("Attempting to create Ceph RGW user for account " + account.getAccountName() + " with UUID " + username); + logger.debug("Attempting to create Ceph RGW user for account " + account.getAccountName() + " with UUID " + username); try { Optional user = rgwAdmin.getUserInfo(username); if (user.isPresent()) { - s_logger.info("User already exists in Ceph RGW: " + username); + logger.info("User already exists in Ceph RGW: " + username); return true; } } catch (Exception e) { - s_logger.debug("User does not exist. Creating user in Ceph RGW: " + username); + logger.debug("User does not exist. Creating user in Ceph RGW: " + username); } try { diff --git a/plugins/storage/object/ceph/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CephObjectStoreLifeCycleImpl.java b/plugins/storage/object/ceph/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CephObjectStoreLifeCycleImpl.java index a9b13bf338e..8740d188ce0 100644 --- a/plugins/storage/object/ceph/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CephObjectStoreLifeCycleImpl.java +++ b/plugins/storage/object/ceph/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CephObjectStoreLifeCycleImpl.java @@ -39,7 +39,7 @@ import java.util.Map; public class CephObjectStoreLifeCycleImpl implements ObjectStoreLifeCycle { - private static final Logger s_logger = LogManager.getLogger(CephObjectStoreLifeCycleImpl.class); + private Logger logger = LogManager.getLogger(CephObjectStoreLifeCycleImpl.class); @Inject ObjectStoreHelper objectStoreHelper; @@ -72,7 +72,7 @@ public class CephObjectStoreLifeCycleImpl implements ObjectStoreLifeCycle { objectStoreParameters.put("accesskey", accessKey); objectStoreParameters.put("secretkey", secretKey); - s_logger.info("Attempting to connect to Ceph RGW at " + url + " with access key " + accessKey); + logger.info("Attempting to connect to Ceph RGW at " + url + " with access key " + accessKey); RgwAdmin rgwAdmin = new RgwAdminBuilder() .accessKey(accessKey) @@ -81,10 +81,10 @@ public class CephObjectStoreLifeCycleImpl implements ObjectStoreLifeCycle { .build(); try { List buckets = rgwAdmin.listBucket(); - s_logger.debug("Found " + buckets + " buckets at Ceph RGW: " + url); - s_logger.info("Successfully connected to Ceph RGW: " + url); + logger.debug("Found " + buckets + " buckets at Ceph RGW: " + url); + logger.info("Successfully connected to Ceph RGW: " + url); } catch (Exception e) { - s_logger.debug("Error while initializing Ceph RGW Object Store: " + e.getMessage()); + logger.debug("Error while initializing Ceph RGW Object Store: " + e.getMessage()); throw new RuntimeException("Error while initializing Ceph RGW Object Store. Invalid credentials or URL"); } diff --git a/plugins/storage/volume/linstor/CHANGELOG.md b/plugins/storage/volume/linstor/CHANGELOG.md index 30f0225b45e..a0a53d20119 100644 --- a/plugins/storage/volume/linstor/CHANGELOG.md +++ b/plugins/storage/volume/linstor/CHANGELOG.md @@ -5,6 +5,19 @@ All notable changes to Linstor CloudStack plugin will be documented in this file The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2024-10-28] + +### Fixed + +- Disable discard="unmap" for ide devices and qemu < 7.0 + https://bugzilla.redhat.com/show_bug.cgi?id=2029980 + +## [2024-10-04] + +### Added + +- Enable qemu discard="unmap" for Linstor block disks + ## [2024-08-27] ### Changed diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java index fabe2ab3536..aca1037d2be 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java @@ -17,6 +17,7 @@ package com.cloud.hypervisor.kvm.storage; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -48,6 +49,7 @@ import com.linbit.linstor.api.model.ProviderKind; import com.linbit.linstor.api.model.Resource; import com.linbit.linstor.api.model.ResourceConnectionModify; import com.linbit.linstor.api.model.ResourceDefinition; +import com.linbit.linstor.api.model.ResourceDefinitionModify; import com.linbit.linstor.api.model.ResourceGroupSpawn; import com.linbit.linstor.api.model.ResourceMakeAvailable; import com.linbit.linstor.api.model.ResourceWithVolumes; @@ -235,6 +237,34 @@ public class LinstorStorageAdaptor implements StorageAdaptor { } } + private void setAllowTwoPrimariesOnRD(DevelopersApi api, String rscName) throws ApiException { + ResourceDefinitionModify rdm = new ResourceDefinitionModify(); + Properties props = new Properties(); + props.put("DrbdOptions/Net/allow-two-primaries", "yes"); + props.put("DrbdOptions/Net/protocol", "C"); + rdm.setOverrideProps(props); + ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm); + if (answers.hasError()) { + logger.error(String.format("Unable to set protocol C and 'allow-two-primaries' on %s", rscName)); + // do not fail here as adding allow-two-primaries property is only a problem while live migrating + } + } + + private void setAllowTwoPrimariesOnRc(DevelopersApi api, String rscName, String inUseNode) throws ApiException { + ResourceConnectionModify rcm = new ResourceConnectionModify(); + Properties props = new Properties(); + props.put("DrbdOptions/Net/allow-two-primaries", "yes"); + props.put("DrbdOptions/Net/protocol", "C"); + rcm.setOverrideProps(props); + ApiCallRcList answers = api.resourceConnectionModify(rscName, inUseNode, localNodeName, rcm); + if (answers.hasError()) { + logger.error(String.format( + "Unable to set protocol C and 'allow-two-primaries' on %s/%s/%s", + inUseNode, localNodeName, rscName)); + // do not fail here as adding allow-two-primaries property is only a problem while live migrating + } + } + /** * Checks if the given resource is in use by drbd on any host and * if so set the drbd option allow-two-primaries @@ -246,16 +276,13 @@ public class LinstorStorageAdaptor implements StorageAdaptor { String inUseNode = LinstorUtil.isResourceInUse(api, rscName); if (inUseNode != null && !inUseNode.equalsIgnoreCase(localNodeName)) { // allow 2 primaries for live migration, should be removed by disconnect on the other end - ResourceConnectionModify rcm = new ResourceConnectionModify(); - Properties props = new Properties(); - props.put("DrbdOptions/Net/allow-two-primaries", "yes"); - props.put("DrbdOptions/Net/protocol", "C"); - rcm.setOverrideProps(props); - ApiCallRcList answers = api.resourceConnectionModify(rscName, inUseNode, localNodeName, rcm); - if (answers.hasError()) { - logger.error("Unable to set protocol C and 'allow-two-primaries' on {}/{}/{}", - inUseNode, localNodeName, rscName); - // do not fail here as adding allow-two-primaries property is only a problem while live migrating + + // if non hyperconverged setup, we have to set allow-two-primaries on the resource-definition + // as there is no resource connection between diskless nodes. + if (LinstorUtil.areResourcesDiskless(api, rscName, Arrays.asList(inUseNode, localNodeName))) { + setAllowTwoPrimariesOnRD(api, rscName); + } else { + setAllowTwoPrimariesOnRc(api, rscName, inUseNode); } } } @@ -294,11 +321,22 @@ public class LinstorStorageAdaptor implements StorageAdaptor { return true; } - private void removeTwoPrimariesRcProps(DevelopersApi api, String inUseNode, String rscName) throws ApiException { + private void removeTwoPrimariesRDProps(DevelopersApi api, String rscName, List deleteProps) + throws ApiException { + ResourceDefinitionModify rdm = new ResourceDefinitionModify(); + rdm.deleteProps(deleteProps); + ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm); + if (answers.hasError()) { + logger.error( + String.format("Failed to remove 'protocol' and 'allow-two-primaries' on %s: %s", + rscName, LinstorUtil.getBestErrorMessage(answers))); + // do not fail here as removing allow-two-primaries property isn't fatal + } + } + + private void removeTwoPrimariesRcProps(DevelopersApi api, String rscName, String inUseNode, List deleteProps) + throws ApiException { ResourceConnectionModify rcm = new ResourceConnectionModify(); - List deleteProps = new ArrayList<>(); - deleteProps.add("DrbdOptions/Net/allow-two-primaries"); - deleteProps.add("DrbdOptions/Net/protocol"); rcm.deleteProps(deleteProps); ApiCallRcList answers = api.resourceConnectionModify(rscName, localNodeName, inUseNode, rcm); if (answers.hasError()) { @@ -310,6 +348,15 @@ public class LinstorStorageAdaptor implements StorageAdaptor { } } + private void removeTwoPrimariesProps(DevelopersApi api, String inUseNode, String rscName) throws ApiException { + List deleteProps = new ArrayList<>(); + deleteProps.add("DrbdOptions/Net/allow-two-primaries"); + deleteProps.add("DrbdOptions/Net/protocol"); + + removeTwoPrimariesRDProps(api, rscName, deleteProps); + removeTwoPrimariesRcProps(api, rscName, inUseNode, deleteProps); + } + private boolean tryDisconnectLinstor(String volumePath, KVMStoragePool pool) { if (volumePath == null) { @@ -343,7 +390,7 @@ public class LinstorStorageAdaptor implements StorageAdaptor { try { String inUseNode = LinstorUtil.isResourceInUse(api, rsc.getName()); if (inUseNode != null && !inUseNode.equalsIgnoreCase(localNodeName)) { - removeTwoPrimariesRcProps(api, inUseNode, rsc.getName()); + removeTwoPrimariesProps(api, inUseNode, rsc.getName()); } } catch (ApiException apiEx) { logger.error(apiEx.getBestMessage()); diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java index d3a8b97cf2a..cbdc9940b80 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.storage.datastore.util; import com.linbit.linstor.api.ApiClient; +import com.linbit.linstor.api.ApiConsts; import com.linbit.linstor.api.ApiException; import com.linbit.linstor.api.DevelopersApi; import com.linbit.linstor.api.model.ApiCallRc; @@ -33,6 +34,7 @@ import com.linbit.linstor.api.model.Volume; import javax.annotation.Nonnull; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -210,6 +212,28 @@ public class LinstorUtil { return null; } + /** + * Check if the given resources are diskless. + * + * @param api developer api object to use + * @param rscName resource name to check in use state. + * @return NodeName where the resource is inUse, if not in use `null` + * @throws ApiException forwards api errors + */ + public static boolean areResourcesDiskless(DevelopersApi api, String rscName, Collection nodeNames) + throws ApiException { + List rscs = api.resourceList(rscName, null, null); + if (rscs != null) { + Collection disklessNodes = rscs.stream() + .filter(rsc -> rsc.getFlags() != null && (rsc.getFlags().contains(ApiConsts.FLAG_DISKLESS) || + rsc.getFlags().contains(ApiConsts.FLAG_DRBD_DISKLESS))) + .map(rsc -> rsc.getNodeName().toLowerCase()) + .collect(Collectors.toList()); + return disklessNodes.containsAll(nodeNames.stream().map(String::toLowerCase).collect(Collectors.toList())); + } + return false; + } + /** * Try to get the device path for the given resource name. * This could be made a bit more direct after java-linstor api is fixed for layer data subtypes. diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java index 97f4e2fe155..dcb190a573b 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java @@ -138,6 +138,8 @@ public class StorPoolUtil { public static final String SP_TIER = "SP_QOSCLASS"; + public static final String OBJECT_DOES_NOT_EXIST = "objectDoesNotExist"; + public static enum StorpoolRights { RO("ro"), RW("rw"), DETACH("detach"); @@ -458,7 +460,7 @@ public class StorPoolUtil { } private static boolean objectExists(SpApiError err) { - if (!err.getName().equals("objectDoesNotExist")) { + if (!err.getName().equals(OBJECT_DOES_NOT_EXIST)) { throw new CloudRuntimeException(err.getDescr()); } return false; diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java index 5cdb7b8cda1..c7bcc8a46b7 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java @@ -16,10 +16,19 @@ // under the License. package org.apache.cloudstack.storage.snapshot; -import java.util.ArrayList; -import java.util.List; - -import javax.inject.Inject; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.hypervisor.kvm.storage.StorPoolStorageAdaptor; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.Snapshot; +import com.cloud.storage.SnapshotVO; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.SnapshotDetailsDao; +import com.cloud.storage.dao.SnapshotDetailsVO; +import com.cloud.storage.dao.SnapshotZoneDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.fsm.NoTransitionException; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; @@ -40,23 +49,13 @@ import org.apache.cloudstack.storage.datastore.util.StorPoolUtil; import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpApiResponse; import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpConnectionDesc; import org.apache.commons.collections.CollectionUtils; -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.exception.InvalidParameterValueException; -import com.cloud.hypervisor.kvm.storage.StorPoolStorageAdaptor; -import com.cloud.storage.DataStoreRole; -import com.cloud.storage.Snapshot; -import com.cloud.storage.SnapshotVO; -import com.cloud.storage.VolumeVO; -import com.cloud.storage.dao.SnapshotDao; -import com.cloud.storage.dao.SnapshotDetailsDao; -import com.cloud.storage.dao.SnapshotDetailsVO; -import com.cloud.storage.dao.SnapshotZoneDao; -import com.cloud.storage.dao.VolumeDao; -import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.fsm.NoTransitionException; +import javax.inject.Inject; +import java.util.ArrayList; +import java.util.List; @Component @@ -117,10 +116,11 @@ public class StorPoolSnapshotStrategy implements SnapshotStrategy { if (resp.getError() != null) { final String err = String.format("Failed to clean-up Storpool snapshot %s. Error: %s", name, resp.getError()); StorPoolUtil.spLog(err); - markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId, resp); + markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId, resp.getError().getName().equals(StorPoolUtil.OBJECT_DOES_NOT_EXIST)); throw new CloudRuntimeException(err); } else { res = deleteSnapshotFromDbIfNeeded(snapshotVO, zoneId); + markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId,true); StorPoolUtil.spLog("StorpoolSnapshotStrategy.deleteSnapshot: executed successfully=%s, snapshot uuid=%s, name=%s", res, snapshotVO.getUuid(), name); } } catch (Exception e) { @@ -129,15 +129,23 @@ public class StorPoolSnapshotStrategy implements SnapshotStrategy { } } + List snapshots = _snapshotStoreDao.listBySnapshotIdAndState(snapshotId, State.Ready); + if (res || CollectionUtils.isEmpty(snapshots)) { + updateSnapshotToDestroyed(snapshotVO); + return true; + } return res; } - private void markSnapshotAsDestroyedIfAlreadyRemoved(Long snapshotId, SpApiResponse resp) { - if (resp.getError().getName().equals("objectDoesNotExist")) { - SnapshotDataStoreVO snapshotOnPrimary = _snapshotStoreDao.findBySourceSnapshot(snapshotId, DataStoreRole.Primary); - if (snapshotOnPrimary != null) { - snapshotOnPrimary.setState(State.Destroyed); - _snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary); + private void markSnapshotAsDestroyedIfAlreadyRemoved(Long snapshotId, boolean isSnapshotDeleted) { + if (!isSnapshotDeleted) { + return; + } + List snapshotsOnStore = _snapshotStoreDao.listBySnapshotIdAndState(snapshotId, State.Ready); + for (SnapshotDataStoreVO snapshot : snapshotsOnStore) { + if (snapshot.getInstallPath() != null && snapshot.getInstallPath().contains(StorPoolUtil.SP_DEV_PATH)) { + snapshot.setState(State.Destroyed); + _snapshotStoreDao.update(snapshot.getId(), snapshot); } } } diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java index 3e6b093abe1..4851973c2ad 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java @@ -47,6 +47,7 @@ import org.apache.cloudstack.api.response.UserResponse; import org.apache.cloudstack.saml.SAML2AuthManager; import org.apache.cloudstack.saml.SAMLUtils; +import com.cloud.api.ApiServer; import com.cloud.api.response.ApiResponseSerializer; import com.cloud.domain.Domain; import com.cloud.domain.dao.DomainDao; @@ -59,6 +60,8 @@ import com.cloud.user.dao.UserAccountDao; import com.cloud.user.dao.UserDao; import com.cloud.utils.HttpUtils; +import org.apache.commons.lang3.EnumUtils; + @APICommand(name = "listAndSwitchSamlAccount", description = "Lists and switches to other SAML accounts owned by the SAML user", responseObject = SuccessResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthenticator { @@ -102,7 +105,9 @@ public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthentic params, responseType)); } - if (!HttpUtils.validateSessionKey(session, params, req.getCookies(), ApiConstants.SESSIONKEY)) { + HttpUtils.ApiSessionKeyCheckOption sessionKeyCheckOption = EnumUtils.getEnumIgnoreCase(HttpUtils.ApiSessionKeyCheckOption.class, + ApiServer.ApiSessionKeyCheckLocations.value(), HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter); + if (!HttpUtils.validateSessionKey(session, params, req.getCookies(), ApiConstants.SESSIONKEY, sessionKeyCheckOption)) { throw new ServerApiException(ApiErrorCode.UNAUTHORIZED, _apiServer.getSerializedApiError(ApiErrorCode.UNAUTHORIZED.getHttpCode(), "Unauthorized session, please re-login", params, responseType)); diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java index 3a4030f9c0d..4e8ba16c739 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java @@ -73,6 +73,12 @@ public interface SAML2AuthManager extends PluggableAPIAuthenticator, PluggableSe ConfigKey SAMLCheckSignature = new ConfigKey("Advanced", Boolean.class, "saml2.check.signature", "true", "When enabled (default and recommended), SAML2 signature checks are enforced and lack of signature in the SAML SSO response will cause login exception. Disabling this is not advisable but provided for backward compatibility for users who are able to accept the risks.", false); + ConfigKey SAMLForceAuthn = new ConfigKey("Advanced", Boolean.class, "saml2.force.authn", "false", + "When enabled (default false), SAML2 will force a new authentication. This can be useful if multiple application use different saml logins from the same application (I.E. browser)", true); + + ConfigKey SAMLUserSessionKeyPathAttribute = new ConfigKey("Advanced", String.class, "saml2.user.sessionkey.path", "", + "The Path attribute of sessionkey cookie when SAML users have logged in. If not set, it will be set to the path of SAML redirection URL (saml2.redirect.url).", true); + SAMLProviderMetadata getSPMetadata(); SAMLProviderMetadata getIdPMetadata(String entityId); Collection getAllIdPMetadata(); diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java index 230c53ac4a9..545b01a4a31 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java @@ -540,6 +540,7 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage SAMLServiceProviderSingleSignOnURL, SAMLServiceProviderSingleLogOutURL, SAMLCloudStackRedirectionUrl, SAMLUserAttributeName, SAMLIdentityProviderMetadataURL, SAMLDefaultIdentityProviderId, - SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature}; + SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature, + SAMLForceAuthn, SAMLUserSessionKeyPathAttribute}; } } diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java index 7ffe07a8609..54f6e84fe36 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java @@ -25,6 +25,8 @@ import java.io.IOException; import java.io.StringWriter; import java.io.UnsupportedEncodingException; import java.math.BigInteger; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URLEncoder; import java.nio.charset.Charset; import java.security.InvalidKeyException; @@ -102,7 +104,9 @@ import org.w3c.dom.Document; import org.w3c.dom.Element; import org.xml.sax.SAXException; +import com.cloud.api.ApiServlet; import com.cloud.utils.HttpUtils; +import com.cloud.utils.exception.CloudRuntimeException; public class SAMLUtils { protected static Logger LOGGER = LogManager.getLogger(SAMLUtils.class); @@ -190,7 +194,7 @@ public class SAMLUtils { authnRequest.setID(authnId); authnRequest.setDestination(idpUrl); authnRequest.setVersion(SAMLVersion.VERSION_20); - authnRequest.setForceAuthn(false); + authnRequest.setForceAuthn(SAML2AuthManager.SAMLForceAuthn.value()); authnRequest.setIsPassive(false); authnRequest.setIssueInstant(new DateTime()); authnRequest.setProtocolBinding(SAMLConstants.SAML2_POST_BINDING_URI); @@ -297,7 +301,26 @@ public class SAMLUtils { resp.addCookie(new Cookie("timezone", URLEncoder.encode(timezone, HttpUtils.UTF_8))); } resp.addCookie(new Cookie("userfullname", URLEncoder.encode(loginResponse.getFirstName() + " " + loginResponse.getLastName(), HttpUtils.UTF_8).replace("+", "%20"))); - resp.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly;Path=/client/api", ApiConstants.SESSIONKEY, loginResponse.getSessionKey())); + + String redirectUrl = SAML2AuthManager.SAMLCloudStackRedirectionUrl.value(); + String path = SAML2AuthManager.SAMLUserSessionKeyPathAttribute.value(); + String domain = null; + try { + URI redirectUri = new URI(redirectUrl); + domain = redirectUri.getHost(); + if (StringUtils.isBlank(path)) { + path = redirectUri.getPath(); + } + if (StringUtils.isBlank(path)) { + path = "/"; + } + } catch (URISyntaxException ex) { + throw new CloudRuntimeException("Invalid URI: " + redirectUrl); + } + String sameSite = ApiServlet.getApiSessionKeySameSite(); + String sessionKeyCookie = String.format("%s=%s;Domain=%s;Path=%s;%s", ApiConstants.SESSIONKEY, loginResponse.getSessionKey(), domain, path, sameSite); + LOGGER.debug("Adding sessionkey cookie to response: " + sessionKeyCookie); + resp.addHeader("SET-COOKIE", sessionKeyCookie); } /** diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java index 9342a0c7f09..9225574ff60 100644 --- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java +++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java @@ -27,6 +27,7 @@ import java.net.InetAddress; import java.util.HashMap; import java.util.Map; +import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; @@ -88,6 +89,9 @@ public class ListAndSwitchSAMLAccountCmdTest extends TestCase { @Mock HttpServletRequest req; + final String sessionId = "node0xxxxxxxxxxxxx"; + Cookie[] cookies; + @Test public void testListAndSwitchSAMLAccountCmd() throws Exception { // Setup @@ -95,6 +99,7 @@ public class ListAndSwitchSAMLAccountCmdTest extends TestCase { final String sessionKeyValue = "someSessionIDValue"; Mockito.when(session.getAttribute(ApiConstants.SESSIONKEY)).thenReturn(sessionKeyValue); Mockito.when(session.getAttribute("userid")).thenReturn(2L); + Mockito.when(session.getId()).thenReturn(sessionId); params.put(ApiConstants.USER_ID, new String[]{"2"}); params.put(ApiConstants.DOMAIN_ID, new String[]{"1"}); Mockito.when(userDao.findByUuid(anyString())).thenReturn(new UserVO(2L)); @@ -146,7 +151,25 @@ public class ListAndSwitchSAMLAccountCmdTest extends TestCase { Mockito.verify(accountService, Mockito.times(0)).getUserAccountById(Mockito.anyLong()); } - // valid sessionkey value test + // valid sessionkey value and invalid JSESSIONID test + cookies = new Cookie[2]; + cookies[0] = new Cookie(ApiConstants.SESSIONKEY, sessionKeyValue); + cookies[1] = new Cookie("JSESSIONID", "invalid-JSESSIONID"); + Mockito.when(req.getCookies()).thenReturn(cookies); + params.put(ApiConstants.SESSIONKEY, new String[]{sessionKeyValue}); + try { + cmd.authenticate("command", params, session, null, HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp); + } catch (ServerApiException exception) { + assertEquals(exception.getErrorCode(), ApiErrorCode.UNAUTHORIZED); + } finally { + Mockito.verify(accountService, Mockito.times(0)).getUserAccountById(Mockito.anyLong()); + } + + // valid sessionkey value and valid JSESSIONID test + cookies = new Cookie[2]; + cookies[0] = new Cookie(ApiConstants.SESSIONKEY, sessionKeyValue); + cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0"); + Mockito.when(req.getCookies()).thenReturn(cookies); params.put(ApiConstants.SESSIONKEY, new String[]{sessionKeyValue}); try { cmd.authenticate("command", params, session, null, HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp); diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 739ad765afa..72e97c3a6ee 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -33,6 +33,7 @@ import java.text.ParseException; import java.util.ArrayList; import java.util.Collections; import java.util.Date; +import java.util.EnumSet; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; @@ -47,6 +48,7 @@ import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; @@ -169,6 +171,8 @@ import com.cloud.storage.VolumeApiService; import com.cloud.utils.ConstantTimeComparator; import com.cloud.utils.DateUtil; import com.cloud.utils.HttpUtils; +import com.cloud.utils.HttpUtils.ApiSessionKeySameSite; +import com.cloud.utils.HttpUtils.ApiSessionKeyCheckOption; import com.cloud.utils.Pair; import com.cloud.utils.ReflectUtil; import com.cloud.utils.StringUtils; @@ -310,6 +314,24 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer , true , ConfigKey.Scope.Global); + static final ConfigKey ApiSessionKeyCookieSameSiteSetting = new ConfigKey<>(String.class + , "api.sessionkey.cookie.samesite" + , ConfigKey.CATEGORY_ADVANCED + , ApiSessionKeySameSite.Lax.name() + , "The SameSite attribute of cookie 'sessionkey'. Valid options are: Lax (default), Strict, NoneAndSecure and Null." + , true + , ConfigKey.Scope.Global, null, null, null, null, null, ConfigKey.Kind.Select, + EnumSet.allOf(ApiSessionKeySameSite.class).stream().map(Enum::toString).collect(Collectors.joining(", "))); + + public static final ConfigKey ApiSessionKeyCheckLocations = new ConfigKey<>(String.class + , "api.sessionkey.check.locations" + , ConfigKey.CATEGORY_ADVANCED + , ApiSessionKeyCheckOption.CookieAndParameter.name() + , "The locations of 'sessionkey' during the validation of the API requests. Valid options are: CookieOrParameter, ParameterOnly, CookieAndParameter (default)." + , true + , ConfigKey.Scope.Global, null, null, null, null, null, ConfigKey.Kind.Select, + EnumSet.allOf(ApiSessionKeyCheckOption.class).stream().map(Enum::toString).collect(Collectors.joining(", "))); + @Override public boolean configure(final String name, final Map params) throws ConfigurationException { messageBus.subscribe(AsyncJob.Topics.JOB_EVENT_PUBLISH, MessageDispatcher.getDispatcher(this)); @@ -1582,7 +1604,9 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer JSONDefaultContentType, proxyForwardList, useForwardHeader, - listOfForwardHeaders + listOfForwardHeaders, + ApiSessionKeyCookieSameSiteSetting, + ApiSessionKeyCheckLocations }; } } diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java b/server/src/main/java/com/cloud/api/ApiServlet.java index f2b5d3c4797..e2ff411f8f4 100644 --- a/server/src/main/java/com/cloud/api/ApiServlet.java +++ b/server/src/main/java/com/cloud/api/ApiServlet.java @@ -47,8 +47,10 @@ import org.apache.cloudstack.api.command.user.consoleproxy.CreateConsoleEndpoint import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.managed.context.ManagedContext; import org.apache.cloudstack.utils.consoleproxy.ConsoleAccessUtils; + import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.commons.lang3.EnumUtils; import org.jetbrains.annotations.Nullable; import org.springframework.stereotype.Component; import org.springframework.web.context.support.SpringBeanAutowiringSupport; @@ -65,6 +67,8 @@ import com.cloud.user.User; import com.cloud.user.UserAccount; import com.cloud.utils.HttpUtils; +import com.cloud.utils.HttpUtils.ApiSessionKeySameSite; +import com.cloud.utils.HttpUtils.ApiSessionKeyCheckOption; import com.cloud.utils.StringUtils; import com.cloud.utils.db.EntityManager; import com.cloud.utils.net.NetUtils; @@ -255,7 +259,8 @@ public class ApiServlet extends HttpServlet { } responseString = apiAuthenticator.authenticate(command, params, session, remoteAddress, responseType, auditTrailSb, req, resp); if (session != null && session.getAttribute(ApiConstants.SESSIONKEY) != null) { - resp.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly", ApiConstants.SESSIONKEY, session.getAttribute(ApiConstants.SESSIONKEY))); + String sameSite = getApiSessionKeySameSite(); + resp.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly;%s", ApiConstants.SESSIONKEY, session.getAttribute(ApiConstants.SESSIONKEY), sameSite)); } } catch (ServerApiException e) { httpResponseCode = e.getErrorCode().getHttpCode(); @@ -264,19 +269,22 @@ public class ApiServlet extends HttpServlet { } if (apiAuthenticator.getAPIType() == APIAuthenticationType.LOGOUT_API) { - if (session != null) { - final Long userId = (Long) session.getAttribute("userid"); - final Account account = (Account) session.getAttribute("accountobj"); - Long accountId = null; - if (account != null) { - accountId = account.getId(); - } - auditTrailSb.insert(0, "(userId=" + userId + " accountId=" + accountId + " sessionId=" + session.getId() + ")"); - if (userId != null) { - apiServer.logoutUser(userId); - } - invalidateHttpSession(session, "invalidating session after logout call"); + if (session == null) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Session not found for the logout process."); } + + final Long userId = (Long) session.getAttribute("userid"); + final Account account = (Account) session.getAttribute("accountobj"); + Long accountId = null; + if (account != null) { + accountId = account.getId(); + } + auditTrailSb.insert(0, "(userId=" + userId + " accountId=" + accountId + " sessionId=" + session.getId() + ")"); + if (userId != null) { + apiServer.logoutUser(userId); + } + invalidateHttpSession(session, "invalidating session after logout call"); + final Cookie[] cookies = req.getCookies(); if (cookies != null) { for (final Cookie cookie : cookies) { @@ -375,6 +383,22 @@ public class ApiServlet extends HttpServlet { } } + public static String getApiSessionKeySameSite() { + ApiSessionKeySameSite sameSite = EnumUtils.getEnumIgnoreCase(ApiSessionKeySameSite.class, + ApiServer.ApiSessionKeyCookieSameSiteSetting.value(), ApiSessionKeySameSite.Lax); + switch (sameSite) { + case Strict: + return "SameSite=Strict"; + case NoneAndSecure: + return "SameSite=None;Secure"; + case Null: + return ""; + case Lax: + default: + return "SameSite=Lax"; + } + } + private boolean checkIfAuthenticatorIsOf2FA(String command) { boolean verify2FA = false; APIAuthenticator apiAuthenticator = authManager.getAPIAuthenticator(command); @@ -510,7 +534,9 @@ public class ApiServlet extends HttpServlet { } private boolean invalidateHttpSessionIfNeeded(HttpServletRequest req, HttpServletResponse resp, StringBuilder auditTrailSb, String responseType, Map params, HttpSession session, String account) { - if (!HttpUtils.validateSessionKey(session, params, req.getCookies(), ApiConstants.SESSIONKEY)) { + ApiSessionKeyCheckOption sessionKeyCheckOption = EnumUtils.getEnumIgnoreCase(ApiSessionKeyCheckOption.class, + ApiServer.ApiSessionKeyCheckLocations.value(), ApiSessionKeyCheckOption.CookieAndParameter); + if (!HttpUtils.validateSessionKey(session, params, req.getCookies(), ApiConstants.SESSIONKEY, sessionKeyCheckOption)) { String msg = String.format("invalidating session %s for account %s", session.getId(), account); invalidateHttpSession(session, msg); auditTrailSb.append(" " + HttpServletResponse.SC_UNAUTHORIZED + " " + "unable to verify user credentials"); diff --git a/server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java b/server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java index 314b83acdb5..c4145755b56 100644 --- a/server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java +++ b/server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java @@ -32,8 +32,6 @@ import java.util.regex.Matcher; import javax.inject.Inject; -import org.apache.cloudstack.acl.ControlledEntity; -import org.apache.cloudstack.acl.InfrastructureEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.ACL; @@ -312,7 +310,7 @@ public class ParamProcessWorker implements DispatchWorker { doAccessChecks(cmd, entitiesToAccess); } - private void doAccessChecks(BaseCmd cmd, Map entitiesToAccess) { + protected void doAccessChecks(BaseCmd cmd, Map entitiesToAccess) { Account caller = CallContext.current().getCallingAccount(); List entityOwners = cmd.getEntityOwnerIds(); Account[] owners = null; @@ -334,19 +332,35 @@ public class ParamProcessWorker implements DispatchWorker { _accountMgr.checkAccess(caller, null, false, owners); } - if (!entitiesToAccess.isEmpty()) { - // check that caller can access the owner account. - _accountMgr.checkAccess(caller, null, false, owners); - for (Map.Entryentry : entitiesToAccess.entrySet()) { - Object entity = entry.getKey(); - if (entity instanceof ControlledEntity) { - _accountMgr.checkAccess(caller, entry.getValue(), true, (ControlledEntity) entity); - } else if (entity instanceof InfrastructureEntity) { - // FIXME: Move this code in adapter, remove code from - // Account manager - } + checkCallerAccessToEntities(caller, owners, entitiesToAccess); + } + + protected Account[] getEntityOwners(BaseCmd cmd) { + List entityOwners = cmd.getEntityOwnerIds(); + if (entityOwners != null) { + return entityOwners.stream().map(id -> _accountMgr.getAccount(id)).toArray(Account[]::new); + } + + if (cmd.getEntityOwnerId() == Account.ACCOUNT_ID_SYSTEM && cmd instanceof BaseAsyncCmd && cmd.getApiResourceType() == ApiCommandResourceType.Network) { + logger.debug("Skipping access check on the network owner if the owner is ROOT/system."); + } else { + Account owner = _accountMgr.getAccount(cmd.getEntityOwnerId()); + if (owner != null) { + return new Account[]{owner}; } } + return new Account[]{}; + } + + protected void checkCallerAccessToEntities(Account caller, Account[] owners, Map entitiesToAccess) { + if (entitiesToAccess.isEmpty()) { + return; + } + _accountMgr.checkAccess(caller, null, false, owners); + for (Map.Entry entry : entitiesToAccess.entrySet()) { + Object entity = entry.getKey(); + _accountMgr.validateAccountHasAccessToResource(caller, entry.getValue(), entity); + } } @SuppressWarnings({"unchecked", "rawtypes"}) 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 4063cfe7a18..046c3c1e6bc 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -245,6 +245,8 @@ import com.cloud.network.VpcVirtualNetworkApplianceService; import com.cloud.network.as.AutoScaleVmGroupVmMapVO; import com.cloud.network.as.dao.AutoScaleVmGroupDao; import com.cloud.network.as.dao.AutoScaleVmGroupVmMapDao; +import com.cloud.network.dao.IPAddressDao; +import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; import com.cloud.network.dao.PublicIpQuarantineDao; @@ -551,6 +553,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q @Inject private NetworkDao networkDao; + @Inject + private IPAddressDao ipAddressDao; + @Inject private NicDao nicDao; @@ -1461,6 +1466,22 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q if (isRootAdmin) { userVmSearchBuilder.or("keywordInstanceName", userVmSearchBuilder.entity().getInstanceName(), Op.LIKE ); } + + SearchBuilder ipAddressSearch = ipAddressDao.createSearchBuilder(); + userVmSearchBuilder.join("ipAddressSearch", ipAddressSearch, + ipAddressSearch.entity().getAssociatedWithVmId(), userVmSearchBuilder.entity().getId(), JoinBuilder.JoinType.LEFT); + + SearchBuilder nicSearch = nicDao.createSearchBuilder(); + userVmSearchBuilder.join("nicSearch", nicSearch, JoinBuilder.JoinType.LEFT, + JoinBuilder.JoinCondition.AND, + nicSearch.entity().getInstanceId(), userVmSearchBuilder.entity().getId(), + nicSearch.entity().getRemoved(), userVmSearchBuilder.entity().setLong(null)); + + userVmSearchBuilder.or("ipAddressSearch", "keywordPublicIpAddress", ipAddressSearch.entity().getAddress(), Op.LIKE); + + userVmSearchBuilder.or("nicSearch", "keywordIpAddress", nicSearch.entity().getIPv4Address(), Op.LIKE); + userVmSearchBuilder.or("nicSearch", "keywordIp6Address", nicSearch.entity().getIPv6Address(), Op.LIKE); + userVmSearchBuilder.cp(); } @@ -1554,6 +1575,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q userVmSearchCriteria.setParameters("keywordDisplayName", keywordMatch); userVmSearchCriteria.setParameters("keywordName", keywordMatch); userVmSearchCriteria.setParameters("keywordState", keyword); + userVmSearchCriteria.setParameters("keywordIpAddress", keywordMatch); + userVmSearchCriteria.setParameters("keywordPublicIpAddress", keywordMatch); + userVmSearchCriteria.setParameters("keywordIp6Address", keywordMatch); if (isRootAdmin) { userVmSearchCriteria.setParameters("keywordInstanceName", keywordMatch); } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 78234497cd0..39e8518f760 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -43,6 +43,7 @@ import javax.naming.ConfigurationException; import org.apache.cloudstack.acl.APIChecker; import org.apache.cloudstack.acl.ControlledEntity; +import org.apache.cloudstack.acl.InfrastructureEntity; import org.apache.cloudstack.acl.QuerySelector; import org.apache.cloudstack.acl.Role; import org.apache.cloudstack.acl.RoleService; @@ -740,6 +741,19 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } + @Override + public void validateAccountHasAccessToResource(Account account, AccessType accessType, Object resource) { + Class resourceClass = resource.getClass(); + if (ControlledEntity.class.isAssignableFrom(resourceClass)) { + checkAccess(account, accessType, true, (ControlledEntity) resource); + } else if (Domain.class.isAssignableFrom(resourceClass)) { + checkAccess(account, (Domain) resource); + } else if (InfrastructureEntity.class.isAssignableFrom(resourceClass)) { + logger.trace("Validation of access to infrastructure entity has been disabled in CloudStack version 4.4."); + } + logger.debug(String.format("Account [%s] has access to resource.", account.getUuid())); + } + @Override public Long checkAccessAndSpecifyAuthority(Account caller, Long zoneId) { // We just care for resource domain admins for now, and they should be permitted to see only their zone. diff --git a/server/src/test/java/com/cloud/api/dispatch/ParamProcessWorkerTest.java b/server/src/test/java/com/cloud/api/dispatch/ParamProcessWorkerTest.java index da70bc1c1bf..a998d2e9ab4 100644 --- a/server/src/test/java/com/cloud/api/dispatch/ParamProcessWorkerTest.java +++ b/server/src/test/java/com/cloud/api/dispatch/ParamProcessWorkerTest.java @@ -18,6 +18,30 @@ */ package com.cloud.api.dispatch; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.InjectMocks; +import org.mockito.Spy; + +import org.mockito.junit.MockitoJUnitRunner; + +import org.apache.cloudstack.api.ApiArgValidator; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.user.address.AssociateIPAddrCmd; +import org.apache.cloudstack.acl.SecurityChecker; +import org.apache.cloudstack.context.CallContext; + import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.NetworkRuleConflictException; @@ -26,29 +50,33 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.User; -import org.apache.cloudstack.api.ApiArgValidator; -import org.apache.cloudstack.api.BaseCmd; -import org.apache.cloudstack.api.Parameter; -import org.apache.cloudstack.api.ServerApiException; -import org.apache.cloudstack.context.CallContext; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; - -import java.util.HashMap; +import com.cloud.vm.VMInstanceVO; @RunWith(MockitoJUnitRunner.class) public class ParamProcessWorkerTest { - @Mock - protected AccountManager accountManager; + @Spy + @InjectMocks + private ParamProcessWorker paramProcessWorkerSpy; - protected ParamProcessWorker paramProcessWorker; + @Mock + private AccountManager accountManagerMock; + + @Mock + private Account callingAccountMock; + + @Mock + private User callingUserMock; + + @Mock + private Account ownerAccountMock; + + @Mock + BaseCmd baseCmdMock; + + private Account[] owners = new Account[]{ownerAccountMock}; + + private Map entities = new HashMap<>(); public static class TestCmd extends BaseCmd { @@ -69,7 +97,7 @@ public class ParamProcessWorkerTest { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, - ResourceAllocationException, NetworkRuleConflictException { + ResourceAllocationException, NetworkRuleConflictException { // well documented nothing } @@ -87,9 +115,7 @@ public class ParamProcessWorkerTest { @Before public void setup() { - CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); - paramProcessWorker = new ParamProcessWorker(); - paramProcessWorker._accountMgr = accountManager; + CallContext.register(callingUserMock, callingAccountMock); } @After @@ -106,7 +132,7 @@ public class ParamProcessWorkerTest { params.put("doubleparam1", "11.89"); params.put("vmHostNameParam", "test-host-name-123"); final TestCmd cmd = new TestCmd(); - paramProcessWorker.processParameters(cmd, params); + paramProcessWorkerSpy.processParameters(cmd, params); Assert.assertEquals("foo", cmd.strparam1); Assert.assertEquals(100, cmd.intparam1); Assert.assertTrue(Double.compare(cmd.doubleparam1, 11.89) == 0); @@ -118,7 +144,7 @@ public class ParamProcessWorkerTest { final HashMap params = new HashMap(); params.put("vmHostNameParam", "123test"); final TestCmd cmd = new TestCmd(); - paramProcessWorker.processParameters(cmd, params); + paramProcessWorkerSpy.processParameters(cmd, params); } @Test(expected = ServerApiException.class) @@ -126,7 +152,7 @@ public class ParamProcessWorkerTest { final HashMap params = new HashMap(); params.put("vmHostNameParam", "-test"); final TestCmd cmd = new TestCmd(); - paramProcessWorker.processParameters(cmd, params); + paramProcessWorkerSpy.processParameters(cmd, params); } @Test(expected = ServerApiException.class) @@ -134,7 +160,7 @@ public class ParamProcessWorkerTest { final HashMap params = new HashMap(); params.put("vmHostNameParam", "test-"); final TestCmd cmd = new TestCmd(); - paramProcessWorker.processParameters(cmd, params); + paramProcessWorkerSpy.processParameters(cmd, params); } @Test(expected = ServerApiException.class) @@ -142,6 +168,68 @@ public class ParamProcessWorkerTest { final HashMap params = new HashMap(); params.put("vmHostNameParam", "test-f2405112-d5a1-47c1-9f00-976909e3a6d3-1e6f3264-955ee76011a99"); final TestCmd cmd = new TestCmd(); - paramProcessWorker.processParameters(cmd, params); + paramProcessWorkerSpy.processParameters(cmd, params); + Mockito.verify(paramProcessWorkerSpy).doAccessChecks(Mockito.any(), Mockito.any()); + } + + @Test + public void doAccessChecksTestChecksCallerAccessToOwnerWhenCmdExtendsBaseAsyncCreateCmd() { + Mockito.lenient().doReturn(owners).when(paramProcessWorkerSpy).getEntityOwners(Mockito.any()); + Mockito.doNothing().when(paramProcessWorkerSpy).checkCallerAccessToEntities(Mockito.any(), Mockito.any(), Mockito.any()); + + paramProcessWorkerSpy.doAccessChecks(new AssociateIPAddrCmd(), entities); + + Mockito.verify(accountManagerMock).checkAccess(callingAccountMock, null, false, owners); + } + + @Test + public void doAccessChecksTestChecksCallerAccessToEntities() { + Mockito.lenient().doReturn(owners).when(paramProcessWorkerSpy).getEntityOwners(Mockito.any()); + Mockito.doNothing().when(paramProcessWorkerSpy).checkCallerAccessToEntities(Mockito.any(), Mockito.any(), Mockito.any()); + + paramProcessWorkerSpy.doAccessChecks(new AssociateIPAddrCmd(), entities); + + Mockito.verify(paramProcessWorkerSpy).checkCallerAccessToEntities(callingAccountMock, owners, entities); + } + + @Test + public void getEntityOwnersTestReturnsAccountsWhenCmdHasMultipleEntityOwners() { + Mockito.when(baseCmdMock.getEntityOwnerIds()).thenReturn(List.of(1L, 2L)); + Mockito.doReturn(callingAccountMock).when(accountManagerMock).getAccount(1L); + Mockito.doReturn(ownerAccountMock).when(accountManagerMock).getAccount(2L); + + List result = List.of(paramProcessWorkerSpy.getEntityOwners(baseCmdMock)); + + Assert.assertEquals(List.of(callingAccountMock, ownerAccountMock), result); + } + + @Test + public void getEntityOwnersTestReturnsAccountWhenCmdHasOneEntityOwner() { + Mockito.when(baseCmdMock.getEntityOwnerId()).thenReturn(1L); + Mockito.when(baseCmdMock.getEntityOwnerIds()).thenReturn(null); + Mockito.doReturn(ownerAccountMock).when(accountManagerMock).getAccount(1L); + + List result = List.of(paramProcessWorkerSpy.getEntityOwners(baseCmdMock)); + + Assert.assertEquals(List.of(ownerAccountMock), result); + } + + @Test + public void checkCallerAccessToEntitiesTestChecksCallerAccessToOwners() { + entities.put(ownerAccountMock, SecurityChecker.AccessType.UseEntry); + + paramProcessWorkerSpy.checkCallerAccessToEntities(callingAccountMock, owners, entities); + + Mockito.verify(accountManagerMock).checkAccess(callingAccountMock, null, false, owners); + } + + @Test + public void checkCallerAccessToEntitiesTestChecksCallerAccessToResource() { + VMInstanceVO vmInstanceVo = new VMInstanceVO(); + entities.put(vmInstanceVo, SecurityChecker.AccessType.UseEntry); + + paramProcessWorkerSpy.checkCallerAccessToEntities(callingAccountMock, owners, entities); + + Mockito.verify(accountManagerMock).validateAccountHasAccessToResource(callingAccountMock, SecurityChecker.AccessType.UseEntry, vmInstanceVo); } } diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java index 4cf7413f3f3..bd6632af1ca 100644 --- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java +++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java @@ -438,6 +438,11 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco // TODO Auto-generated method stub } + @Override + public void validateAccountHasAccessToResource(Account account, AccessType accessType, Object resource) { + // TODO Auto-generated method stub + } + @Override public Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly) { // TODO Auto-generated method stub diff --git a/server/src/test/java/com/cloud/user/MockUsageEventDao.java b/server/src/test/java/com/cloud/user/MockUsageEventDao.java index fb8c193780f..7e15a4f0aaf 100644 --- a/server/src/test/java/com/cloud/user/MockUsageEventDao.java +++ b/server/src/test/java/com/cloud/user/MockUsageEventDao.java @@ -210,11 +210,6 @@ public class MockUsageEventDao implements UsageEventDao{ return 0; } - @Override - public int expunge(SearchCriteria sc, long limit) { - return 0; - } - @Override public void expunge() { diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/formatinspector/Qcow2HeaderField.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/formatinspector/Qcow2HeaderField.java new file mode 100644 index 00000000000..4a8e8b51a47 --- /dev/null +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/formatinspector/Qcow2HeaderField.java @@ -0,0 +1,51 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.storage.formatinspector; + +public enum Qcow2HeaderField { + MAGIC(0, 4), + VERSION(4, 4), + BACKING_FILE_OFFSET(8, 8), + BACKING_FILE_NAME_LENGTH(16, 4), + CLUSTER_BITS(20, 4), + SIZE(24, 8), + CRYPT_METHOD(32, 4), + L1_SIZE(36, 4), + LI_TABLE_OFFSET(40, 8), + REFCOUNT_TABLE_OFFSET(48, 8), + REFCOUNT_TABLE_CLUSTERS(56, 4), + NB_SNAPSHOTS(60, 4), + SNAPSHOTS_OFFSET(64, 8), + INCOMPATIBLE_FEATURES(72, 8); + + private final int offset; + private final int length; + + Qcow2HeaderField(int offset, int length) { + this.offset = offset; + this.length = length; + } + + public int getLength() { + return length; + } + + public int getOffset() { + return offset; + } +} diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/formatinspector/Qcow2Inspector.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/formatinspector/Qcow2Inspector.java new file mode 100644 index 00000000000..9d80064d292 --- /dev/null +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/formatinspector/Qcow2Inspector.java @@ -0,0 +1,268 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.storage.formatinspector; + +import com.cloud.utils.NumbersUtil; +import org.apache.commons.lang3.ArrayUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +/** + * Class to inspect QCOW2 files/objects. In our context, a QCOW2 might be a threat to the environment if it meets one of the following criteria when coming from external sources + * (like registering or uploading volumes and templates): + *
    + *
  • has a backing file reference;
    • + *
  • has an external data file reference;
    • + *
  • has unknown incompatible features.
    • + *
+ * + * The implementation was done based on the QEMU's official interoperability documentation + * and on the OpenStack's Cinder implementation for Python. + */ +public class Qcow2Inspector { + protected static Logger LOGGER = LogManager.getLogger(Qcow2Inspector.class); + + private static final byte[] QCOW_MAGIC_STRING = ArrayUtils.add("QFI".getBytes(), (byte) 0xfb); + private static final int INCOMPATIBLE_FEATURES_MAX_KNOWN_BIT = 4; + private static final int INCOMPATIBLE_FEATURES_MAX_KNOWN_BYTE = 0; + private static final int EXTERNAL_DATA_FILE_BYTE_POSITION = 7; + private static final int EXTERNAL_DATA_FILE_BIT = 2; + private static final byte EXTERNAL_DATA_FILE_BITMASK = (byte) (1 << EXTERNAL_DATA_FILE_BIT); + + private static final Set SET_OF_HEADER_FIELDS_TO_READ = Set.of(Qcow2HeaderField.MAGIC, + Qcow2HeaderField.VERSION, + Qcow2HeaderField.SIZE, + Qcow2HeaderField.BACKING_FILE_OFFSET, + Qcow2HeaderField.INCOMPATIBLE_FEATURES); + + /** + * Validates if the file is a valid and allowed QCOW2 (i.e.: does not contain external references). + * @param filePath Path of the file to be validated. + * @throws RuntimeException If the QCOW2 file meets one of the following criteria: + *
    + *
  • has a backing file reference;
    • + *
  • has an external data file reference;
    • + *
  • has unknown incompatible features.
    • + *
+ */ + public static void validateQcow2File(String filePath) throws RuntimeException { + LOGGER.info(String.format("Verifying if [%s] is a valid and allowed QCOW2 file .", filePath)); + + Map headerFieldsAndValues; + try (InputStream inputStream = new FileInputStream(filePath)) { + headerFieldsAndValues = unravelQcow2Header(inputStream, filePath); + } catch (IOException ex) { + throw new RuntimeException(String.format("Unable to validate file [%s] due to: ", filePath), ex); + } + + validateQcow2HeaderFields(headerFieldsAndValues, filePath); + + LOGGER.info(String.format("[%s] is a valid and allowed QCOW2 file.", filePath)); + } + + /** + * Unravels the QCOW2 header in a serial fashion, iterating through the {@link Qcow2HeaderField}, reading the fields specified in + * {@link Qcow2Inspector#SET_OF_HEADER_FIELDS_TO_READ} and skipping the others. + * @param qcow2InputStream InputStream of the QCOW2 being unraveled. + * @param qcow2LogReference A reference (like the filename) of the QCOW2 being unraveled to print in the logs and exceptions. + * @return A map of the header fields and their values according to the {@link Qcow2Inspector#SET_OF_HEADER_FIELDS_TO_READ}. + * @throws IOException If the field cannot be read or skipped. + */ + public static Map unravelQcow2Header(InputStream qcow2InputStream, String qcow2LogReference) throws IOException { + Map result = new HashMap<>(); + + LOGGER.debug(String.format("Unraveling QCOW2 [%s] headers.", qcow2LogReference)); + for (Qcow2HeaderField qcow2Header : Qcow2HeaderField.values()) { + if (!SET_OF_HEADER_FIELDS_TO_READ.contains(qcow2Header)) { + skipHeader(qcow2InputStream, qcow2Header, qcow2LogReference); + continue; + } + + byte[] headerValue = readHeader(qcow2InputStream, qcow2Header, qcow2LogReference); + result.put(qcow2Header.name(), headerValue); + } + + return result; + } + + /** + * Skips the field's length in the InputStream. + * @param qcow2InputStream InputStream of the QCOW2 being unraveled. + * @param field Field being skipped (name and length). + * @param qcow2LogReference A reference (like the filename) of the QCOW2 being unraveled to print in the logs and exceptions. + * @throws IOException If the bytes skipped do not match the field length. + */ + protected static void skipHeader(InputStream qcow2InputStream, Qcow2HeaderField field, String qcow2LogReference) throws IOException { + LOGGER.trace(String.format("Skipping field [%s] of QCOW2 [%s].", field, qcow2LogReference)); + + if (qcow2InputStream.skip(field.getLength()) != field.getLength()) { + throw new IOException(String.format("Unable to skip field [%s] of QCOW2 [%s].", field, qcow2LogReference)); + } + } + + /** + * Reads the field's length in the InputStream. + * @param qcow2InputStream InputStream of the QCOW2 being unraveled. + * @param field Field being read (name and length). + * @param qcow2LogReference A reference (like the filename) of the QCOW2 being unraveled to print in the logs and exceptions. + * @throws IOException If the bytes read do not match the field length. + */ + protected static byte[] readHeader(InputStream qcow2InputStream, Qcow2HeaderField field, String qcow2LogReference) throws IOException { + byte[] readBytes = new byte[field.getLength()]; + + LOGGER.trace(String.format("Reading field [%s] of QCOW2 [%s].", field, qcow2LogReference)); + if (qcow2InputStream.read(readBytes) != field.getLength()) { + throw new IOException(String.format("Unable to read field [%s] of QCOW2 [%s].", field, qcow2LogReference)); + } + + LOGGER.trace(String.format("Read %s as field [%s] of QCOW2 [%s].", ArrayUtils.toString(readBytes), field, qcow2LogReference)); + return readBytes; + } + + /** + * Validates the values of the header fields {@link Qcow2HeaderField#MAGIC}, {@link Qcow2HeaderField#BACKING_FILE_OFFSET}, and {@link Qcow2HeaderField#INCOMPATIBLE_FEATURES}. + * @param headerFieldsAndValues A map of the header fields and their values. + * @param qcow2LogReference A reference (like the filename) of the QCOW2 being unraveled to print in the logs and exceptions. + * @throws SecurityException If the QCOW2 does not contain the QCOW magic string or contains a backing file reference or incompatible features. + */ + public static void validateQcow2HeaderFields(Map headerFieldsAndValues, String qcow2LogReference) throws SecurityException{ + byte[] fieldValue = headerFieldsAndValues.get(Qcow2HeaderField.MAGIC.name()); + validateQcowMagicString(fieldValue, qcow2LogReference); + + fieldValue = headerFieldsAndValues.get(Qcow2HeaderField.BACKING_FILE_OFFSET.name()); + validateAbsenceOfBackingFileReference(NumbersUtil.bytesToLong(fieldValue), qcow2LogReference); + + fieldValue = headerFieldsAndValues.get(Qcow2HeaderField.INCOMPATIBLE_FEATURES.name()); + validateAbsenceOfIncompatibleFeatures(fieldValue, qcow2LogReference); + } + + /** + * Verifies if the first 4 bytes of the header are the QCOW magic string. Throws an exception if not. + * @param headerMagicString The first 4 bytes of the header. + * @param qcow2LogReference A reference (like the filename) of the QCOW2 being unraveled to print in the logs and exceptions. + * @throws SecurityException If the header's magic string is not the QCOW magic string. + */ + private static void validateQcowMagicString(byte[] headerMagicString, String qcow2LogReference) throws SecurityException { + LOGGER.debug(String.format("Verifying if [%s] has a valid QCOW magic string.", qcow2LogReference)); + + if (!Arrays.equals(QCOW_MAGIC_STRING, headerMagicString)) { + throw new SecurityException(String.format("[%s] is not a valid QCOW2 because its first 4 bytes are not the QCOW magic string.", qcow2LogReference)); + } + + LOGGER.debug(String.format("[%s] has a valid QCOW magic string.", qcow2LogReference)); + } + + /** + * Verifies if the QCOW2 has a backing file and throws an exception if so. + * @param backingFileOffset The backing file offset value of the QCOW2 header. + * @param qcow2LogReference A reference (like the filename) of the QCOW2 being unraveled to print in the logs and exceptions. + * @throws SecurityException If the QCOW2 has a backing file reference. + */ + private static void validateAbsenceOfBackingFileReference(long backingFileOffset, String qcow2LogReference) throws SecurityException { + LOGGER.debug(String.format("Verifying if [%s] has a backing file reference.", qcow2LogReference)); + + if (backingFileOffset != 0) { + throw new SecurityException(String.format("[%s] has a backing file reference. This can be an attack to the infrastructure; therefore, we will not accept" + + " this QCOW2.", qcow2LogReference)); + } + + LOGGER.debug(String.format("[%s] does not have a backing file reference.", qcow2LogReference)); + } + + /** + * Verifies if the QCOW2 has incompatible features and throw an exception if it has an external data file reference or unknown incompatible features. + * @param incompatibleFeatures The incompatible features bytes of the QCOW2 header. + * @param qcow2LogReference A reference (like the filename) of the QCOW2 being unraveled to print in the logs and exceptions. + * @throws SecurityException If the QCOW2 has an external data file reference or unknown incompatible features. + */ + private static void validateAbsenceOfIncompatibleFeatures(byte[] incompatibleFeatures, String qcow2LogReference) throws SecurityException { + LOGGER.debug(String.format("Verifying if [%s] has incompatible features.", qcow2LogReference)); + + if (NumbersUtil.bytesToLong(incompatibleFeatures) == 0) { + LOGGER.debug(String.format("[%s] does not have incompatible features.", qcow2LogReference)); + return; + } + + LOGGER.debug(String.format("[%s] has incompatible features.", qcow2LogReference)); + + validateAbsenceOfExternalDataFileReference(incompatibleFeatures, qcow2LogReference); + validateAbsenceOfUnknownIncompatibleFeatures(incompatibleFeatures, qcow2LogReference); + } + + /** + * Verifies if the QCOW2 has an external data file reference and throw an exception if so. + * @param incompatibleFeatures The incompatible features bytes of the QCOW2 header. + * @param qcow2LogReference A reference (like the filename) of the QCOW2 being unraveled to print in the logs and exceptions. + * @throws SecurityException If the QCOW2 has an external data file reference. + */ + private static void validateAbsenceOfExternalDataFileReference(byte[] incompatibleFeatures, String qcow2LogReference) throws SecurityException { + LOGGER.debug(String.format("Verifying if [%s] has an external data file reference.", qcow2LogReference)); + + if ((incompatibleFeatures[EXTERNAL_DATA_FILE_BYTE_POSITION] & EXTERNAL_DATA_FILE_BITMASK) != 0) { + throw new SecurityException(String.format("[%s] has an external data file reference. This can be an attack to the infrastructure; therefore, we will discard" + + " this file.", qcow2LogReference)); + } + + LOGGER.info(String.format("[%s] does not have an external data file reference.", qcow2LogReference)); + } + + /** + * Verifies if the QCOW2 has unknown incompatible features and throw an exception if so. + *

+ * Unknown incompatible features are those with bit greater than + * {@link Qcow2Inspector#INCOMPATIBLE_FEATURES_MAX_KNOWN_BIT}, which will be the represented by bytes in positions greater than + * {@link Qcow2Inspector#INCOMPATIBLE_FEATURES_MAX_KNOWN_BYTE} (in Big Endian order). Therefore, we expect that those bytes are always zero. If not, an exception is thrown. + * @param incompatibleFeatures The incompatible features bytes of the QCOW2 header. + * @param qcow2LogReference A reference (like the filename) of the QCOW2 being unraveled to print in the logs and exceptions. + * @throws SecurityException If the QCOW2 has unknown incompatible features. + */ + private static void validateAbsenceOfUnknownIncompatibleFeatures(byte[] incompatibleFeatures, String qcow2LogReference) throws SecurityException { + LOGGER.debug(String.format("Verifying if [%s] has unknown incompatible features [%s].", qcow2LogReference, ArrayUtils.toString(incompatibleFeatures))); + + for (int byteNum = incompatibleFeatures.length - 1; byteNum >= 0; byteNum--) { + int bytePosition = incompatibleFeatures.length - 1 - byteNum; + LOGGER.trace(String.format("Looking for unknown incompatible feature bit in position [%s].", bytePosition)); + + byte bitmask = 0; + if (byteNum == INCOMPATIBLE_FEATURES_MAX_KNOWN_BYTE) { + bitmask = ((1 << INCOMPATIBLE_FEATURES_MAX_KNOWN_BIT) - 1); + } + + LOGGER.trace(String.format("Bitmask for byte in position [%s] is [%s].", bytePosition, Integer.toBinaryString(bitmask))); + + int featureBit = incompatibleFeatures[bytePosition] & ~bitmask; + if (featureBit != 0) { + throw new SecurityException(String.format("Found unknown incompatible feature bit [%s] in byte [%s] of [%s]. This can be an attack to the infrastructure; " + + "therefore, we will discard this QCOW2.", featureBit, bytePosition + Qcow2HeaderField.INCOMPATIBLE_FEATURES.getOffset(), qcow2LogReference)); + } + + LOGGER.trace(String.format("Did not find unknown incompatible feature in position [%s].", bytePosition)); + } + + LOGGER.info(String.format("[%s] does not have unknown incompatible features.", qcow2LogReference)); + } + +} diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java index 26b9e3ea65e..4cd4e8caa30 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java @@ -71,6 +71,7 @@ import org.apache.cloudstack.storage.command.UploadStatusCommand; import org.apache.cloudstack.storage.command.browser.ListDataStoreObjectsCommand; import org.apache.cloudstack.storage.configdrive.ConfigDrive; import org.apache.cloudstack.storage.configdrive.ConfigDriveBuilder; +import org.apache.cloudstack.storage.formatinspector.Qcow2Inspector; import org.apache.cloudstack.storage.template.DownloadManager; import org.apache.cloudstack.storage.template.DownloadManagerImpl; import org.apache.cloudstack.storage.template.UploadEntity; @@ -3482,8 +3483,19 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S return result; } + String finalFilename = resourcePath + "/" + templateFilename; + + if (ImageStoreUtil.isCorrectExtension(finalFilename, "qcow2")) { + try { + Qcow2Inspector.validateQcow2File(finalFilename); + } catch (RuntimeException e) { + logger.error(String.format("Uploaded file [%s] is not a valid QCOW2.", finalFilename), e); + return "The uploaded file is not a valid QCOW2. Ask the administrator to check the logs for more details."; + } + } + // Set permissions for the downloaded template - File downloadedTemplate = new File(resourcePath + "/" + templateFilename); + File downloadedTemplate = new File(finalFilename); _storage.setWorldReadableAndWriteable(downloadedTemplate); // Set permissions for template/volume.properties diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java index ad73a9bc708..e2eecbfcf22 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java @@ -16,8 +16,6 @@ // under the License. package org.apache.cloudstack.storage.template; -import static com.cloud.utils.NumbersUtil.toHumanReadableSize; - import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -51,9 +49,10 @@ import org.apache.cloudstack.storage.command.DownloadProgressCommand.RequestType import org.apache.cloudstack.storage.resource.IpTablesHelper; import org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource; import org.apache.cloudstack.storage.resource.SecondaryStorageResource; +import org.apache.cloudstack.storage.formatinspector.Qcow2HeaderField; +import org.apache.cloudstack.storage.formatinspector.Qcow2Inspector; import org.apache.cloudstack.utils.security.ChecksumValue; import org.apache.cloudstack.utils.security.DigestHelper; -import org.apache.commons.lang3.StringUtils; import com.cloud.agent.api.storage.DownloadAnswer; import com.cloud.agent.api.to.DataStoreTO; @@ -89,11 +88,14 @@ import com.cloud.utils.NumbersUtil; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.Proxy; +import com.cloud.utils.StringUtils; import com.cloud.utils.script.Script; -import com.cloud.utils.storage.QCOW2Utils; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import static com.cloud.utils.NumbersUtil.toHumanReadableSize; + public class DownloadManagerImpl extends ManagerBase implements DownloadManager { protected static Logger LOGGER = LogManager.getLogger(DownloadManagerImpl.class); private String _name; @@ -366,11 +368,17 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager // The QCOW2 is the only format with a header, // and as such can be easily read. - try (InputStream inputStream = td.getS3ObjectInputStream();) { - dnld.setTemplatesize(QCOW2Utils.getVirtualSize(inputStream, false)); - } - catch (IOException e) { - result = "Couldn't read QCOW2 virtual size. Error: " + e.getMessage(); + try (InputStream inputStream = td.getS3ObjectInputStream()) { + Map qcow2HeaderFieldsAndValues = Qcow2Inspector.unravelQcow2Header(inputStream, td.getDownloadUrl()); + Qcow2Inspector.validateQcow2HeaderFields(qcow2HeaderFieldsAndValues, td.getDownloadUrl()); + + dnld.setTemplatesize(NumbersUtil.bytesToLong(qcow2HeaderFieldsAndValues.get(Qcow2HeaderField.SIZE.name()))); + } catch (IOException ex) { + result = String.format("Unable to read QCOW2 metadata. Error: %s", ex.getMessage()); + LOGGER.error(result, ex); + } catch (SecurityException ex) { + result = String.format("[%s] is not a valid QCOW2:", td.getDownloadUrl()); + LOGGER.error(result, ex); } } @@ -516,8 +524,19 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager return result; } + String finalFilename = resourcePath + "/" + templateFilename; + + if (ImageFormat.QCOW2.equals(dnld.getFormat())) { + try { + Qcow2Inspector.validateQcow2File(finalFilename); + } catch (RuntimeException e) { + LOGGER.error(String.format("The downloaded file [%s] is not a valid QCOW2.", finalFilename), e); + return "The downloaded file is not a valid QCOW2. Ask the administrator to check the logs for more details."; + } + } + // Set permissions for the downloaded template - File downloadedTemplate = new File(resourcePath + "/" + templateFilename); + File downloadedTemplate = new File(finalFilename); _storage.setWorldReadableAndWriteable(downloadedTemplate); setPermissionsForTheDownloadedTemplate(resourcePath, resourceType); diff --git a/systemvm/debian/opt/cloud/bin/cs/CsFile.py b/systemvm/debian/opt/cloud/bin/cs/CsFile.py index b33dbcff1c2..bb46bab350a 100755 --- a/systemvm/debian/opt/cloud/bin/cs/CsFile.py +++ b/systemvm/debian/opt/cloud/bin/cs/CsFile.py @@ -174,6 +174,6 @@ class CsFile: self.new_config = list(temp_config) def compare(self, o): - result = (isinstance(o, self.__class__) and set(self.config) == set(o.config)) + result = (isinstance(o, self.__class__) and self.config == o.config) logging.debug("Comparison of CsFiles content is ==> %s" % result) return result diff --git a/test/integration/smoke/test_login.py b/test/integration/smoke/test_login.py index 40d8349a13d..acd6c4334ac 100644 --- a/test/integration/smoke/test_login.py +++ b/test/integration/smoke/test_login.py @@ -133,6 +133,7 @@ class TestLogin(cloudstackTestCase): args["command"] = 'listUsers' args["listall"] = 'true' args["response"] = "json" + args["sessionkey"] = response.json()['loginresponse']['sessionkey'] response = session.get(self.server_url, params=args) self.assertEqual( response.status_code, diff --git a/ui/src/api/index.js b/ui/src/api/index.js index 1db41661276..7ab87780a9d 100644 --- a/ui/src/api/index.js +++ b/ui/src/api/index.js @@ -15,8 +15,13 @@ // specific language governing permissions and limitations // under the License. +import Cookies from 'js-cookie' import { axios, sourceToken } from '@/utils/request' import { message, notification } from 'ant-design-vue' +import { vueProps } from '@/vue-app' +import { + ACCESS_TOKEN +} from '@/store/mutation-types' export function api (command, args = {}, method = 'GET', data = {}) { let params = {} @@ -30,6 +35,11 @@ export function api (command, args = {}, method = 'GET', data = {}) { }) } + const sessionkey = vueProps.$localStorage.get(ACCESS_TOKEN) || Cookies.get('sessionkey') + if (sessionkey) { + args.sessionkey = sessionkey + } + return axios({ params: { ...args @@ -65,7 +75,6 @@ export function login (arg) { } export function logout () { - sourceToken.cancel() message.destroy() notification.destroy() return api('logout') diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index 5a82c1ce48a..5737126d463 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -542,7 +542,7 @@ export default { if (store.listAllProjects) { fields.push('project') } - if (store.apis.scaleKubernetesCluster.params.filter(x => x.name === 'autoscalingenabled').length > 0) { + if (store.apis.scaleKubernetesCluster?.params?.filter(x => x.name === 'autoscalingenabled').length > 0) { fields.splice(2, 0, 'autoscalingenabled') } fields.push('zonename') diff --git a/ui/src/store/modules/user.js b/ui/src/store/modules/user.js index 22a2fab959d..bd0afaa17bf 100644 --- a/ui/src/store/modules/user.js +++ b/ui/src/store/modules/user.js @@ -444,11 +444,6 @@ const user = { cloudianUrl = state.cloudian.url + 'logout.htm?redirect=' + encodeURIComponent(window.location.href) } - Object.keys(Cookies.get()).forEach(cookieName => { - Cookies.remove(cookieName) - Cookies.remove(cookieName, { path: '/client' }) - }) - commit('SET_TOKEN', '') commit('SET_APIS', {}) commit('SET_PROJECT', {}) @@ -476,6 +471,11 @@ const user = { } }).catch(() => { resolve() + }).finally(() => { + Object.keys(Cookies.get()).forEach(cookieName => { + Cookies.remove(cookieName) + Cookies.remove(cookieName, { path: '/client' }) + }) }) }) }, diff --git a/ui/src/views/compute/KubernetesServiceTab.vue b/ui/src/views/compute/KubernetesServiceTab.vue index d6f6880621b..1c2c83a35ce 100644 --- a/ui/src/views/compute/KubernetesServiceTab.vue +++ b/ui/src/views/compute/KubernetesServiceTab.vue @@ -284,7 +284,7 @@ export default { } }, mounted () { - if (this.$store.getters.apis.scaleKubernetesCluster.params.filter(x => x.name === 'nodeids').length > 0 && this.resource.clustertype === 'CloudManaged') { + if (this.$store.getters.apis.scaleKubernetesCluster?.params?.filter(x => x.name === 'nodeids').length > 0 && this.resource.clustertype === 'CloudManaged') { this.vmColumns.push({ key: 'actions', title: this.$t('label.actions'), diff --git a/ui/src/views/infra/UpdatePrimaryStorage.vue b/ui/src/views/infra/UpdatePrimaryStorage.vue index 7c026630a99..16de255a988 100644 --- a/ui/src/views/infra/UpdatePrimaryStorage.vue +++ b/ui/src/views/infra/UpdatePrimaryStorage.vue @@ -121,7 +121,6 @@ export default { }, created () { this.initForm() - this.form.name = this.resource.name }, computed: { canUpdateNFSMountOpts () { @@ -136,7 +135,14 @@ export default { methods: { initForm () { this.formRef = ref() - this.form = reactive({ }) + this.form = reactive({ + name: this.resource.name, + tags: this.resource.tags, + isTagARule: this.resource.istagarule, + capacityBytes: this.resource.disksizetotal, + capacityIOPS: this.resource.capacityiops, + nfsMountOpts: this.resource.nfsmountopts + }) this.rules = reactive({ }) }, isAdmin () { diff --git a/ui/tests/unit/components/view/ActionButton.spec.js b/ui/tests/unit/components/view/ActionButton.spec.js index 7e41f0bd2f8..1565b471548 100644 --- a/ui/tests/unit/components/view/ActionButton.spec.js +++ b/ui/tests/unit/components/view/ActionButton.spec.js @@ -23,6 +23,16 @@ import mockData from '../../../mockData/ActionButton.mock.json' import ActionButton from '@/components/view/ActionButton' jest.mock('axios', () => mockAxios) +jest.mock('@/vue-app', () => ({ + vueProps: { + $localStorage: { + set: jest.fn((key, value) => {}), + get: jest.fn((key) => { + return null + }) + } + } +})) let router, store, i18n const state = { diff --git a/ui/tests/unit/views/compute/MigrateWizard.spec.js b/ui/tests/unit/views/compute/MigrateWizard.spec.js index f352b2de2c4..d3ee49426dc 100644 --- a/ui/tests/unit/views/compute/MigrateWizard.spec.js +++ b/ui/tests/unit/views/compute/MigrateWizard.spec.js @@ -23,6 +23,16 @@ import mockData from '../../../mockData/MigrateWizard.mock' import MigrateWizard from '@/views/compute/MigrateWizard' jest.mock('axios', () => mockAxios) +jest.mock('@/vue-app', () => ({ + vueProps: { + $localStorage: { + set: jest.fn((key, value) => {}), + get: jest.fn((key) => { + return null + }) + } + } +})) let i18n let store diff --git a/ui/vue.config.js b/ui/vue.config.js index c74218b4bcf..9cae2ff66fb 100644 --- a/ui/vue.config.js +++ b/ui/vue.config.js @@ -142,7 +142,11 @@ const vueConfig = { secure: false, ws: false, changeOrigin: true, - proxyTimeout: 10 * 60 * 1000 // 10 minutes + proxyTimeout: 10 * 60 * 1000, // 10 minutes + cookieDomainRewrite: '*', + cookiePathRewrite: { + '/client': '/' + } } }, https: process.env.HTTPS_KEY ? { diff --git a/utils/src/main/java/com/cloud/utils/HttpUtils.java b/utils/src/main/java/com/cloud/utils/HttpUtils.java index 434296c1900..1cbc4c79c17 100644 --- a/utils/src/main/java/com/cloud/utils/HttpUtils.java +++ b/utils/src/main/java/com/cloud/utils/HttpUtils.java @@ -38,6 +38,14 @@ public class HttpUtils { public static final String JSON_CONTENT_TYPE = "application/json; charset=UTF-8"; public static final String XML_CONTENT_TYPE = "text/xml; charset=UTF-8"; + public enum ApiSessionKeySameSite { + Lax, Strict, NoneAndSecure, Null + } + + public enum ApiSessionKeyCheckOption { + CookieOrParameter, ParameterOnly, CookieAndParameter + } + public static void addSecurityHeaders(final HttpServletResponse resp) { if (resp.containsHeader("X-Content-Type-Options")) { resp.setHeader("X-Content-Type-Options", "nosniff"); @@ -104,23 +112,43 @@ public class HttpUtils { return null; } - public static boolean validateSessionKey(final HttpSession session, final Map params, final Cookie[] cookies, final String sessionKeyString) { + public static boolean validateSessionKey(final HttpSession session, final Map params, final Cookie[] cookies, final String sessionKeyString, final ApiSessionKeyCheckOption apiSessionKeyCheckLocations) { if (session == null || sessionKeyString == null) { return false; } + final String jsessionidFromCookie = HttpUtils.findCookie(cookies, "JSESSIONID"); + if (jsessionidFromCookie != null + && !(jsessionidFromCookie.equals(session.getId()) || jsessionidFromCookie.startsWith(session.getId() + '.'))) { + LOGGER.error("JSESSIONID from cookie is invalid."); + return false; + } final String sessionKey = (String) session.getAttribute(sessionKeyString); + if (sessionKey == null) { + LOGGER.error("sessionkey attribute of the session is null."); + return false; + } final String sessionKeyFromCookie = HttpUtils.findCookie(cookies, sessionKeyString); + boolean isSessionKeyFromCookieValid = sessionKeyFromCookie != null && sessionKey.equals(sessionKeyFromCookie); + String[] sessionKeyFromParams = null; if (params != null) { sessionKeyFromParams = (String[]) params.get(sessionKeyString); } - if ((sessionKey == null) - || (sessionKeyFromParams == null && sessionKeyFromCookie == null) - || (sessionKeyFromParams != null && !sessionKey.equals(sessionKeyFromParams[0])) - || (sessionKeyFromCookie != null && !sessionKey.equals(sessionKeyFromCookie))) { - return false; + boolean isSessionKeyFromParamsValid = sessionKeyFromParams != null && sessionKey.equals(sessionKeyFromParams[0]); + + switch (apiSessionKeyCheckLocations) { + case CookieOrParameter: + return (sessionKeyFromCookie != null || sessionKeyFromParams != null) + && (sessionKeyFromCookie == null || isSessionKeyFromCookieValid) + && (sessionKeyFromParams == null || isSessionKeyFromParamsValid); + case ParameterOnly: + return sessionKeyFromParams != null && isSessionKeyFromParamsValid + && (sessionKeyFromCookie == null || isSessionKeyFromCookieValid); + case CookieAndParameter: + default: + return sessionKeyFromCookie != null && isSessionKeyFromCookieValid + && sessionKeyFromParams != null && isSessionKeyFromParamsValid; } - return true; } } diff --git a/utils/src/test/java/com/cloud/utils/HttpUtilsTest.java b/utils/src/test/java/com/cloud/utils/HttpUtilsTest.java index e10a5a36b27..9047934c75c 100644 --- a/utils/src/test/java/com/cloud/utils/HttpUtilsTest.java +++ b/utils/src/test/java/com/cloud/utils/HttpUtilsTest.java @@ -60,35 +60,97 @@ public class HttpUtilsTest { final String sessionKeyValue = "randomUniqueSessionID"; // session and sessionKeyString null test - assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); sessionKeyString = "sessionkey"; - assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); // param and cookie null test session = new MockHttpSession(); + final String sessionId = session.getId(); session.setAttribute(sessionKeyString, sessionKeyValue); - assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); - // param null, cookies not null test + // param null, cookies not null test (JSESSIONID is null) params = null; cookies = new Cookie[]{new Cookie(sessionKeyString, sessionKeyValue)}; - assertFalse(HttpUtils.validateSessionKey(session, params, cookies, "randomString")); - assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, "randomString", HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + + // param null, cookies not null test (JSESSIONID is not null and matches) + cookies = new Cookie[2]; + cookies[0] = new Cookie(sessionKeyString, sessionKeyValue); + cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0"); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, "randomString", HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + + // param null, cookies not null test (JSESSIONID is not null but mismatches) + cookies = new Cookie[2]; + cookies[0] = new Cookie(sessionKeyString, sessionKeyValue); + cookies[1] = new Cookie("JSESSIONID", "node0xxxxxxxxxxxxx.node0"); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, "randomString", HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); // param not null, cookies null test params = new HashMap(); params.put(sessionKeyString, new String[]{"randomString"}); cookies = null; - assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); params.put(sessionKeyString, new String[]{sessionKeyValue}); - assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString)); + assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); - // both param and cookies not null test + // both param and cookies not null test (JSESSIONID is null) params = new HashMap(); - cookies = new Cookie[]{new Cookie(sessionKeyString, sessionKeyValue)}; + cookies = new Cookie[2]; + cookies[0] = new Cookie(sessionKeyString, sessionKeyValue); params.put(sessionKeyString, new String[]{"incorrectValue"}); - assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); params.put(sessionKeyString, new String[]{sessionKeyValue}); - assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString)); + assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + + // both param and cookies not null test (JSESSIONID is not null but mismatches) + params = new HashMap(); + cookies = new Cookie[2]; + cookies[0] = new Cookie(sessionKeyString, sessionKeyValue); + cookies[1] = new Cookie("JSESSIONID", "node0xxxxxxxxxxxxx.node0"); + params.put(sessionKeyString, new String[]{"incorrectValue"}); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + params.put(sessionKeyString, new String[]{sessionKeyValue}); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + + // both param and cookies not null test (JSESSIONID is not null amd matches) + params = new HashMap(); + cookies = new Cookie[2]; + cookies[0] = new Cookie(sessionKeyString, sessionKeyValue); + cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0"); + params.put(sessionKeyString, new String[]{"incorrectValue"}); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + params.put(sessionKeyString, new String[]{sessionKeyValue}); + assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + + // param not null, cookies null test (JSESSIONID is not null amd matches) + params = new HashMap(); + cookies = new Cookie[1]; + cookies[0] = new Cookie("JSESSIONID", sessionId + ".node0"); + params.put(sessionKeyString, new String[]{"incorrectValue"}); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.ParameterOnly)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter)); + params.put(sessionKeyString, new String[]{sessionKeyValue}); + assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.ParameterOnly)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter)); + + // param not null (correct), cookies not null test (correct) + cookies = new Cookie[2]; + cookies[0] = new Cookie(sessionKeyString, sessionKeyValue); + cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0"); + assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.ParameterOnly)); + assertTrue(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter)); + + // param not null (correct), cookies not null test (wrong) + cookies = new Cookie[2]; + cookies[0] = new Cookie(sessionKeyString, "incorrectValue"); + cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0"); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.ParameterOnly)); + assertFalse(HttpUtils.validateSessionKey(session, params, cookies, sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter)); } }