You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by ab...@apache.org on 2020/08/11 04:19:34 UTC

[ranger] branch master updated: RANGER-2876: allow-exception policy-items are not correctly handled when access-type is '_any'

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 f29d29b  RANGER-2876: allow-exception policy-items are not correctly handled when access-type is '_any'
f29d29b is described below

commit f29d29b8da7ac8d4afa0d10e504b146381307d6d
Author: Abhay Kulkarni <ab...@apache.org>
AuthorDate: Mon Aug 10 21:04:50 2020 -0700

    RANGER-2876: allow-exception policy-items are not correctly handled when access-type is '_any'
---
 .../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":{