This is an automated email from the ASF dual-hosted git repository. mehul pushed a commit to branch ranger-2.1 in repository https://gitbox.apache.org/repos/asf/ranger.git
commit 90877c6d3364cacb13b1ef29541da6505d13cc2a Author: Abhay Kulkarni <[email protected]> AuthorDate: Mon Aug 10 21:04:50 2020 -0700 RANGER-2876: allow-exception policy-items are not correctly handled when access-type is '_any' Signed-off-by: Mehul Parikh <[email protected]> --- .../RangerDefaultPolicyEvaluator.java | 105 ++++++++++++++++----- .../RangerDefaultPolicyItemEvaluator.java | 45 +-------- .../test_policyengine_descendant_tags.json | 4 +- .../policyengine/test_policyengine_hive.json | 26 +++++ .../policyengine/test_policyengine_tag_hive.json | 10 +- .../test_policyengine_tag_hive_filebased.json | 4 +- 6 files changed, 122 insertions(+), 72 deletions(-) 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 d75bf46..24cb424 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 @@ -45,6 +45,7 @@ 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; @@ -685,44 +686,36 @@ public class RangerDefaultPolicyEvaluator extends RangerAbstractPolicyEvaluator return ret; } - private Integer getAccessResultForAnyAccess(Map<String, PolicyACLSummary.AccessResult> accessses) { - Integer ret = null; + private Integer getAccessResultForAnyAccess(Map<String, PolicyACLSummary.AccessResult> accesses) { + final Integer ret; int allowedAccessCount = 0; int deniedAccessCount = 0; - int undeterminedAccessCount = 0; - int accessesSize = 0; - for (Map.Entry<String, PolicyACLSummary.AccessResult> entry : accessses.entrySet()) { + for (Map.Entry<String, PolicyACLSummary.AccessResult> entry : accesses.entrySet()) { if (StringUtils.equals(entry.getKey(), RangerPolicyEngine.ADMIN_ACCESS)) { - // Dont count admin access if present + // Don't count admin access if present continue; } PolicyACLSummary.AccessResult accessResult = entry.getValue(); if (accessResult.getResult() == RangerPolicyEvaluator.ACCESS_ALLOWED) { allowedAccessCount++; + break; } else if (accessResult.getResult() == RangerPolicyEvaluator.ACCESS_DENIED) { deniedAccessCount++; - } else if (accessResult.getResult() == RangerPolicyEvaluator.ACCESS_UNDETERMINED && !accessResult.getHasSeenDeny()) { - undeterminedAccessCount++; } - accessesSize++; } - int accessTypeCount = getServiceDef().getAccessTypes().size(); - - if (accessTypeCount == accessesSize) { - // All permissions are represented - if (deniedAccessCount > 0 || undeterminedAccessCount == accessTypeCount) { - // at least one is denied or all are undetermined - ret = RangerPolicyEvaluator.ACCESS_DENIED; - } - } - if (ret == null) { - if (allowedAccessCount > 0 || undeterminedAccessCount > 0) { - ret = RangerPolicyEvaluator.ACCESS_ALLOWED; - } + if (allowedAccessCount > 0) { + // At least one access allowed + ret = RangerPolicyEvaluator.ACCESS_ALLOWED; + } else if (deniedAccessCount == getServiceDef().getAccessTypes().size()) { + // All accesses explicitly denied + ret = RangerPolicyEvaluator.ACCESS_DENIED; + } else { + ret = null; } + return ret; } @@ -1102,11 +1095,8 @@ public class RangerDefaultPolicyEvaluator extends RangerAbstractPolicyEvaluator switch (policyType) { case RangerPolicy.POLICY_TYPE_ACCESS: { - ret = getMatchingPolicyItem(request, denyEvaluators, denyExceptionEvaluators); + ret = getMatchingPolicyItemForAccessPolicy(request, result); - if(ret == null && !result.getIsAccessDetermined()) { // a deny policy could have set isAllowed=true, but in such case it wouldn't set isAccessDetermined=true - ret = getMatchingPolicyItem(request, allowEvaluators, allowExceptionEvaluators); - } break; } case RangerPolicy.POLICY_TYPE_DATAMASK: { @@ -1124,6 +1114,69 @@ 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); + + if(ret == null && !result.getIsAccessDetermined()) { // a deny policy could have set isAllowed=true, but in such case it wouldn't set isAccessDetermined=true + ret = getMatchingPolicyItem(request, allowEvaluators, allowExceptionEvaluators); + } + + return ret; + } + protected <T extends RangerPolicyItemEvaluator> T getMatchingPolicyItem(RangerAccessRequest request, List<T> evaluators) { T ret = getMatchingPolicyItem(request, evaluators, null); diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java index 90d96d9..8f2d3f1 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java @@ -19,7 +19,6 @@ package org.apache.ranger.plugin.policyevaluator; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Set; @@ -52,7 +51,6 @@ public class RangerDefaultPolicyItemEvaluator extends RangerAbstractPolicyItemEv private boolean hasCurrentUser; private boolean hasResourceOwner; - private boolean hasAllPerms; public RangerDefaultPolicyItemEvaluator(RangerServiceDef serviceDef, RangerPolicy policy, RangerPolicyItem policyItem, int policyItemType, int policyItemIndex, RangerPolicyEngineOptions options) { super(serviceDef, policy, policyItem, policyItemType, policyItemIndex, options); @@ -63,26 +61,6 @@ public class RangerDefaultPolicyItemEvaluator extends RangerAbstractPolicyItemEv LOG.debug("==> RangerDefaultPolicyItemEvaluator(policyId=" + policyId + ", policyItem=" + policyItem + ", serviceType=" + getServiceType() + ", conditionsDisabled=" + getConditionsDisabledOption() + ")"); } - Set<String> accessPerms = new HashSet<String>(); - - List<RangerPolicy.RangerPolicyItemAccess> policyItemAccesses = policyItem.getAccesses(); - for(RangerPolicy.RangerPolicyItemAccess policyItemAccess : policyItemAccesses) { - - if (policyItemAccess.getIsAllowed()) { - accessPerms.add(policyItemAccess.getType()); - } - } - - hasAllPerms = true; - List<RangerServiceDef.RangerAccessTypeDef> serviceAccessTypes = serviceDef.getAccessTypes(); - for (RangerServiceDef.RangerAccessTypeDef serviceAccessType : serviceAccessTypes) { - String serviceAccessTypeName = serviceAccessType.getName(); - if (!accessPerms.contains(serviceAccessTypeName)) { - hasAllPerms = false; - break; - } - } - RangerCustomConditionEvaluator rangerCustomConditionEvaluator = new RangerCustomConditionEvaluator(); conditionEvaluators = rangerCustomConditionEvaluator.getPolicyItemConditionEvaluator(policy,policyItem,serviceDef,options,policyItemIndex); @@ -119,25 +97,10 @@ public class RangerDefaultPolicyItemEvaluator extends RangerAbstractPolicyItemEv } else if (CollectionUtils.isNotEmpty(policyItem.getAccesses())) { boolean isAccessTypeMatched = false; - if (request.isAccessTypeAny()) { - if (getPolicyItemType() == POLICY_ITEM_TYPE_DENY || getPolicyItemType() == POLICY_ITEM_TYPE_DENY_EXCEPTIONS) { - if (hasAllPerms) { - isAccessTypeMatched = true; - } - } else { - for (RangerPolicy.RangerPolicyItemAccess access : policyItem.getAccesses()) { - if (access.getIsAllowed()) { - isAccessTypeMatched = true; - break; - } - } - } - } else { - for (RangerPolicy.RangerPolicyItemAccess access : policyItem.getAccesses()) { - if (access.getIsAllowed() && StringUtils.equalsIgnoreCase(access.getType(), request.getAccessType())) { - isAccessTypeMatched = true; - break; - } + for (RangerPolicy.RangerPolicyItemAccess access : policyItem.getAccesses()) { + if (access.getIsAllowed() && StringUtils.equalsIgnoreCase(access.getType(), request.getAccessType())) { + isAccessTypeMatched = true; + break; } } 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 a2ec460..934655b 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 @@ -156,7 +156,7 @@ {"id":2,"name":"PII_TAG_POLICY","isEnabled":true,"isAuditEnabled":true, "resources":{"tag":{"values":["PII"],"isRecursive":false}}, "policyItems":[ - {"accesses":[{"type":"hive:select","isAllowed":true}],"users":["hive", "user1"],"groups":[],"delegateAdmin":false} + {"accesses":[{"type":"hive:select","isAllowed":true}, {"type":"hive:update","isAllowed":true}],"users":["hive", "user1"],"groups":[],"delegateAdmin":false} , {"accesses":[{"type":"hive:all","isAllowed":true}],"users":["user2"],"groups":[],"delegateAdmin":false} ], @@ -170,7 +170,7 @@ {"id":3,"name":"EXPIRES_ON_TAG_POLICY","isEnabled":true,"isAuditEnabled":true, "resources":{"tag":{"values":["EXPIRES_ON"],"isRecursive":false}}, "policyItems":[ - {"accesses":[{"type":"hive:select","isAllowed":true}],"users":["user", "user1"],"groups":[],"delegateAdmin":false} + {"accesses":[{"type":"hive:select","isAllowed":true}, {"type":"hive:update","isAllowed":true}],"users":["user", "user1"],"groups":[],"delegateAdmin":false} ], "denyPolicyItems":[ {"accesses":[{"type":"hive:select","isAllowed":true}],"users":["user"],"groups":[],"delegateAdmin":false} 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 ba5a53c..52864a0 100644 --- a/agents-common/src/test/resources/policyengine/test_policyengine_hive.json +++ b/agents-common/src/test/resources/policyengine/test_policyengine_hive.json @@ -99,10 +99,36 @@ "policyItems":[ {"accesses":[{"type":"create","isAllowed":true}],"users":["user1","user2"],"groups":["group1","group2"],"delegateAdmin":false} ] + }, + {"id":8,"name":"db=dummy; table=*; column=*","isEnabled":true,"isAuditEnabled":true, + "resources":{"database":{"values":["dummy"]},"table":{"values":["*"]},"column":{"values":["*"]}}, + "policyItems":[ + {"accesses":[{"type":"create","isAllowed":true},{"type":"update","isAllowed":true},{"type":"drop","isAllowed":true}],"users":["user1","user2"],"groups":[],"delegateAdmin":false} + ], + "allowExceptions":[ + {"accesses":[{"type":"create","isAllowed":true}, {"type":"update","isAllowed":true}],"users":["user1"],"groups":[],"delegateAdmin":false}, + {"accesses":[{"type":"create","isAllowed":true}, {"type":"update","isAllowed":true},{"type":"drop","isAllowed":true}],"users":["user2"],"groups":[],"delegateAdmin":false} + ] } ], "tests":[ + {"name":"ALLOW 'any dummy/*/*;' for user1", + "request":{ + "resource":{"elements":{"database":"dummy", "table": "dummy", "column": "dummy"}}, + "accessType":"","user":"user1","userGroups":["users"],"requestData":"any dummy/dummy/dummy for user1" + }, + "result":{"isAudited":true,"isAllowed":true,"policyId":8} + } + , + {"name":"DENY 'any dummy/*/*;' for user2", + "request":{ + "resource":{"elements":{"database":"dummy", "table": "dummy", "column": "dummy"}}, + "accessType":"","user":"user2","userGroups":["users"],"requestData":"any dummy/dummy/dummy for user2" + }, + "result":{"isAudited":true,"isAllowed":false,"policyId":-1} + } + , {"name":"ALLOW 'read s3a://qe-s3-bucket-mst/demo;' for user1", "request":{ "resource":{"elements":{"url":"s3a://qe-s3-bucket-mst/demo"}}, 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 79417a0..a8ec027 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 @@ -322,13 +322,21 @@ }, "result":{"isAudited":true,"isAllowed":false,"policyId":3} }, + {"name":"DENY 'desc default.table1;' for testuser", + "request":{ + "resource":{"elements":{"database":"default", "table":"table1"}}, + "accessType":"","user":"testuser","userGroups":[],"requestData":"desc default.table1;' for testuser", + "context": {"TAGS":"[{\"type\":\"PII-FINAL\", \"attributes\":{\"expiry\":\"2026/06/15\"}}]"} + }, + "result":{"isAudited":true,"isAllowed":false,"policyId":-1} + }, {"name":"ALLOW 'use default;' for hive", "request":{ "resource":{"elements":{"database":"default"}}, "accessType":"","user":"hive","userGroups":[],"requestData":"use default", "context": {"TAGS":"[{\"type\":\"PII-FINAL\", \"attributes\":{\"expiry\":\"2026/06/15\"}}]"} }, - "result":{"isAudited":true,"isAllowed":false,"policyId":3} + "result":{"isAudited":true,"isAllowed":true,"policyId":101} }, {"name":"DENY 'use default;' for user1", "request":{ diff --git a/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive_filebased.json b/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive_filebased.json index 73fe540..21d936a 100644 --- a/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive_filebased.json +++ b/agents-common/src/test/resources/policyengine/test_policyengine_tag_hive_filebased.json @@ -283,12 +283,12 @@ }, "result":{"isAudited":true,"isAllowed":true,"policyId":2} }, - {"name":"DENY 'desc default.table1;' for hive using PII, PII-FINAL tags", + {"name":"ALLOW 'desc default.table1;' for hive using PII, PII-FINAL tags", "request":{ "resource":{"elements":{"database":"default", "table":"table1"}}, "accessType":"","user":"hive","userGroups":[],"requestData":"desc default.table1;' for hive" }, - "result":{"isAudited":true,"isAllowed":false,"policyId":3} + "result":{"isAudited":true,"isAllowed":true,"policyId":101} }, {"name":"DENY 'desc default.table2;' for user1 using PII-FINAL tag", "request":{
