CLOUDSTACK-6517: IAM - Admin is allowed to create PortFowarding rule for a regular user, when admin does not have " UseEntry" permission for IpAddress.

Changes:
- IAM was applying ordering on accessTypes. Thus if an account had Operate, he got USe access as well. So even if IAM schema did not have 'UseEntry" permission for IpAddress, some other 'OperateEntry' permission on IpAddress was letting this operation go through.
- Fixed IAM to NOT do ordering of access types anymore. IAm will perform strict accessType check only.
- This fix is needed so that admin does not get permission to USE resources from other account just becase he has OPERATE access on those resources due to some other APIs.

- However due to this fix, we break backwards compatibilty with CS 4.3.
- CS 4.3 allowed root admin to do the createPF operation for a user by passing in networkId of the user.
- Same was the case for domain admins within their domains
- Why this worked was due to CS 4.3 simply returning true for root admin/domain admin

- So to maintain backwards compatibilty, we are adding the logic to return "true" for root admin and domain admin just like CS 4.3.
- Exception is: For Network, AffinityGroup and Templates, we still call IAM even for root admin/domain admin, since thats what CS 4.3 did. Just for these 3 resource_types, it used to perform access checks even for root admin/domain admin.
This commit is contained in:
Prachi Damle 2014-04-29 18:32:55 -07:00
parent 42b803dd4b
commit e14c2ec724
9 changed files with 121 additions and 56 deletions

View File

@ -39,6 +39,7 @@ import javax.naming.ConfigurationException;
import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.affinity.AffinityGroup;
import org.apache.cloudstack.affinity.AffinityGroupService;
import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
@ -4326,7 +4327,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
throw new InvalidParameterValueException("Can't update system networks");
}
_accountMgr.checkAccess(caller, null, network);
_accountMgr.checkAccess(caller, AccessType.ListEntry, network);
List<Long> offeringIds = _networkModel.listNetworkOfferingsForUpgrade(networkId);

View File

@ -46,6 +46,7 @@ import org.apache.cloudstack.acl.QuerySelector;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.affinity.AffinityGroup;
import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
import org.apache.cloudstack.api.InternalIdentity;
import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd;
@ -89,6 +90,7 @@ import com.cloud.exception.PermissionDeniedException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.IpAddress;
import com.cloud.network.IpAddressManager;
import com.cloud.network.Network;
import com.cloud.network.VpnUserVO;
import com.cloud.network.as.AutoScaleManager;
import com.cloud.network.dao.AccountGuestVlanMapDao;
@ -123,6 +125,7 @@ import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.storage.snapshot.SnapshotManager;
import com.cloud.template.TemplateManager;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.user.Account.State;
import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.UserAccountDao;
@ -484,13 +487,67 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
}
entityBuf.append("}");
String entityStr = entityBuf.toString();
for (SecurityChecker checker : _securityCheckers) {
if (checker.checkAccess(caller, accessType, apiName, entities)) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Access to " + entityStr + " granted to " + caller + " by " + checker.getName());
boolean isRootAdmin = isRootAdmin(caller.getAccountId());
boolean isDomainAdmin = isDomainAdmin(caller.getAccountId());
boolean isResourceDomainAdmin = isResourceDomainAdmin(caller.getAccountId());
if ((isRootAdmin || isDomainAdmin || isResourceDomainAdmin || caller.getId() == Account.ACCOUNT_ID_SYSTEM)
&& (accessType == null || accessType == AccessType.UseEntry)) {
for (ControlledEntity entity : entities) {
if (entity instanceof VirtualMachineTemplate || entity instanceof Network
|| entity instanceof AffinityGroup) {
for (SecurityChecker checker : _securityCheckers) {
if (checker.checkAccess(caller, accessType, apiName, entity)) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Access to " + entityStr + " granted to " + caller + " by "
+ checker.getName());
}
granted = true;
break;
}
}
} else {
if (isRootAdmin || caller.getId() == Account.ACCOUNT_ID_SYSTEM) {
// no need to make permission checks if the system/root
// admin makes the call
if (s_logger.isTraceEnabled()) {
s_logger.trace("No need to make permission check for System/RootAdmin account, returning true");
}
granted = true;
} else if (isDomainAdmin || isResourceDomainAdmin) {
Domain entityDomain = getEntityDomain(entity);
if (entityDomain != null) {
try {
checkAccess(caller, entityDomain);
granted = true;
} catch (PermissionDeniedException e) {
List<ControlledEntity> entityList = new ArrayList<ControlledEntity>();
entityList.add(entity);
e.addDetails(caller, entityList);
throw e;
}
}
}
}
if (!granted) {
assert false : "How can all of the security checkers pass on checking this check: " + entityStr;
throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to "
+ entityStr);
}
}
} else {
for (SecurityChecker checker : _securityCheckers) {
if (checker.checkAccess(caller, accessType, apiName, entities)) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Access to " + entityStr + " granted to " + caller + " by " + checker.getName());
}
granted = true;
break;
}
granted = true;
break;
}
}
@ -500,6 +557,27 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
}
}
private Domain getEntityDomain(ControlledEntity entity) {
Domain entityDomain = null;
long domainId = entity.getDomainId();
if (domainId != -1) {
entityDomain = _domainMgr.getDomain(domainId);
} else {
if (entity.getAccountId() != -1) {
// If account exists domainId should too so
// calculate
// it. This condition might be hit for templates or
// entities which miss domainId in their tables
Account account = getAccount(entity.getAccountId());
domainId = account != null ? account.getDomainId() : -1;
entityDomain = _domainMgr.getDomain(domainId);
}
}
return entityDomain;
}
@Override
public Long checkAccessAndSpecifyAuthority(Account caller, Long zoneId) {
// We just care for resource domain admin for now. He should be permitted to see only his zone.

View File

@ -247,12 +247,8 @@ public class RoleBasedAPIAccessChecker extends AdapterBase implements APIChecker
try {
cmdObj = (BaseCmd) cmdClass.newInstance();
if (cmdObj instanceof BaseListCmd) {
if (permissionScope == PermissionScope.ACCOUNT) {
accessType = AccessType.UseEntry;
} else {
accessType = AccessType.ListEntry;
addAccountScopedUseEntry = true;
}
accessType = AccessType.ListEntry;
addAccountScopedUseEntry = true;
} else {
accessType = AccessType.OperateEntry;
}

View File

@ -107,22 +107,14 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
permissions = _iamSrv.listPolicyPermissionByActionAndEntity(policy.getId(), action, entityType);
if (permissions.isEmpty()) {
if (accessType != null) {
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(),
type.toString(), entityType));
}
}
permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(),
accessType.toString(), entityType));
}
}
} else {
if (accessType != null) {
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(),
type.toString(), entityType));
}
}
permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(),
accessType.toString(), entityType));
}
}
for (IAMPolicyPermission permission : permissions) {

View File

@ -57,11 +57,9 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe
List<Long> domainIds = new ArrayList<Long>();
for (IAMPolicy policy : policies) {
List<IAMPolicyPermission> pp = new ArrayList<IAMPolicyPermission>();
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.DOMAIN.toString(), type));
}
}
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action,
PermissionScope.DOMAIN.toString(), accessType.toString()));
if (pp != null) {
for (IAMPolicyPermission p : pp) {
if (p.getScopeId() != null) {
@ -101,11 +99,9 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe
List<Long> accountIds = new ArrayList<Long>();
for (IAMPolicy policy : policies) {
List<IAMPolicyPermission> pp = new ArrayList<IAMPolicyPermission>();
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ACCOUNT.toString(), type));
}
}
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action,
PermissionScope.ACCOUNT.toString(), accessType.toString()));
if (pp != null) {
for (IAMPolicyPermission p : pp) {
if (p.getScopeId() != null) {
@ -144,11 +140,9 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe
List<Long> entityIds = new ArrayList<Long>();
for (IAMPolicy policy : policies) {
List<IAMPolicyPermission> pp = new ArrayList<IAMPolicyPermission>();
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.RESOURCE.toString(), type));
}
}
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action,
PermissionScope.RESOURCE.toString(), accessType.toString()));
if (pp != null) {
for (IAMPolicyPermission p : pp) {
if (p.getScopeId() != null) {
@ -171,13 +165,10 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe
// for each policy, find granted permission with ALL scope
for (IAMPolicy policy : policies) {
List<IAMPolicyPermission> pp = new ArrayList<IAMPolicyPermission>();
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ALL.toString(), type));
}
if (pp != null && pp.size() > 0) {
return true;
}
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ALL.toString(),
accessType.toString()));
if (pp != null && pp.size() > 0) {
return true;
}
}
return false;

View File

@ -18,7 +18,7 @@ package org.apache.cloudstack.iam.api;
import java.util.List;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.iam.api.IAMPolicyPermission.Permission;
import com.cloud.utils.Pair;
@ -73,7 +73,7 @@ public interface IAMService {
List<IAMPolicyPermission> listPolicyPermissions(long policyId);
List<IAMPolicyPermission> listPolicyPermissionsByScope(long policyId, String action, String scope, AccessType accessType);
List<IAMPolicyPermission> listPolicyPermissionsByScope(long policyId, String action, String scope, String accessType);
List<IAMPolicyPermission> listPolicyPermissionByActionAndEntity(long policyId, String action, String entityType);

View File

@ -25,7 +25,6 @@ import javax.inject.Inject;
import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.PermissionScope;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.iam.api.IAMGroup;
import org.apache.cloudstack.iam.api.IAMPolicy;
import org.apache.cloudstack.iam.api.IAMPolicyPermission;
@ -632,7 +631,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
}
// add entry in acl_policy_permission table
IAMPolicyPermissionVO permit = _policyPermissionDao.findByPolicyAndEntity(iamPolicyId, entityType, scope, scopeId, action, perm);
IAMPolicyPermissionVO permit = _policyPermissionDao.findByPolicyAndEntity(iamPolicyId, entityType, scope,
scopeId, action, perm, accessType);
if (permit == null) {
// not there already
permit = new IAMPolicyPermissionVO(iamPolicyId, action, entityType, accessType, scope, scopeId, perm,
@ -654,7 +654,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
+ "; failed to revoke permission from policy.");
}
// remove entry from acl_entity_permission table
IAMPolicyPermissionVO permit = _policyPermissionDao.findByPolicyAndEntity(iamPolicyId, entityType, scope, scopeId, action, Permission.Allow);
IAMPolicyPermissionVO permit = _policyPermissionDao.findByPolicyAndEntity(iamPolicyId, entityType, scope,
scopeId, action, Permission.Allow, null);
if (permit != null) {
// not removed yet
_policyPermissionDao.remove(permit.getId());
@ -766,9 +767,10 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
@SuppressWarnings("unchecked")
@Override
public List<IAMPolicyPermission> listPolicyPermissionsByScope(long policyId, String action, String scope, AccessType accessType) {
public List<IAMPolicyPermission> listPolicyPermissionsByScope(long policyId, String action, String scope,
String accessType) {
@SuppressWarnings("rawtypes")
List pp = _policyPermissionDao.listByPolicyActionAndScope(policyId, action, scope, accessType.toString());
List pp = _policyPermissionDao.listByPolicyActionAndScope(policyId, action, scope, accessType);
return pp;
}

View File

@ -27,7 +27,7 @@ public interface IAMPolicyPermissionDao extends GenericDao<IAMPolicyPermissionVO
List<IAMPolicyPermissionVO> listByPolicy(long policyId);
IAMPolicyPermissionVO findByPolicyAndEntity(long policyId, String entityType, String scope, Long scopeId,
String action, Permission perm);
String action, Permission perm, String accessType);
List<IAMPolicyPermissionVO> listByPolicyActionAndScope(long policyId, String action, String scope, String accessType);

View File

@ -70,7 +70,7 @@ public class IAMPolicyPermissionDaoImpl extends GenericDaoBase<IAMPolicyPermissi
@Override
public IAMPolicyPermissionVO findByPolicyAndEntity(long policyId, String entityType, String scope, Long scopeId,
String action, Permission perm) {
String action, Permission perm, String accessType) {
SearchCriteria<IAMPolicyPermissionVO> sc = fullSearch.create();
sc.setParameters("policyId", policyId);
sc.setParameters("entityType", entityType);
@ -78,6 +78,11 @@ public class IAMPolicyPermissionDaoImpl extends GenericDaoBase<IAMPolicyPermissi
sc.setParameters("scopeId", scopeId);
sc.setParameters("action", action);
sc.setParameters("permission", perm);
if (accessType != null) {
// accessType can be optional, used mainly in list apis with
// ListEntry and UseEntry distinction
sc.setParameters("accessType", accessType);
}
return findOneBy(sc);
}