From 5481485a083957ff58da3b6fea9d7b6d20f06875 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Fri, 27 Feb 2015 18:20:10 +0530 Subject: [PATCH] CLOUDSTACK-5238: password checks, NPE fixes and minor fixes - insecure authenticators excluded in configuration - snapshot response should have zone - remove vmsnapshots when removing accounts Signed-off-by: Rohit Yadav --- .../api/response/VMSnapshotResponse.java | 8 ++++++ .../cloud/vm/snapshot/dao/VMSnapshotDao.java | 2 ++ .../vm/snapshot/dao/VMSnapshotDaoImpl.java | 6 +++++ .../src/com/cloud/api/ApiResponseHelper.java | 13 ++++++--- .../cloud/ha/dao/HighAvailabilityDaoImpl.java | 2 ++ .../cloud/hypervisor/HypervisorGuruBase.java | 6 +++-- server/src/com/cloud/hypervisor/KVMGuru.java | 27 ++++++++++++++----- .../cloud/resource/ResourceManagerImpl.java | 6 ++--- .../ResourceLimitManagerImpl.java | 3 ++- .../cloud/server/ConfigurationServerImpl.java | 3 +++ .../storage/download/DownloadActiveState.java | 1 + .../com/cloud/user/AccountManagerImpl.java | 23 +++++++++++++++- 12 files changed, 83 insertions(+), 17 deletions(-) diff --git a/api/src/org/apache/cloudstack/api/response/VMSnapshotResponse.java b/api/src/org/apache/cloudstack/api/response/VMSnapshotResponse.java index 168c5482213..4d430a5d644 100644 --- a/api/src/org/apache/cloudstack/api/response/VMSnapshotResponse.java +++ b/api/src/org/apache/cloudstack/api/response/VMSnapshotResponse.java @@ -184,6 +184,14 @@ public class VMSnapshotResponse extends BaseResponse implements ControlledEntity return parentName; } + public String getParent() { + return parent; + } + + public void setParent(String parent) { + this.parent = parent; + } + public String getType() { return type; } diff --git a/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDao.java b/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDao.java index e714a6e30e9..31999ef15d6 100644 --- a/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDao.java +++ b/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDao.java @@ -36,4 +36,6 @@ public interface VMSnapshotDao extends GenericDao, StateDao< List listByParent(Long vmSnapshotId); VMSnapshotVO findByName(Long vmId, String name); + + List listByAccountId(Long accountId); } diff --git a/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java b/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java index dccd19fca94..a87d284dc12 100644 --- a/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java +++ b/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java @@ -121,6 +121,12 @@ public class VMSnapshotDaoImpl extends GenericDaoBase implem return null; } + public List listByAccountId(Long accountId) { + SearchCriteria sc = this.AllFieldsSearch.create(); + sc.setParameters("accountId", new Object[] { accountId }); + return listBy(sc, null); + } + @Override public boolean updateState(State currentState, Event event, State nextState, VMSnapshot vo, Object data) { diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java index a13e9969423..c943f6f223a 100755 --- a/server/src/com/cloud/api/ApiResponseHelper.java +++ b/server/src/com/cloud/api/ApiResponseHelper.java @@ -452,6 +452,10 @@ public class ApiResponseHelper implements ResponseGenerator { snapshotResponse.setVolumeId(volume.getUuid()); snapshotResponse.setVolumeName(volume.getName()); snapshotResponse.setVolumeType(volume.getVolumeType().name()); + DataCenter zone = ApiDBUtils.findZoneById(volume.getDeviceId()); + if (zone != null) { + snapshotResponse.setZoneId(zone.getUuid()); + } } snapshotResponse.setCreated(snapshot.getCreated()); snapshotResponse.setName(snapshot.getName()); @@ -502,6 +506,7 @@ public class ApiResponseHelper implements ResponseGenerator { if (vmSnapshot.getParent() != null) { VMSnapshot vmSnapshotParent = ApiDBUtils.getVMSnapshotById(vmSnapshot.getParent()); if (vmSnapshotParent != null) { + vmSnapshotResponse.setParent(vmSnapshotParent.getUuid()); vmSnapshotResponse.setParentName(vmSnapshotParent.getDisplayName()); } } @@ -3096,9 +3101,11 @@ public class ApiResponseHelper implements ResponseGenerator { } } //Network ID - NetworkVO network = _entityMgr.findByIdIncludingRemoved(NetworkVO.class, usageRecord.getNetworkId().toString()); - if (network != null) { - usageRecResponse.setNetworkId(network.getUuid()); + if (usageRecord.getNetworkId() != null && usageRecord.getNetworkId() != 0L) { + NetworkVO network = _entityMgr.findByIdIncludingRemoved(NetworkVO.class, usageRecord.getNetworkId().toString()); + if (network != null) { + usageRecResponse.setNetworkId(network.getUuid()); + } } } else if (usageRecord.getUsageType() == UsageTypes.VM_DISK_IO_READ || usageRecord.getUsageType() == UsageTypes.VM_DISK_IO_WRITE diff --git a/server/src/com/cloud/ha/dao/HighAvailabilityDaoImpl.java b/server/src/com/cloud/ha/dao/HighAvailabilityDaoImpl.java index d25c6a79378..724f4f6d7c9 100644 --- a/server/src/com/cloud/ha/dao/HighAvailabilityDaoImpl.java +++ b/server/src/com/cloud/ha/dao/HighAvailabilityDaoImpl.java @@ -63,6 +63,7 @@ public class HighAvailabilityDaoImpl extends GenericDaoBase impl TBASearch.and("server", TBASearch.entity().getServerId(), Op.NULL); TBASearch.and("taken", TBASearch.entity().getDateTaken(), Op.NULL); TBASearch.and("time", TBASearch.entity().getTimeToTry(), Op.LTEQ); + TBASearch.and("step", TBASearch.entity().getStep(), Op.NIN); TBASearch.done(); PreviousInstanceSearch = createSearchBuilder(); @@ -151,6 +152,7 @@ public class HighAvailabilityDaoImpl extends GenericDaoBase impl try { final SearchCriteria sc = TBASearch.create(); sc.setParameters("time", System.currentTimeMillis() >> 10); + sc.setParameters("step", Step.Done, Step.Cancelled); final Filter filter = new Filter(HaWorkVO.class, null, true, 0l, 1l); diff --git a/server/src/com/cloud/hypervisor/HypervisorGuruBase.java b/server/src/com/cloud/hypervisor/HypervisorGuruBase.java index 0cb9af5bf8c..0188778e764 100644 --- a/server/src/com/cloud/hypervisor/HypervisorGuruBase.java +++ b/server/src/com/cloud/hypervisor/HypervisorGuruBase.java @@ -35,6 +35,7 @@ import com.cloud.offering.ServiceOffering; import com.cloud.resource.ResourceManager; import com.cloud.server.ConfigurationServer; import com.cloud.service.ServiceOfferingDetailsVO; +import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.service.dao.ServiceOfferingDetailsDao; import com.cloud.storage.dao.VMTemplateDetailsDao; import com.cloud.utils.Pair; @@ -71,6 +72,8 @@ public abstract class HypervisorGuruBase extends AdapterBase implements Hypervis ResourceManager _resourceMgr; @Inject ServiceOfferingDetailsDao _serviceOfferingDetailsDao; + @Inject + ServiceOfferingDao _serviceOfferingDao; protected HypervisorGuruBase() { super(); @@ -125,8 +128,7 @@ public abstract class HypervisorGuruBase extends AdapterBase implements Hypervis } protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) { - - ServiceOffering offering = vmProfile.getServiceOffering(); + ServiceOffering offering = _serviceOfferingDao.findById(vmProfile.getId(), vmProfile.getServiceOfferingId()); VirtualMachine vm = vmProfile.getVirtualMachine(); Long minMemory = (long)(offering.getRamSize() / vmProfile.getMemoryOvercommitRatio()); int minspeed = (int)(offering.getSpeed() / vmProfile.getCpuOvercommitRatio()); diff --git a/server/src/com/cloud/hypervisor/KVMGuru.java b/server/src/com/cloud/hypervisor/KVMGuru.java index a2534bdef2a..e15a41752e6 100644 --- a/server/src/com/cloud/hypervisor/KVMGuru.java +++ b/server/src/com/cloud/hypervisor/KVMGuru.java @@ -16,24 +16,25 @@ // under the License. package com.cloud.hypervisor; -import java.util.Map; - -import javax.ejb.Local; -import javax.inject.Inject; - -import org.apache.cloudstack.storage.command.StorageSubSystemCommand; - import com.cloud.agent.api.Command; +import com.cloud.agent.api.to.DataObjectType; import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.DataStoreRole; import com.cloud.storage.GuestOSHypervisorVO; import com.cloud.storage.GuestOSVO; import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.GuestOSHypervisorDao; import com.cloud.utils.Pair; import com.cloud.vm.VirtualMachineProfile; +import org.apache.cloudstack.storage.command.CopyCommand; +import org.apache.cloudstack.storage.command.StorageSubSystemCommand; + +import javax.ejb.Local; +import javax.inject.Inject; +import java.util.Map; @Local(value = HypervisorGuru.class) public class KVMGuru extends HypervisorGuruBase implements HypervisorGuru { @@ -77,6 +78,18 @@ public class KVMGuru extends HypervisorGuruBase implements HypervisorGuru { @Override public Pair getCommandHostDelegation(long hostId, Command cmd) { + if (cmd instanceof CopyCommand) { + CopyCommand c = (CopyCommand) cmd; + boolean inSeq = true; + if (c.getSrcTO().getObjectType() == DataObjectType.SNAPSHOT || + c.getDestTO().getObjectType() == DataObjectType.SNAPSHOT) { + inSeq = false; + } else if (c.getDestTO().getDataStore().getRole() == DataStoreRole.Image || + c.getDestTO().getDataStore().getRole() == DataStoreRole.ImageCache) { + inSeq = false; + } + c.setExecuteInSequence(inSeq); + } if (cmd instanceof StorageSubSystemCommand) { StorageSubSystemCommand c = (StorageSubSystemCommand)cmd; c.setExecuteInSequence(false); diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java index f8ca84dce67..b0b18490cab 100755 --- a/server/src/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/com/cloud/resource/ResourceManagerImpl.java @@ -30,7 +30,6 @@ import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.capacity.CapacityState; import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.api.ApiConstants; @@ -71,6 +70,7 @@ import com.cloud.agent.api.to.GPUDeviceTO; import com.cloud.agent.transport.Request; import com.cloud.capacity.Capacity; import com.cloud.capacity.CapacityManager; +import com.cloud.capacity.CapacityState; import com.cloud.capacity.CapacityVO; import com.cloud.capacity.dao.CapacityDao; import com.cloud.cluster.ClusterManager; @@ -1174,12 +1174,13 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, MaintainAnswer answer = (MaintainAnswer)_agentMgr.easySend(hostId, new MaintainCommand()); if (answer == null || !answer.getResult()) { s_logger.warn("Unable to send MaintainCommand to host: " + hostId); + return false; } try { resourceStateTransitTo(host, ResourceState.Event.AdminAskMaintenace, _nodeId); } catch (NoTransitionException e) { - String err = "Cannot transimit resource state of host " + host.getId() + " to " + ResourceState.Maintenance; + String err = "Cannot transmit resource state of host " + host.getId() + " to " + ResourceState.Maintenance; s_logger.debug(err, e); throw new CloudRuntimeException(err + e.getMessage()); } @@ -1210,7 +1211,6 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, } } } - return true; } diff --git a/server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index fd348af7496..8bf091811bd 100755 --- a/server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -847,8 +847,9 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim // this lock guards against the updates to user_vm, volume, snapshot, public _ip and template table // as any resource creation precedes with the resourceLimitExceeded check which needs this lock too + Set rowIdsToLock = _resourceCountDao.listAllRowsToUpdate(accountId, Resource.ResourceOwnerType.Account, type); SearchCriteria sc = ResourceCountSearch.create(); - sc.setParameters("accountId", accountId); + sc.setParameters("id", rowIdsToLock.toArray()); _resourceCountDao.lockRows(sc, null, true); ResourceCountVO accountRC = _resourceCountDao.findByOwnerAndType(accountId, ResourceOwnerType.Account, type); diff --git a/server/src/com/cloud/server/ConfigurationServerImpl.java b/server/src/com/cloud/server/ConfigurationServerImpl.java index 306c0e7bf0f..b85f13764b0 100755 --- a/server/src/com/cloud/server/ConfigurationServerImpl.java +++ b/server/src/com/cloud/server/ConfigurationServerImpl.java @@ -219,6 +219,9 @@ public class ConfigurationServerImpl extends ManagerBase implements Configuratio _configDao.update("secstorage.secure.copy.cert", "realhostip"); s_logger.debug("ConfigurationServer made secondary storage copy use realhostip."); + _configDao.update("user.password.encoders.exclude", "MD5,LDAP,PLAINTEXT"); + s_logger.debug("Configuration server excluded insecure encoders"); + // Save default service offerings createServiceOffering(User.UID_SYSTEM, "Small Instance", 1, 512, 500, "Small Instance", ProvisioningType.THIN, false, false, null); createServiceOffering(User.UID_SYSTEM, "Medium Instance", 1, 1024, 1000, "Medium Instance", ProvisioningType.THIN, false, false, null); diff --git a/server/src/com/cloud/storage/download/DownloadActiveState.java b/server/src/com/cloud/storage/download/DownloadActiveState.java index 59709f26c04..d202d9add31 100644 --- a/server/src/com/cloud/storage/download/DownloadActiveState.java +++ b/server/src/com/cloud/storage/download/DownloadActiveState.java @@ -59,6 +59,7 @@ public abstract class DownloadActiveState extends DownloadState { @Override public void onEntry(String prevState, DownloadEvent event, Object evtObj) { + super.onEntry(prevState, event, evtObj); if (s_logger.isTraceEnabled()) { getDownloadListener().log("onEntry, prev state= " + prevState + ", curr state=" + getName() + ", event=" + event, Level.TRACE); } diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 20cd3f97f0c..3d818ab0239 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -115,6 +115,10 @@ import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.InstanceGroupDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotManager; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.QuerySelector; import org.apache.cloudstack.acl.RoleType; @@ -198,6 +202,10 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Inject private SnapshotManager _snapMgr; @Inject + private VMSnapshotManager _vmSnapshotMgr; + @Inject + private VMSnapshotDao _vmSnapshotDao; + @Inject private UserVmManager _vmMgr; @Inject private TemplateManager _tmpltMgr; @@ -727,6 +735,16 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M accountCleanupNeeded = true; } + // Destroy VM Snapshots + List vmSnapshots = _vmSnapshotDao.listByAccountId(Long.valueOf(accountId)); + for (VMSnapshot vmSnapshot : vmSnapshots) { + try { + _vmSnapshotMgr.deleteVMSnapshot(vmSnapshot.getId()); + } catch (Exception e) { + s_logger.debug("Failed to cleanup vm snapshot " + vmSnapshot.getId() + " due to " + e.toString()); + } + } + // Destroy the account's VMs List vms = _userVmDao.listByAccountId(accountId); if (s_logger.isDebugEnabled()) { @@ -1166,6 +1184,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } if (password != null) { + if (password.isEmpty()) { + throw new InvalidParameterValueException("Password cannot be empty"); + } String encodedPassword = null; for (Iterator en = _userPasswordEncoders.iterator(); en.hasNext();) { UserAuthenticator authenticator = en.next(); @@ -1974,7 +1995,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public UserAccount authenticateUser(String username, String password, Long domainId, String loginIpAddress, Map requestParameters) { UserAccount user = null; - if (password != null) { + if (password != null && !password.isEmpty()) { user = getUserAccount(username, password, domainId, requestParameters); } else { String key = _configDao.getValue("security.singlesignon.key");