You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by mc...@apache.org on 2014/05/08 02:12:16 UTC
git commit: updated refs/heads/4.4-forward to 500c99e
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 <mi...@citrix.com>
Authored: Wed May 7 16:42:19 2014 -0700
Committer: Min Chen <mi...@citrix.com>
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;
}