This is an automated email from the ASF dual-hosted git repository. abhay pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/master by this push: new db2bd7c RANGER-3329: Request for _any access-type is denied only when on all access-types are denied db2bd7c is described below commit db2bd7c4f50be5987cf272c42a2b8a2565175461 Author: Abhay Kulkarni <ab...@apache.org> AuthorDate: Tue Jul 20 08:55:31 2021 -0700 RANGER-3329: Request for _any access-type is denied only when on all access-types are denied --- .../policyengine/RangerAccessRequestImpl.java | 18 ++++++ .../policyengine/RangerPolicyEngineImpl.java | 60 ++++++++++++++++--- .../RangerDefaultPolicyEvaluator.java | 69 +++------------------- .../plugin/util/RangerAccessRequestUtil.java | 9 +++ .../ranger/plugin/policyengine/TestPolicyACLs.java | 4 +- .../test_policyengine_descendant_tags.json | 8 +-- .../policyengine/test_policyengine_hive.json | 2 +- .../policyengine/test_policyengine_tag_hive.json | 14 +++++ ...t_policyengine_tag_hive_for_show_databases.json | 10 ++-- 9 files changed, 114 insertions(+), 80 deletions(-) diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessRequestImpl.java b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessRequestImpl.java index 74a7a26..3d0168a 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessRequestImpl.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessRequestImpl.java @@ -78,6 +78,24 @@ public class RangerAccessRequestImpl implements RangerAccessRequest { setClusterName(null); } + public RangerAccessRequestImpl(RangerAccessRequest request) { + setResource(request.getResource()); + setAccessType(request.getAccessType()); + setUser(request.getUser()); + setUserGroups(request.getUserGroups()); + setUserRoles(request.getUserRoles()); + setForwardedAddresses(request.getForwardedAddresses()); + setAccessTime(request.getAccessTime()); + setRemoteIPAddress(request.getRemoteIPAddress()); + setClientType(request.getClientType()); + setAction(request.getAction()); + setRequestData(request.getRequestData()); + setSessionId(request.getSessionId()); + setContext(request.getContext()); + setClusterName(request.getClusterName()); + setResourceMatchingScope(request.getResourceMatchingScope()); + } + @Override public RangerAccessResource getResource() { return resource; diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java index 3c0e32c..9e0a89e 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java @@ -661,11 +661,59 @@ public class RangerPolicyEngineImpl implements RangerPolicyEngine { LOG.debug("==> RangerPolicyEngineImpl.evaluatePoliciesNoAudit(" + request + ", policyType =" + policyType + ", zoneName=" + zoneName + ")"); } - final Date accessTime = request.getAccessTime() != null ? request.getAccessTime() : new Date(); - final RangerAccessResult ret = createAccessResult(request, policyType); + RangerAccessResult ret = createAccessResult(request, policyType); + + if (request.isAccessTypeAny()) { + RangerAccessResult denyResult = null; + RangerAccessResult allowResult = null; + + List<RangerServiceDef.RangerAccessTypeDef> allAccessDefs = getServiceDef().getAccessTypes(); + + for (RangerServiceDef.RangerAccessTypeDef accessTypeDef : allAccessDefs) { + RangerAccessRequestImpl requestForOneAccessType = new RangerAccessRequestImpl(request); + RangerAccessRequestUtil.setIsAnyAccessInContext(requestForOneAccessType.getContext(), Boolean.TRUE); + + requestForOneAccessType.setAccessType(accessTypeDef.getName()); + + RangerAccessResult resultForOneAccessType = evaluatePoliciesForOneAccessTypeNoAudit(requestForOneAccessType, policyType, zoneName, policyRepository, tagPolicyRepository); + + ret.setAuditResultFrom(resultForOneAccessType); + + if (resultForOneAccessType.getIsAccessDetermined()) { + if (resultForOneAccessType.getIsAllowed()) { + allowResult = resultForOneAccessType; + break; + } else if (denyResult == null) { + denyResult = resultForOneAccessType; + } + } + } + + if (allowResult != null) { + ret = allowResult; + } else if (denyResult != null) { + ret = denyResult; + } + } else { + ret = evaluatePoliciesForOneAccessTypeNoAudit(request, policyType, zoneName, policyRepository, tagPolicyRepository); + } + + if (LOG.isDebugEnabled()) { + LOG.debug("<== RangerPolicyEngineImpl.evaluatePoliciesNoAudit(" + request + ", policyType =" + policyType + ", zoneName=" + zoneName + "): " + ret); + } + + return ret; + } + + private RangerAccessResult evaluatePoliciesForOneAccessTypeNoAudit(RangerAccessRequest request, int policyType, String zoneName, RangerPolicyRepository policyRepository, RangerPolicyRepository tagPolicyRepository) { + if (LOG.isDebugEnabled()) { + LOG.debug("==> RangerPolicyEngineImpl.evaluatePoliciesForOneAccessTypeNoAudit(" + request + ", policyType =" + policyType + ", zoneName=" + zoneName + ")"); + } + final boolean isSuperUser = isSuperUser(request.getUser(), request.getUserGroups()); + final Date accessTime = request.getAccessTime() != null ? request.getAccessTime() : new Date(); + final RangerAccessResult ret = createAccessResult(request, policyType); - // for superusers, set access as allowed if (isSuperUser || StringUtils.equals(request.getAccessType(), RangerPolicyEngine.SUPER_USER_ACCESS)) { ret.setIsAllowed(isSuperUser); ret.setIsAccessDetermined(true); @@ -694,9 +742,7 @@ public class RangerPolicyEngineImpl implements RangerPolicyEngine { boolean findAuditByResource = !ret.getIsAuditedDetermined(); boolean foundInCache = findAuditByResource && policyRepository.setAuditEnabledFromCache(request, ret); - if (!isSuperUser) { - ret.setIsAccessDetermined(false); // discard result by tag-policies, to evaluate resource policies for possible override - } + ret.setIsAccessDetermined(false); // discard result by tag-policies, to evaluate resource policies for possible override List<RangerPolicyEvaluator> evaluators = policyRepository.getLikelyMatchPolicyEvaluators(request, policyType); @@ -752,7 +798,7 @@ public class RangerPolicyEngineImpl implements RangerPolicyEngine { } if (LOG.isDebugEnabled()) { - LOG.debug("<== RangerPolicyEngineImpl.evaluatePoliciesNoAudit(" + request + ", policyType =" + policyType + ", zoneName=" + zoneName + "): " + ret); + LOG.debug("<== RangerPolicyEngineImpl.evaluatePoliciesForOneAccessTypeNoAudit(" + request + ", policyType =" + policyType + ", zoneName=" + zoneName + "): " + ret); } return ret; diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java index 014fe6f..831b6d4 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Set; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -45,7 +46,6 @@ import org.apache.ranger.plugin.model.RangerServiceDef; import org.apache.ranger.plugin.model.RangerServiceDef.RangerAccessTypeDef; import org.apache.ranger.plugin.model.RangerValiditySchedule; import org.apache.ranger.plugin.policyengine.RangerAccessRequest; -import org.apache.ranger.plugin.policyengine.RangerAccessRequestImpl; import org.apache.ranger.plugin.policyengine.RangerAccessResource; import org.apache.ranger.plugin.policyengine.RangerAccessResult; import org.apache.ranger.plugin.policyengine.RangerPolicyEngine; @@ -55,6 +55,7 @@ import org.apache.ranger.plugin.policyengine.RangerTagAccessRequest; import org.apache.ranger.plugin.policyresourcematcher.RangerDefaultPolicyResourceMatcher; import org.apache.ranger.plugin.policyresourcematcher.RangerPolicyResourceMatcher; import org.apache.ranger.plugin.resourcematcher.RangerResourceMatcher; +import org.apache.ranger.plugin.util.RangerAccessRequestUtil; import org.apache.ranger.plugin.util.RangerPerfTracer; import org.apache.ranger.plugin.util.ServiceDefUtil; @@ -247,11 +248,8 @@ public class RangerDefaultPolicyEvaluator extends RangerAbstractPolicyEvaluator final boolean isMatched; - if (request.isAccessTypeAny()) { + if (request.isAccessTypeAny() || Boolean.TRUE.equals(RangerAccessRequestUtil.getIsAnyAccessInContext(request.getContext()))) { isMatched = matchType != RangerPolicyResourceMatcher.MatchType.NONE; - if (request.getResourceMatchingScope() == RangerAccessRequest.ResourceMatchingScope.SELF_OR_CHILD) { - matchType = RangerPolicyResourceMatcher.MatchType.DESCENDANT; // So that a deny policy does not take effect! - } } else if (request.getResourceMatchingScope() == RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS) { isMatched = matchType != RangerPolicyResourceMatcher.MatchType.NONE; } else { @@ -493,7 +491,8 @@ public class RangerDefaultPolicyEvaluator extends RangerAbstractPolicyEvaluator LOG.debug("==> RangerDefaultPolicyEvaluator.updateAccessResult(" + result + ", " + matchType +", " + isAllowed + ", " + reason + ", " + getId() + ")"); } if (!isAllowed) { - if (matchType != RangerPolicyResourceMatcher.MatchType.DESCENDANT || !result.getAccessRequest().isAccessTypeAny()) { + RangerAccessResource resource = result.getAccessRequest().getResource(); + if (resource != null && MapUtils.isNotEmpty(resource.getAsMap())) { result.setIsAllowed(false); result.setPolicyPriority(getPolicyPriority()); result.setPolicyId(getId()); @@ -643,7 +642,7 @@ public class RangerDefaultPolicyEvaluator extends RangerAbstractPolicyEvaluator if (LOG.isDebugEnabled()) { LOG.debug("Using ACL Summary for access evaluation. PolicyId=[" + getId() + "]"); } - Integer accessResult = lookupPolicyACLSummary(request.getUser(), request.getUserGroups(), request.getUserRoles(), request.getAccessType()); + Integer accessResult = lookupPolicyACLSummary(request.getUser(), request.getUserGroups(), request.getUserRoles(), request.isAccessTypeAny() || Boolean.TRUE.equals(RangerAccessRequestUtil.getIsAnyAccessInContext(request.getContext())) ? RangerPolicyEngine.ANY_ACCESS : request.getAccessType()); if (accessResult != null) { updateAccessResult(result, matchType, accessResult.equals(RangerPolicyEvaluator.ACCESS_ALLOWED), null); } else if (getPolicy().getIsDenyAllElse()) { @@ -1131,8 +1130,7 @@ public class RangerDefaultPolicyEvaluator extends RangerAbstractPolicyEvaluator switch (policyType) { case RangerPolicy.POLICY_TYPE_ACCESS: { - ret = getMatchingPolicyItemForAccessPolicy(request, result); - + ret = getMatchingPolicyItemForAccessPolicyForSpecificAccess(request, result); break; } case RangerPolicy.POLICY_TYPE_DATAMASK: { @@ -1150,59 +1148,6 @@ public class RangerDefaultPolicyEvaluator extends RangerAbstractPolicyEvaluator return ret; } - protected RangerPolicyItemEvaluator getMatchingPolicyItemForAccessPolicy(RangerAccessRequest request, RangerAccessResult result) { - RangerPolicyItemEvaluator ret = null; - - if (request.isAccessTypeAny()) { - RangerPolicyItemEvaluator denyingPolicyItemEvaluator = null; - int deniedAccessesCount = 0; - int accessDefsCount = 0; - - List<RangerAccessTypeDef> allAccessDefs = getServiceDef().getAccessTypes(); - - for (RangerAccessTypeDef accessTypeDef : allAccessDefs) { - RangerAccessRequestImpl newRequest = new RangerAccessRequestImpl(); - newRequest.setResource(request.getResource()); - newRequest.setUser(request.getUser()); - newRequest.setUserGroups(request.getUserGroups()); - newRequest.setUserRoles(request.getUserRoles()); - newRequest.setForwardedAddresses(request.getForwardedAddresses()); - newRequest.setAccessTime(request.getAccessTime()); - newRequest.setRemoteIPAddress(request.getRemoteIPAddress()); - newRequest.setClientType(request.getClientType()); - newRequest.setAction(request.getAction()); - newRequest.setRequestData(request.getRequestData()); - newRequest.setSessionId(request.getSessionId()); - newRequest.setContext(request.getContext()); - newRequest.setClusterName(request.getClusterName()); - - newRequest.setAccessType(accessTypeDef.getName()); - - ret = getMatchingPolicyItemForAccessPolicyForSpecificAccess(newRequest, result); - - if (ret != null) { - if (ret.getPolicyItemType() == RangerPolicyItemEvaluator.POLICY_ITEM_TYPE_ALLOW) { - break; - } else if (ret.getPolicyItemType() == RangerPolicyItemEvaluator.POLICY_ITEM_TYPE_DENY) { - if (denyingPolicyItemEvaluator == null) { - denyingPolicyItemEvaluator = ret; - } - ret = null; - deniedAccessesCount++; - } - } - accessDefsCount++; - } - if (ret == null && denyingPolicyItemEvaluator != null && deniedAccessesCount == accessDefsCount) { - ret = denyingPolicyItemEvaluator; - } - } else { - ret = getMatchingPolicyItemForAccessPolicyForSpecificAccess(request, result); - } - - return ret; - } - protected RangerPolicyItemEvaluator getMatchingPolicyItemForAccessPolicyForSpecificAccess(RangerAccessRequest request, RangerAccessResult result) { RangerPolicyItemEvaluator ret = getMatchingPolicyItem(request, denyEvaluators, denyExceptionEvaluators); diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java b/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java index 696a3f6..4740055 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java @@ -44,6 +44,7 @@ public class RangerAccessRequestUtil { public static final String KEY_OWNER = "OWNER"; public static final String KEY_ROLES = "ROLES"; public static final String KEY_CONTEXT_ACCESSTYPES = "ACCESSTYPES"; + public static final String KEY_CONTEXT_IS_ANY_ACCESS = "ISANYACCESS"; public static void setRequestTagsInContext(Map<String, Object> context, Set<RangerTagForEval> tags) { if(CollectionUtils.isEmpty(tags)) { @@ -175,4 +176,12 @@ public class RangerAccessRequestUtil { return ret; } + + public static void setIsAnyAccessInContext(Map<String, Object> context, Boolean value) { + context.put(KEY_CONTEXT_IS_ANY_ACCESS, value); + } + + public static Boolean getIsAnyAccessInContext(Map<String, Object> context) { + return (Boolean)context.get(KEY_CONTEXT_IS_ANY_ACCESS); + } } diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyACLs.java b/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyACLs.java index f8eba5f..d2983a2 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyACLs.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyACLs.java @@ -259,7 +259,9 @@ public class TestPolicyACLs { } else if (!(MapUtils.isEmpty(acls.getRoleACLs()) && MapUtils.isEmpty(oneTest.rolePermissions))) { roleACLsMatched = false; } - assertTrue("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name, userACLsMatched && groupACLsMatched && roleACLsMatched && rowFiltersMatched && dataMaskingMatched); + + assertTrue("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name + " - userACLsMatched=" + userACLsMatched + "; groupACLsMatched=" + groupACLsMatched + "; roleACLsMatched=" + roleACLsMatched + "; rowFiltersMatched=" + rowFiltersMatched + "; dataMaskingMatched=" + dataMaskingMatched, + userACLsMatched && groupACLsMatched && roleACLsMatched && rowFiltersMatched && dataMaskingMatched); } } } diff --git a/agents-common/src/test/resources/policyengine/test_policyengine_descendant_tags.json b/agents-common/src/test/resources/policyengine/test_policyengine_descendant_tags.json index 934655b..1b3fc04 100644 --- a/agents-common/src/test/resources/policyengine/test_policyengine_descendant_tags.json +++ b/agents-common/src/test/resources/policyengine/test_policyengine_descendant_tags.json @@ -248,12 +248,12 @@ } , { - "name":"ALLOW 'use default;' for hive", + "name":"DENY 'use default;' for hive", "request":{ "resource":{"elements":{"database":"default"}}, "accessType":"","user":"hive","userGroups":[],"requestData":"'use default;' for hive" }, - "result":{"isAudited":true,"isAllowed":true,"policyId":2} + "result":{"isAudited":true,"isAllowed":false,"policyId":3} } , { @@ -284,12 +284,12 @@ } , { - "name":"ALLOW 'show databases;' for hive", + "name":"DENY 'show databases;' for hive", "request":{ "resource":{"elements":{}}, "accessType":"","user":"hive","userGroups":[],"requestData":"'show databases;' for hive" }, - "result":{"isAudited":true,"isAllowed":true,"policyId":2} + "result":{"isAudited":true,"isAllowed":false,"policyId":3} } ] } diff --git a/agents-common/src/test/resources/policyengine/test_policyengine_hive.json b/agents-common/src/test/resources/policyengine/test_policyengine_hive.json index bd2f67b..70d7e64 100644 --- a/agents-common/src/test/resources/policyengine/test_policyengine_hive.json +++ b/agents-common/src/test/resources/policyengine/test_policyengine_hive.json @@ -126,7 +126,7 @@ "resource":{"elements":{}}, "accessType":"","user":"user5","userGroups":["users"],"requestData":"show databases" }, - "result":{"isAudited":true,"isAllowed":false,"policyId":9} + "result":{"isAudited":true,"isAllowed":true,"policyId":200} } , {"name":"ALLOW '_any access to db1' for user5: match when request has less levels than policy", diff --git a/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive.json b/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive.json index a8ec027..81feced 100644 --- a/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive.json +++ b/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive.json @@ -49,6 +49,12 @@ "policyItems":[ {"accesses":[{"type":"all","isAllowed":true}],"users":["hive", "user1", "user2"],"groups":["public"],"delegateAdmin":false} ] + }, + {"id":103,"name":"db=default, table=table2: audit-all-access","isEnabled":true,"isAuditEnabled":true, + "resources":{"database":{"values":["default"]},"table":{"values":["table2"]},"column":{"values":["*"]}}, + "denyPolicyItems":[ + {"accesses":[{"type":"select","isAllowed":true}],"users":["denieduser"],"groups":[],"delegateAdmin":false} + ] } ], "tagPolicyInfo": { @@ -222,6 +228,14 @@ }, "tests":[ + {"name":"DENY 'desc default.table2;' for denieduser", + "request":{ + "resource":{"elements":{"database":"default", "table":"table2"}}, + "accessType":"","user":"denieduser","userGroups":[],"requestData":"desc default.table2;' for denieduser", + "context": {"TAGS":"[{\"type\":\"PII-FINAL\", \"attributes\":{\"expiry\":\"2026/06/15\"}}]"} + }, + "result":{"isAudited":true,"isAllowed":false,"policyId":103} + }, {"name":"DENY 'select ssn from employee.personal;' for testuser using EXPIRES_ON tag with DESCENDANT match", "request":{ "resource":{"elements":{"database":"employee", "table":"personal", "column":"ssn"}}, diff --git a/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive_for_show_databases.json b/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive_for_show_databases.json index f42df3e..1686b1f 100644 --- a/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive_for_show_databases.json +++ b/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive_for_show_databases.json @@ -174,14 +174,14 @@ }, "tests":[ - {"name":"ALLOW 'show databases;' for hive", + {"name":"DENY 'show databases;' for hive", "request":{ "resource":{"elements":{}}, "accessType":"","user":"hive","userGroups":["public"],"requestData":"show databases for hive" }, - "result":{"isAudited":true,"isAllowed":true,"policyId":101} + "result":{"isAudited":true,"isAllowed":false,"policyId":1} } - , + , {"name":"ALLOW 'use default;' for hive", "request":{ "resource":{"elements":{"database":"default"}}, @@ -190,12 +190,12 @@ "result":{"isAudited":true,"isAllowed":true,"policyId":101} } , - {"name":"ALLOW 'use employee;' for hive", + {"name":"DENY 'use employee;' for hive", "request":{ "resource":{"elements":{"database":"employee"}}, "accessType":"","user":"hive","userGroups":["public"],"requestData":"use employee for hive" }, - "result":{"isAudited":true,"isAllowed":true,"policyId":101} + "result":{"isAudited":true,"isAllowed":false,"policyId":1} } , {"name":"DENY 'select ssn from employee.personal;' for hive",