Repository: incubator-ranger
Updated Branches:
  refs/heads/master 7a80c8e35 -> a733b7c33


RANGER-763: Optimize policy evaluation by reordering match-checks


Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/a733b7c3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/a733b7c3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/a733b7c3

Branch: refs/heads/master
Commit: a733b7c3312f7bf954a233acaaf50ebf85b93b1b
Parents: 7a80c8e
Author: Abhay Kulkarni <[email protected]>
Authored: Wed Dec 2 11:36:19 2015 -0800
Committer: Madhan Neethiraj <[email protected]>
Committed: Wed Dec 2 11:36:19 2015 -0800

----------------------------------------------------------------------
 .../RangerAbstractPolicyEvaluator.java          |  6 +-
 .../RangerDefaultPolicyEvaluator.java           | 35 +++++-----
 .../RangerOptimizedPolicyEvaluator.java         | 69 ++++++++++----------
 3 files changed, 61 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/a733b7c3/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyEvaluator.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyEvaluator.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyEvaluator.java
index fa35f1c..adc7d8c 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyEvaluator.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyEvaluator.java
@@ -26,9 +26,9 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.ranger.plugin.model.RangerPolicy;
 import org.apache.ranger.plugin.model.RangerServiceDef;
+import org.apache.ranger.plugin.policyengine.RangerAccessRequest;
 import org.apache.ranger.plugin.policyengine.RangerPolicyEngineOptions;
 
-
 public abstract class RangerAbstractPolicyEvaluator implements 
RangerPolicyEvaluator {
        private static final Log LOG = 
LogFactory.getLog(RangerAbstractPolicyEvaluator.class);
 
@@ -65,6 +65,10 @@ public abstract class RangerAbstractPolicyEvaluator 
implements RangerPolicyEvalu
                return policy != null && 
CollectionUtils.isNotEmpty(policy.getPolicyItems());
        }
 
+       protected boolean hasMatchablePolicyItem(RangerAccessRequest request) {
+               return hasAllow() || hasDeny();
+       }
+
        public boolean hasDeny() {
                return policy != null && 
CollectionUtils.isNotEmpty(policy.getDenyPolicyItems());
        }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/a733b7c3/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java
----------------------------------------------------------------------
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 439b58d..77fdb90 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
@@ -159,24 +159,29 @@ public class RangerDefaultPolicyEvaluator extends 
RangerAbstractPolicyEvaluator
             }
 
             if (!result.getIsAccessDetermined()) {
-                // Try Match only if it was not attempted as part of 
evaluating Audit requirement
-                if (!isResourceMatchAttempted) {
-                    isResourceMatch = isMatch(request.getResource());
-                    isResourceMatchAttempted = true;
-                }
 
-                // Try Head Match only if no match was found so far AND a head 
match was not attempted as part of evaluating
-                // Audit requirement
-                if (!isResourceMatch) {
-                    if (attemptResourceHeadMatch && 
!isResourceHeadMatchAttempted) {
-                        isResourceHeadMatch = 
matchResourceHead(request.getResource());
-                        isResourceHeadMatchAttempted = true;
+                // Attempt resource matching only if there may be a matchable 
policyItem
+                if (hasMatchablePolicyItem(request)) {
+
+                    // Try Match only if it was not attempted as part of 
evaluating Audit requirement
+                    if (!isResourceMatchAttempted) {
+                        isResourceMatch = isMatch(request.getResource());
+                        isResourceMatchAttempted = true;
                     }
-                }
 
-                // Go further to evaluate access only if match or head match 
was found at this point
-                if (isResourceMatch || isResourceHeadMatch) {
-                    evaluatePolicyItems(request, result, isResourceMatch);
+                    // Try Head Match only if no match was found so far AND a 
head match was not attempted as part of evaluating
+                    // Audit requirement
+                    if (!isResourceMatch) {
+                        if (attemptResourceHeadMatch && 
!isResourceHeadMatchAttempted) {
+                            isResourceHeadMatch = 
matchResourceHead(request.getResource());
+                            isResourceHeadMatchAttempted = true;
+                        }
+                    }
+
+                    // Go further to evaluate access only if match or head 
match was found at this point
+                    if (isResourceMatch || isResourceHeadMatch) {
+                        evaluatePolicyItems(request, result, isResourceMatch);
+                    }
                 }
             }
         }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/a733b7c3/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
index 8cd854f..47dcd54 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
@@ -26,7 +26,6 @@ import org.apache.commons.logging.LogFactory;
 import org.apache.ranger.plugin.model.RangerPolicy;
 import org.apache.ranger.plugin.model.RangerServiceDef;
 import org.apache.ranger.plugin.policyengine.RangerAccessRequest;
-import org.apache.ranger.plugin.policyengine.RangerAccessResult;
 import org.apache.ranger.plugin.policyengine.RangerPolicyEngine;
 import org.apache.ranger.plugin.policyengine.RangerPolicyEngineOptions;
 import org.apache.ranger.plugin.util.RangerPerfTracer;
@@ -224,52 +223,56 @@ public class RangerOptimizedPolicyEvaluator extends 
RangerDefaultPolicyEvaluator
         return evalOrder;
     }
 
-       @Override
-       protected boolean isAccessAllowed(String user, Set<String> userGroups, 
String accessType) {
-               if(LOG.isDebugEnabled()) {
-                       LOG.debug("==> 
RangerOptimizedPolicyEvaluator.isAccessAllowed(" + user + ", " + userGroups + 
", " + accessType + ")");
-               }
+    @Override
+    protected boolean isAccessAllowed(String user, Set<String> userGroups, 
String accessType) {
+        if(LOG.isDebugEnabled()) {
+            LOG.debug("==> RangerOptimizedPolicyEvaluator.isAccessAllowed(" + 
user + ", " + userGroups + ", " + accessType + ")");
+        }
 
-               boolean ret = false;
+        boolean ret = hasMatchablePolicyItem(user, userGroups, accessType) && 
super.isAccessAllowed(user, userGroups, accessType);
 
-        if (hasPublicGroup || users.contains(user) || 
CollectionUtils.containsAny(groups, userGroups)) {
-                       if (StringUtils.isEmpty(accessType)) {
-                               accessType = RangerPolicyEngine.ANY_ACCESS;
-                       }
+        if(LOG.isDebugEnabled()) {
+            LOG.debug("<== RangerOptimizedPolicyEvaluator.isAccessAllowed(" + 
user + ", " + userGroups + ", " + accessType + "): " + ret);
+        }
+
+        return ret;
+    }
 
-                       boolean isAnyAccess   = StringUtils.equals(accessType, 
RangerPolicyEngine.ANY_ACCESS);
-                       boolean isAdminAccess = StringUtils.equals(accessType, 
RangerPolicyEngine.ADMIN_ACCESS);
+    @Override
+    protected boolean hasMatchablePolicyItem(RangerAccessRequest request) {
+        boolean ret = false;
 
-            if (isAnyAccess || (isAdminAccess && delegateAdmin) || hasAllPerms 
|| accessPerms.contains(accessType)) {
-                ret = super.isAccessAllowed(user, userGroups, accessType);
+        if (hasPublicGroup || users.contains(request.getUser()) || 
CollectionUtils.containsAny(groups, request.getUserGroups())) {
+            if(request.isAccessTypeDelegatedAdmin()) {
+                ret = delegateAdmin;
+            } else if(hasAllPerms) {
+                ret = true;
+            } else {
+                ret = request.isAccessTypeAny() || 
accessPerms.contains(request.getAccessType());
             }
         }
 
-        if(LOG.isDebugEnabled()) {
-            LOG.debug("<== RangerOptimizedPolicyEvaluator.isAccessAllowed(" + 
user + ", " + userGroups + ", " + accessType + "): " + ret);
-        }
+        return ret;
+    }
 
-               return ret;
-       }
+    private boolean hasMatchablePolicyItem(String user, Set<String> 
userGroups, String accessType) {
+        boolean ret = false;
 
-       @Override
-    protected void evaluatePolicyItems(RangerAccessRequest request, 
RangerAccessResult result, boolean isResourceMatch) {
-        if(LOG.isDebugEnabled()) {
-            LOG.debug("==> 
RangerOptimizedPolicyEvaluator.evaluatePolicyItems(" + request + ", " + result 
+ ", " + isResourceMatch + ")");
-        }
+        if (hasPublicGroup || users.contains(user) || 
CollectionUtils.containsAny(groups, userGroups)) {
+            boolean isAdminAccess = StringUtils.equals(accessType, 
RangerPolicyEngine.ADMIN_ACCESS);
 
-        if (hasPublicGroup || users.contains(request.getUser()) || 
CollectionUtils.containsAny(groups, request.getUserGroups())) {
-            // No need to reject based on users and groups
+            if(isAdminAccess) {
+                   ret = delegateAdmin;
+            } else if(hasAllPerms) {
+                ret = true;
+            } else {
+                boolean isAccessTypeAny = StringUtils.isEmpty(accessType) || 
StringUtils.equals(accessType, RangerPolicyEngine.ANY_ACCESS);
 
-            if (request.isAccessTypeAny() || 
(request.isAccessTypeDelegatedAdmin() && delegateAdmin) || hasAllPerms || 
accessPerms.contains(request.getAccessType())) {
-                // No need to reject based on aggregated access permissions
-                super.evaluatePolicyItems(request, result, isResourceMatch);
+                   ret = isAccessTypeAny || accessPerms.contains(accessType);
             }
         }
 
-        if(LOG.isDebugEnabled()) {
-            LOG.debug("<== 
RangerOptimizedPolicyEvaluator.evaluatePolicyItems(" + request + ", " + result 
+ ", " + isResourceMatch + ")");
-        }
+        return ret;
     }
 
     private void preprocessPolicyItems(List<RangerPolicy.RangerPolicyItem> 
policyItems) {

Reply via email to