Repository: cloudstack Updated Branches: refs/heads/4.4-forward 515fa261b -> 500c99eef
CLOUDSTACK-6600:IAM Security checker needs to have cache to improve checkAccess performance. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/500c99ee Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/500c99ee Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/500c99ee Branch: refs/heads/4.4-forward Commit: 500c99eef7f7a04d95ed95697a2678fef283e61d Parents: 515fa26 Author: Min Chen <[email protected]> Authored: Wed May 7 16:42:19 2014 -0700 Committer: Min Chen <[email protected]> Committed: Wed May 7 16:44:20 2014 -0700 ---------------------------------------------------------------------- .../iam/RoleBasedEntityAccessChecker.java | 98 ++++++++++++++++++-- services/iam/server/pom.xml | 4 + .../core/spring-iam-server-context.xml | 9 +- .../apache/cloudstack/iam/api/IAMService.java | 8 +- .../cloudstack/iam/server/IAMServiceImpl.java | 88 +++++++++++++++++- 5 files changed, 193 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/500c99ee/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java ---------------------------------------------------------------------- diff --git a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java index 63aa827..eaa4302 100644 --- a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java +++ b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java @@ -59,6 +59,22 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur return checkAccess(caller, entity, accessType, null); } + private String buildAccessCacheKey(Account caller, ControlledEntity entity, AccessType accessType, String action) { + StringBuffer key = new StringBuffer(); + key.append(caller.getAccountId()); + key.append("-"); + String entityType = null; + if (entity != null && entity.getEntityType() != null) { + entityType = entity.getEntityType().getSimpleName(); + } + key.append(entityType != null ? entityType : "null"); + key.append("-"); + key.append(accessType != null ? accessType.toString() : "null"); + key.append("-"); + key.append(action != null ? action : "null"); + return key.toString(); + } + @Override public boolean checkAccess(Account caller, ControlledEntity entity, AccessType accessType, String action) throws PermissionDeniedException { @@ -66,24 +82,46 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur if (caller == null) { throw new InvalidParameterValueException("Caller cannot be passed as NULL to IAM!"); } + + if (entity == null && action == null) { + throw new InvalidParameterValueException("Entity and action cannot be both NULL in checkAccess!"); + } + + // check IAM cache first + String accessKey = buildAccessCacheKey(caller, entity, accessType, action); + CheckAccessResult allowDeny = (CheckAccessResult)_iamSrv.getFromIAMCache(accessKey); + if (allowDeny != null) { + s_logger.debug("IAM access check for " + accessKey + " from cache"); + if (allowDeny.isAllow()) { + return true; + } else { + if (allowDeny.getDenyMsg() != null) { + throw new PermissionDeniedException(allowDeny.getDenyMsg()); + } else { + return false; + } + } + } + if (entity == null && action != null) { // check if caller can do this action List<IAMPolicy> policies = _iamSrv.listIAMPolicies(caller.getAccountId()); boolean isAllowed = _iamSrv.isActionAllowedForPolicies(action, policies); if (!isAllowed) { - throw new PermissionDeniedException("The action '" + action + "' not allowed for account " + caller); + String msg = "The action '" + action + "' not allowed for account " + caller; + _iamSrv.addToIAMCache(accessKey, new CheckAccessResult(msg)); + throw new PermissionDeniedException(msg); } + _iamSrv.addToIAMCache(accessKey, new CheckAccessResult(true)); return true; } - if (entity == null) { - throw new InvalidParameterValueException("Entity and action cannot be both NULL in checkAccess!"); - } // if a Project entity, skip Account entityAccount = _accountService.getAccount(entity.getAccountId()); if (entityAccount != null && entityAccount.getType() == Account.ACCOUNT_TYPE_PROJECT) { + _iamSrv.addToIAMCache(accessKey, new CheckAccessResult(false)); return false; } @@ -96,8 +134,8 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur accessType = AccessType.UseEntry; } - // get all Policies of this caller w.r.t the entity - List<IAMPolicy> policies = getEffectivePolicies(caller, entity); + // get all Policies of this caller by considering recursive domain group policy + List<IAMPolicy> policies = getEffectivePolicies(caller); HashMap<IAMPolicy, Boolean> policyPermissionMap = new HashMap<IAMPolicy, Boolean>(); for (IAMPolicy policy : policies) { @@ -128,6 +166,7 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur } } if (policyPermissionMap.containsKey(policy) && policyPermissionMap.get(policy)) { + _iamSrv.addToIAMCache(accessKey, new CheckAccessResult(true)); return true; } } @@ -135,13 +174,16 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur if (!policies.isEmpty()) { // Since we reach this point, none of the // roles granted access + String msg = "Account " + caller + " does not have permission to access resource " + entity + + " for access type: " + accessType; if (s_logger.isDebugEnabled()) { - s_logger.debug("Account " + caller + " does not have permission to access resource " + entity - + " for access type: " + accessType); + s_logger.debug(msg); } - throw new PermissionDeniedException(caller + " does not have permission to access resource " + entity); + _iamSrv.addToIAMCache(accessKey, new CheckAccessResult(msg)); + throw new PermissionDeniedException(msg); } + _iamSrv.addToIAMCache(accessKey, new CheckAccessResult(false)); return false; } @@ -225,7 +267,7 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur return false; } - private List<IAMPolicy> getEffectivePolicies(Account caller, ControlledEntity entity) { + private List<IAMPolicy> getEffectivePolicies(Account caller) { List<IAMPolicy> policies = _iamSrv.listIAMPolicies(caller.getId()); @@ -240,4 +282,40 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur return policies; } + + private class CheckAccessResult { + boolean allow; + String denyMsg; + + public CheckAccessResult(boolean allow) { + this(allow, null); + } + + public CheckAccessResult(String msg) { + this(false, msg); + } + + public CheckAccessResult(boolean allow, String msg) { + allow = allow; + denyMsg = msg; + } + + public boolean isAllow() { + return allow; + } + + public void setAllow(boolean allow) { + this.allow = allow; + } + + + public String getDenyMsg() { + return denyMsg; + } + + public void setDenyMsg(String denyMsg) { + this.denyMsg = denyMsg; + } + + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/500c99ee/services/iam/server/pom.xml ---------------------------------------------------------------------- diff --git a/services/iam/server/pom.xml b/services/iam/server/pom.xml index bed8811..77b2522 100644 --- a/services/iam/server/pom.xml +++ b/services/iam/server/pom.xml @@ -32,6 +32,10 @@ <artifactId>commons-io</artifactId> </dependency> <dependency> + <groupId>net.sf.ehcache</groupId> + <artifactId>ehcache-core</artifactId> + </dependency> + <dependency> <groupId>org.apache.cloudstack</groupId> <artifactId>cloud-utils</artifactId> <version>${project.version}</version> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/500c99ee/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml ---------------------------------------------------------------------- diff --git a/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml b/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml index c9f383f..4994a34 100644 --- a/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml +++ b/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml @@ -35,6 +35,13 @@ <bean id="IAMAccountPolicyMapDaoImpl" class="org.apache.cloudstack.iam.server.dao.IAMAccountPolicyMapDaoImpl" /> - <bean id="IAMServiceImpl" class="org.apache.cloudstack.iam.server.IAMServiceImpl" /> + <bean id="IAMServiceImpl" class="org.apache.cloudstack.iam.server.IAMServiceImpl" > + <property name="configParams"> + <map> + <entry key="cache.size" value="5000" /> + <entry key="cache.time.to.live" value="300" /> + </map> + </property> + </bean> </beans> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/500c99ee/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java ---------------------------------------------------------------------- diff --git a/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java b/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java index 20326e97..3a470ee 100644 --- a/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java +++ b/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java @@ -18,7 +18,6 @@ package org.apache.cloudstack.iam.api; import java.util.List; - import org.apache.cloudstack.iam.api.IAMPolicyPermission.Permission; import com.cloud.utils.Pair; @@ -90,4 +89,11 @@ public interface IAMService { List<IAMPolicy> listRecursiveIAMPoliciesByGroup(long groupId); + /* Interface used for cache IAM checkAccess result */ + void addToIAMCache(Object accessKey, Object allowDeny); + + Object getFromIAMCache(Object accessKey); + + void invalidateIAMCache(); + } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/500c99ee/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java ---------------------------------------------------------------------- diff --git a/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java b/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java index c35ac1d..796ae43 100644 --- a/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java +++ b/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java @@ -18,9 +18,15 @@ package org.apache.cloudstack.iam.server; import java.util.ArrayList; import java.util.List; +import java.util.Map; import javax.ejb.Local; import javax.inject.Inject; +import javax.naming.ConfigurationException; + +import net.sf.ehcache.Cache; +import net.sf.ehcache.CacheManager; +import net.sf.ehcache.Element; import org.apache.log4j.Logger; @@ -38,6 +44,7 @@ import org.apache.cloudstack.iam.server.dao.IAMPolicyDao; import org.apache.cloudstack.iam.server.dao.IAMPolicyPermissionDao; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; import com.cloud.utils.component.Manager; import com.cloud.utils.component.ManagerBase; @@ -82,6 +89,62 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { @Inject IAMPolicyPermissionDao _policyPermissionDao; + private Cache _iamCache; + + private void createIAMCache(final Map<String, ? extends Object> params) { + final String value = (String)params.get("cache.size"); + + if (value != null) { + final CacheManager cm = CacheManager.create(); + final int maxElements = NumbersUtil.parseInt(value, 0); + final int live = NumbersUtil.parseInt((String)params.get("cache.time.to.live"), 300); + final int idle = NumbersUtil.parseInt((String)params.get("cache.time.to.idle"), 300); + _iamCache = new Cache(getName(), maxElements, false, live == -1, live == -1 ? Integer.MAX_VALUE : live, idle); + cm.addCache(_iamCache); + s_logger.info("IAM Cache created: " + _iamCache.toString()); + } else { + _iamCache = null; + } + } + + @Override + public void addToIAMCache(Object accessKey, Object allowDeny) { + if (_iamCache != null) { + try { + s_logger.debug("Put IAM access check for " + accessKey + " in cache"); + _iamCache.put(new Element(accessKey, allowDeny)); + } catch (final Exception e) { + s_logger.debug("Can't put " + accessKey + " to IAM cache", e); + } + } + } + + @Override + public void invalidateIAMCache() { + //This may need to use event bus to publish to other MS, but event bus now is missing this functionality to handle PublishScope.GLOBAL + if (_iamCache != null) { + s_logger.debug("Invalidate IAM cache"); + _iamCache.removeAll(); + } + } + + @Override + public Object getFromIAMCache(Object accessKey) { + if (_iamCache != null) { + final Element element = _iamCache.get(accessKey); + return element == null ? null : element.getObjectValue(); + } + return null; + } + + @Override + public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException { + boolean result = super.configure(name, params); + // create IAM cache + createIAMCache(params); + return result; + } + @DB @Override public IAMGroup createIAMGroup(String iamGroupName, String description, String path) { @@ -111,7 +174,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { Transaction.execute(new TransactionCallbackNoReturn() { @Override public void doInTransactionWithoutResult(TransactionStatus status) { - // remove this group related entry in acl_group_role_map + // remove this group related entry in acl_group_policy_map List<IAMGroupPolicyMapVO> groupPolicyMap = _aclGroupPolicyMapDao.listByGroupId(grp.getId()); if (groupPolicyMap != null) { for (IAMGroupPolicyMapVO gr : groupPolicyMap) { @@ -132,6 +195,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } }); + invalidateIAMCache(); return true; } @@ -184,6 +248,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); return group; } @@ -210,6 +276,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); return group; } @@ -345,7 +413,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { Transaction.execute(new TransactionCallbackNoReturn() { @Override public void doInTransactionWithoutResult(TransactionStatus status) { - // remove this role related entry in acl_group_role_map + // remove this policy related entry in acl_group_policy_map List<IAMGroupPolicyMapVO> groupPolicyMap = _aclGroupPolicyMapDao.listByPolicyId(policy.getId()); if (groupPolicyMap != null) { for (IAMGroupPolicyMapVO gr : groupPolicyMap) { @@ -374,6 +442,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } }); + invalidateIAMCache(); + return true; } @@ -536,6 +606,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } }); + invalidateIAMCache(); return group; } @@ -568,6 +639,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); return group; } @@ -594,6 +667,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); } @Override @@ -617,6 +692,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); } @DB @@ -639,6 +716,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { recursive); _policyPermissionDao.persist(permit); } + + invalidateIAMCache(); return policy; } @@ -660,6 +739,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { // not removed yet _policyPermissionDao.remove(permit.getId()); } + + invalidateIAMCache(); return policy; } @@ -682,6 +763,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); } @DB @@ -702,6 +785,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { permissionSC.setParameters("policyId", iamPolicyId); _policyPermissionDao.expunge(permissionSC); + invalidateIAMCache(); return policy; }
