From b51a7fec505f817d45bb681310d712e7add620cb Mon Sep 17 00:00:00 2001 From: abhishek Date: Tue, 30 Nov 2010 17:46:31 -0800 Subject: [PATCH] adding a security checker whilst creating a volume from a disk offering --- api/src/com/cloud/acl/SecurityChecker.java | 3 ++ server/src/com/cloud/acl/DomainChecker.java | 47 +++++++++++++++++++ .../configuration/ConfigurationManager.java | 6 ++- .../ConfigurationManagerImpl.java | 19 +++++++- .../cloud/server/ManagementServerImpl.java | 17 +++++-- .../com/cloud/storage/StorageManagerImpl.java | 6 +++ 6 files changed, 93 insertions(+), 5 deletions(-) diff --git a/api/src/com/cloud/acl/SecurityChecker.java b/api/src/com/cloud/acl/SecurityChecker.java index e745607dab4..53b8438b8f0 100644 --- a/api/src/com/cloud/acl/SecurityChecker.java +++ b/api/src/com/cloud/acl/SecurityChecker.java @@ -6,6 +6,7 @@ package com.cloud.acl; import com.cloud.dc.DataCenter; import com.cloud.domain.Domain; import com.cloud.exception.PermissionDeniedException; +import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; import com.cloud.user.Account; import com.cloud.user.User; @@ -59,6 +60,8 @@ public interface SecurityChecker extends Adapter { boolean checkAccess(Account account, DataCenter zone) throws PermissionDeniedException; public boolean checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException; + + boolean checkAccess(Account account, DiskOffering dof) throws PermissionDeniedException; // We should be able to use this method to check against commands. For example, we can // annotate the command with access annotations and this method can use it to extract diff --git a/server/src/com/cloud/acl/DomainChecker.java b/server/src/com/cloud/acl/DomainChecker.java index a9aacddc5bb..7af3f8b42d8 100644 --- a/server/src/com/cloud/acl/DomainChecker.java +++ b/server/src/com/cloud/acl/DomainChecker.java @@ -25,6 +25,7 @@ import com.cloud.domain.Domain; import com.cloud.domain.DomainVO; import com.cloud.domain.dao.DomainDao; import com.cloud.exception.PermissionDeniedException; +import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; import com.cloud.storage.LaunchPermissionVO; import com.cloud.storage.dao.LaunchPermissionDao; @@ -101,6 +102,52 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { return checkAccess(account, entity); } + @Override + public boolean checkAccess(Account account, DiskOffering dof) throws PermissionDeniedException + { + if(account == null || dof.getDomainId() == null) + {//public offering + return true; + } + else + { + //admin has all permissions + if(account.getType() == Account.ACCOUNT_TYPE_ADMIN) + { + return true; + } + //if account is normal user or domain admin + //check if account's domain is a child of zone's domain (Note: This is made consistent with the list command for disk offering) + else if(account.getType() == Account.ACCOUNT_TYPE_NORMAL || account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) + { + if(account.getDomainId() == dof.getDomainId()) + { + return true; //disk offering and account at exact node + } + else + { + DomainVO domainRecord = _domainDao.findById(account.getDomainId()); + if(domainRecord != null) + { + while(true) + { + if(domainRecord.getId() == dof.getDomainId()) + { + //found as a child + return true; + } + if(domainRecord.getParent() != null) + domainRecord = _domainDao.findById(domainRecord.getParent()); + else + break; + } + } + } + } + } + //not found + return false; + } @Override public boolean checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException diff --git a/server/src/com/cloud/configuration/ConfigurationManager.java b/server/src/com/cloud/configuration/ConfigurationManager.java index a1cee1a56b8..f05e478226a 100644 --- a/server/src/com/cloud/configuration/ConfigurationManager.java +++ b/server/src/com/cloud/configuration/ConfigurationManager.java @@ -26,6 +26,7 @@ import com.cloud.dc.HostPodVO; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientAddressCapacityException; import com.cloud.exception.PermissionDeniedException; +import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; import com.cloud.service.ServiceOfferingVO; import com.cloud.storage.DiskOfferingVO; @@ -155,6 +156,9 @@ public interface ConfigurationManager extends Manager { void checkAccess(Account caller, DataCenter zone) throws PermissionDeniedException; - void checkAccess(Account caller, ServiceOffering so) + void checkServiceOfferingAccess(Account caller, ServiceOffering so) throws PermissionDeniedException; + + void checkDiskOfferingAccess(Account caller, DiskOffering dof) + throws PermissionDeniedException; } diff --git a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java index 170ae7f55ca..9f3e82439bd 100755 --- a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java @@ -2320,7 +2320,24 @@ public class ConfigurationManagerImpl implements ConfigurationManager, Configura } @Override - public void checkAccess(Account caller, ServiceOffering so) throws PermissionDeniedException { + public void checkDiskOfferingAccess(Account caller, DiskOffering dof) throws PermissionDeniedException { + for (SecurityChecker checker : _secChecker) { + if (checker.checkAccess(caller, dof)) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Access granted to " + caller + " to disk offering:" + dof.getId() + " by " + checker.getName()); + } + return; + }else{ + throw new PermissionDeniedException("Access denied to "+caller+" by "+checker.getName()); + } + } + + assert false : "How can all of the security checkers pass on checking this caller?"; + throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to disk offering:" + dof.getId()); + } + + @Override + public void checkServiceOfferingAccess(Account caller, ServiceOffering so) throws PermissionDeniedException { for (SecurityChecker checker : _secChecker) { if (checker.checkAccess(caller, so)) { if (s_logger.isDebugEnabled()) { diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index a9dedba7524..d9a4b9c5b86 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -1200,9 +1200,9 @@ public class ManagementServerImpl implements ManagementServer { //do nothing as offering is public }else{ if(userAccount != null){ - _configMgr.checkAccess(userAccount, offering);//user deploying his own vm + _configMgr.checkServiceOfferingAccess(userAccount, offering);//user deploying his own vm }else{ - _configMgr.checkAccess(ctxAccount, offering); + _configMgr.checkServiceOfferingAccess(ctxAccount, offering); } } @@ -1230,6 +1230,18 @@ public class ManagementServerImpl implements ManagementServer { throw new InvalidParameterValueException("Please specify a valid disk offering ID."); } + if(diskOffering != null){ + if(diskOffering.getDomainId() == null){ + //do nothing as offering is public + }else{ + if(userAccount != null){ + _configMgr.checkDiskOfferingAccess(userAccount, diskOffering);//user deploying his own vm + }else{ + _configMgr.checkDiskOfferingAccess(ctxAccount, diskOffering); + } + } + } + if (isIso) { /*iso template doesn;t have hypervisor type, temporarily set it's type as user specified, pass it to storage allocator */ template.setHypervisorType(HypervisorType.getType(cmd.getHypervisor())); @@ -4416,7 +4428,6 @@ public class ManagementServerImpl implements ManagementServer { Object id = cmd.getId(); Object keyword = cmd.getKeyword(); Long domainId = cmd.getDomainId(); - //Keeping this logic consistent with domain specific zones //if a domainId is provided, we just return the disk offering associated with this domain if(domainId != null){ diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index 767327d85c6..5a840d2b551 100755 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -1742,6 +1742,12 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag throw new InvalidParameterValueException("Please specify a valid disk offering."); } + if(diskOffering.getDomainId() == null){ + //do nothing as offering is public + }else{ + _configMgr.checkDiskOfferingAccess(account, diskOffering); + } + if(!validateVolumeSizeRange(diskOffering.getDiskSize()/1024)){//convert size from mb to gb for validation throw new InvalidParameterValueException("Invalid size for custom volume creation: " + size+" ,max volume size is:"+_maxVolumeSizeInGb); }