server: Some APIs should have access check (#4859)

This PR fixes the CLOUDSTACK-10434. I think some APIs lack access check and list them in below table. I also give the pattch to add the access check for the api in this table. Anyone chould change this table, If you think the APIs do not need access check and change their lable as "no".

API	Lack?
VolumeApiServiceImpl # updateVolume	yes
VolumeApiServiceImpl # detachVolumeViaDestroyVM	yes
VolumeApiServiceImpl # takeSnapshot	yes
VolumeApiServiceImpl # migrateVolume	yes
AccountManagerImpl#createApiKeyAndSecretKey	yes
LoadBalancingRulesManagerImpl#applyLBStickinessPolicy	yes
LoadBalancingRulesManagerImpl#applyLBHealthCheckPolicy	yes
TemplateManagerImpl#createPrivateTemplate	yes
SnapshotManagerImpl#updateSnapshotPolicy

Co-authored-by: lujie <lujie@foxmail.com>
This commit is contained in:
lujiefsi 2021-04-26 16:09:03 +08:00 committed by GitHub
parent f9ca881ebe
commit f8ba33d570
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 40 additions and 1 deletions

View File

@ -663,6 +663,9 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements
if (loadBalancer == null) {
throw new InvalidParameterException("Invalid Load balancer Id:" + cmd.getLbRuleId());
}
_accountMgr.checkAccess(CallContext.current().getCallingAccount(), null, true, loadBalancer);
List<LBStickinessPolicyVO> stickinessPolicies = _lb2stickinesspoliciesDao.listByLoadBalancerId(cmd.getLbRuleId(), false);
for (LBStickinessPolicyVO stickinessPolicy : stickinessPolicies) {
if (stickinessPolicy.getId() == cmd.getEntityId()) {
@ -717,6 +720,7 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements
if (loadBalancer == null) {
throw new InvalidParameterException("Invalid Load balancer Id:" + cmd.getLbRuleId());
}
_accountMgr.checkAccess(CallContext.current().getCallingAccount(), null, true, loadBalancer);
FirewallRule.State backupState = loadBalancer.getState();
loadBalancer.setState(FirewallRule.State.Add);
_lbDao.persist(loadBalancer);

View File

@ -1783,13 +1783,16 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
@Override
@ActionEvent(eventType = EventTypes.EVENT_VOLUME_UPDATE, eventDescription = "updating volume", async = true)
public Volume updateVolume(long volumeId, String path, String state, Long storageId, Boolean displayVolume, String customId, long entityOwnerId, String chainInfo) {
Account caller = CallContext.current().getCallingAccount();
VolumeVO volume = _volsDao.findById(volumeId);
if (volume == null) {
throw new InvalidParameterValueException("The volume id doesn't exist");
}
/* Does the caller have authority to act on this volume? */
_accountMgr.checkAccess(caller, null, true, volume);
if (path != null) {
volume.setPath(path);
}
@ -2013,6 +2016,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
@ActionEvent(eventType = EventTypes.EVENT_VOLUME_DETACH, eventDescription = "detaching volume")
public Volume detachVolumeViaDestroyVM(long vmId, long volumeId) {
Account caller = CallContext.current().getCallingAccount();
Volume volume = _volsDao.findById(volumeId);
// Permissions check
_accountMgr.checkAccess(caller, null, true, volume);
return orchestrateDetachVolumeFromVM(vmId, volumeId);
}
@ -2202,6 +2209,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
@Override
@ActionEvent(eventType = EventTypes.EVENT_VOLUME_MIGRATE, eventDescription = "migrating volume", async = true)
public Volume migrateVolume(MigrateVolumeCmd cmd) {
Account caller = CallContext.current().getCallingAccount();
Long volumeId = cmd.getVolumeId();
Long storagePoolId = cmd.getStoragePoolId();
@ -2210,6 +2218,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
throw new InvalidParameterValueException("Failed to find the volume id: " + volumeId);
}
_accountMgr.checkAccess(caller, null, true, vol);
if (vol.getState() != Volume.State.Ready) {
throw new InvalidParameterValueException("Volume must be in ready state");
}
@ -2575,11 +2585,14 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
private Snapshot takeSnapshotInternal(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup)
throws ResourceAllocationException {
Account caller = CallContext.current().getCallingAccount();
VolumeInfo volume = volFactory.getVolume(volumeId);
if (volume == null) {
throw new InvalidParameterValueException("Creating snapshot failed due to volume:" + volumeId + " doesn't exist");
}
_accountMgr.checkAccess(caller, null, true, volume);
if (volume.getState() != Volume.State.Ready) {
throw new InvalidParameterValueException("VolumeId: " + volumeId + " is not in " + Volume.State.Ready + " state but " + volume.getState() + ". Cannot take snapshot.");
}
@ -2596,6 +2609,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
}
if (vm != null) {
_accountMgr.checkAccess(caller, null, true, vm);
// serialize VM operation
AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
if (jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {

View File

@ -335,6 +335,14 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
Boolean display = cmd.getDisplay();
SnapshotPolicyVO policyVO = _snapshotPolicyDao.findById(id);
VolumeInfo volume = volFactory.getVolume(policyVO.getVolumeId());
if (volume == null) {
throw new InvalidParameterValueException("No such volume exist");
}
// does the caller have the authority to act on this volume
_accountMgr.checkAccess(CallContext.current().getCallingAccount(), null, true, volume);
if (display != null) {
boolean previousDisplay = policyVO.isDisplay();
policyVO.setDisplay(display);

View File

@ -1648,6 +1648,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
final Long accountId = CallContext.current().getCallingAccountId();
SnapshotVO snapshot = null;
VolumeVO volume = null;
Account caller = CallContext.current().getCallingAccount();
try {
TemplateInfo tmplInfo = _tmplFactory.getTemplate(templateId, DataStoreRole.Image);
@ -1686,6 +1687,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
throw new CloudRuntimeException("Cannot find snapshot " + snapshotId + " on secondary and could not create backup");
}
}
_accountMgr.checkAccess(caller, null, true, snapInfo);
DataStore snapStore = snapInfo.getDataStore();
if (snapStore != null) {
@ -1696,7 +1698,11 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
future = _tmpltSvr.createTemplateFromSnapshotAsync(snapInfo, tmplInfo, store);
} else if (volumeId != null) {
VolumeInfo volInfo = _volFactory.getVolume(volumeId);
if (volInfo == null) {
throw new InvalidParameterValueException("No such volume exist");
}
_accountMgr.checkAccess(caller, null, true, volInfo);
future = _tmpltSvr.createTemplateFromVolumeAsync(volInfo, tmplInfo, store);
} else {
throw new CloudRuntimeException("Creating private Template need to specify snapshotId or volumeId");

View File

@ -2487,10 +2487,13 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
@DB
@ActionEvent(eventType = EventTypes.EVENT_REGISTER_FOR_SECRET_API_KEY, eventDescription = "register for the developer API keys")
public String[] createApiKeyAndSecretKey(final long userId) {
Account caller = getCurrentCallingAccount();
User user = getUserIncludingRemoved(userId);
if (user == null) {
throw new InvalidParameterValueException("Unable to find user by id");
}
Account account = _accountDao.findById(user.getAccountId());
checkAccess(caller, null, true, account);
final String[] keys = new String[2];
Transaction.execute(new TransactionCallbackNoReturn() {
@Override

View File

@ -56,6 +56,7 @@ import com.cloud.deployasis.dao.UserVmDeployAsIsDetailsDao;
import com.cloud.exception.UnsupportedServiceException;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.deployasis.dao.TemplateDeployAsIsDetailsDao;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.affinity.AffinityGroupService;
@ -2966,6 +2967,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
checkForUnattachedVolumes(vmId, volumesToBeDeleted);
validateVolumes(volumesToBeDeleted);
final ControlledEntity[] volumesToDelete = volumesToBeDeleted.toArray(new ControlledEntity[0]);
_accountMgr.checkAccess(ctx.getCallingAccount(), null, true, volumesToDelete);
stopVirtualMachine(vmId, VmDestroyForcestop.value());
detachVolumesFromVm(volumesToBeDeleted);