Return-Path: X-Original-To: apmail-cloudstack-commits-archive@www.apache.org Delivered-To: apmail-cloudstack-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 7FDFA11818 for ; Wed, 14 May 2014 23:24:23 +0000 (UTC) Received: (qmail 36826 invoked by uid 500); 10 May 2014 22:00:41 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 93749 invoked by uid 500); 10 May 2014 21:57:53 -0000 Mailing-List: contact commits-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list commits@cloudstack.apache.org Received: (qmail 59654 invoked by uid 99); 10 May 2014 21:56:16 -0000 Received: from Unknown (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 10 May 2014 21:56:16 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 235D4942B81; Fri, 9 May 2014 00:56:31 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: mchen@apache.org To: commits@cloudstack.apache.org Date: Fri, 09 May 2014 00:56:32 -0000 Message-Id: <4aca1456ad5c42279b36b0c874de5e31@git.apache.org> In-Reply-To: <164ed0d765e0417bb696f5db8c8a5a1f@git.apache.org> References: <164ed0d765e0417bb696f5db8c8a5a1f@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [2/2] git commit: updated refs/heads/master to 51cb0f9 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/218158b9 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/218158b9 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/218158b9 Branch: refs/heads/master Commit: 218158b9ab5f2d113287874d0ba3dd0f87fc2f2c Parents: f21e527 Author: Min Chen Authored: Thu May 8 15:37:05 2014 -0700 Committer: Min Chen Committed: Thu May 8 17:56: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 | 7 ++ .../cloudstack/iam/server/IAMServiceImpl.java | 88 +++++++++++++++++- 5 files changed, 193 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/218158b9/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 cc29ab5..b384d7c 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 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 policies = getEffectivePolicies(caller, entity); + // get all Policies of this caller by considering recursive domain group policy + List policies = getEffectivePolicies(caller); HashMap policyPermissionMap = new HashMap(); for (IAMPolicy policy : policies) { @@ -136,6 +174,7 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur } } if (policyPermissionMap.containsKey(policy) && policyPermissionMap.get(policy)) { + _iamSrv.addToIAMCache(accessKey, new CheckAccessResult(true)); return true; } } @@ -143,13 +182,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; } @@ -233,7 +275,7 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur return false; } - private List getEffectivePolicies(Account caller, ControlledEntity entity) { + private List getEffectivePolicies(Account caller) { List policies = _iamSrv.listIAMPolicies(caller.getId()); @@ -248,4 +290,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/218158b9/services/iam/server/pom.xml ---------------------------------------------------------------------- diff --git a/services/iam/server/pom.xml b/services/iam/server/pom.xml index 8b05111..42daa33 100644 --- a/services/iam/server/pom.xml +++ b/services/iam/server/pom.xml @@ -32,6 +32,10 @@ commons-io + net.sf.ehcache + ehcache-core + + org.apache.cloudstack cloud-utils ${project.version} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/218158b9/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 @@ - + + + + + + + + http://git-wip-us.apache.org/repos/asf/cloudstack/blob/218158b9/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 c396fa9..0c0e9c6 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 @@ -90,4 +90,11 @@ public interface IAMService { List 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/218158b9/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 d6a61a1..33869c8 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; @@ -39,6 +45,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; @@ -83,6 +90,62 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { @Inject IAMPolicyPermissionDao _policyPermissionDao; + private Cache _iamCache; + + private void createIAMCache(final Map 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 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) { @@ -112,7 +175,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 groupPolicyMap = _aclGroupPolicyMapDao.listByGroupId(grp.getId()); if (groupPolicyMap != null) { for (IAMGroupPolicyMapVO gr : groupPolicyMap) { @@ -133,6 +196,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } }); + invalidateIAMCache(); return true; } @@ -185,6 +249,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); return group; } @@ -211,6 +277,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); return group; } @@ -346,7 +414,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 groupPolicyMap = _aclGroupPolicyMapDao.listByPolicyId(policy.getId()); if (groupPolicyMap != null) { for (IAMGroupPolicyMapVO gr : groupPolicyMap) { @@ -375,6 +443,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } }); + invalidateIAMCache(); + return true; } @@ -537,6 +607,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } }); + invalidateIAMCache(); return group; } @@ -569,6 +640,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); return group; } @@ -595,6 +668,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); } @Override @@ -618,6 +693,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; } @@ -659,6 +738,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { // not removed yet _policyPermissionDao.remove(permit.getId()); } + + invalidateIAMCache(); return policy; } @@ -681,6 +762,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { } } }); + + invalidateIAMCache(); } @DB @@ -701,6 +784,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager { permissionSC.setParameters("policyId", iamPolicyId); _policyPermissionDao.expunge(permissionSC); + invalidateIAMCache(); return policy; }