Repository: ranger
Updated Branches:
  refs/heads/master 0c339bdf4 -> ded33518f


RANGER-1984: Hbase audit log records may not show all tags associated with 
accessed column


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

Branch: refs/heads/master
Commit: ded33518f3bfb153b10c770ffb82dc835c1fc18b
Parents: 0c339bd
Author: Abhay Kulkarni <[email protected]>
Authored: Wed Feb 14 13:04:44 2018 -0800
Committer: Abhay Kulkarni <[email protected]>
Committed: Wed Feb 14 13:04:44 2018 -0800

----------------------------------------------------------------------
 .../contextenricher/RangerTagForEval.java       |  2 +-
 .../policyengine/RangerTagAccessRequest.java    |  1 +
 .../RangerDefaultPolicyEvaluator.java           | 38 ++++++++-------
 .../test_policyengine_tag_hdfs.json             |  4 +-
 .../hbase/RangerAuthorizationCoprocessor.java   | 49 +++++++++-----------
 .../hbase/HBaseRangerAuthorizationTest.java     | 15 +++---
 6 files changed, 57 insertions(+), 52 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ranger/blob/ded33518/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagForEval.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagForEval.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagForEval.java
index b8f5b42..e31efa3 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagForEval.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagForEval.java
@@ -33,7 +33,7 @@ import java.util.Map;
 
 @JsonAutoDetect(fieldVisibility=JsonAutoDetect.Visibility.ANY)
 @JsonSerialize(include=JsonSerialize.Inclusion.NON_NULL)
-@JsonIgnoreProperties(ignoreUnknown=true, value="matchType")
+@JsonIgnoreProperties(ignoreUnknown=true)
 @XmlRootElement
 @XmlAccessorType(XmlAccessType.FIELD)
 

http://git-wip-us.apache.org/repos/asf/ranger/blob/ded33518/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerTagAccessRequest.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerTagAccessRequest.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerTagAccessRequest.java
index dbdcacd..cf590f9 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerTagAccessRequest.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerTagAccessRequest.java
@@ -52,6 +52,7 @@ public class RangerTagAccessRequest extends 
RangerAccessRequestImpl {
                super.setRemoteIPAddress(request.getRemoteIPAddress());
                super.setForwardedAddresses(request.getForwardedAddresses());
                super.setSessionId(request.getSessionId());
+               
super.setResourceMatchingScope(request.getResourceMatchingScope());
        }
        public RangerPolicyResourceMatcher.MatchType getMatchType() {
                return matchType;

http://git-wip-us.apache.org/repos/asf/ranger/blob/ded33518/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 8051ec3..2b66c70 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
@@ -166,20 +166,26 @@ public class RangerDefaultPolicyEvaluator extends 
RangerAbstractPolicyEvaluator
         if (request != null && result != null) {
 
                        if (!result.getIsAccessDetermined() || 
!result.getIsAuditedDetermined()) {
-                               RangerPolicyResourceMatcher.MatchType matchType 
= resourceMatcher != null ? resourceMatcher.getMatchType(request.getResource(), 
request.getContext()) : RangerPolicyResourceMatcher.MatchType.NONE;
+                               RangerPolicyResourceMatcher.MatchType matchType;
 
-                               final boolean isMatched;
-                               if (request.isAccessTypeAny()) {
-                                       isMatched = matchType != 
RangerPolicyResourceMatcher.MatchType.NONE;
-                               } else if (request.getResourceMatchingScope() 
== RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS) {
-                                       isMatched = matchType == 
RangerPolicyResourceMatcher.MatchType.SELF || matchType == 
RangerPolicyResourceMatcher.MatchType.DESCENDANT;
+                               if 
(RangerTagAccessRequest.class.isInstance(request)) {
+                                       matchType = ((RangerTagAccessRequest) 
request).getMatchType();
                                } else {
-                                       isMatched = matchType == 
RangerPolicyResourceMatcher.MatchType.SELF || matchType == 
RangerPolicyResourceMatcher.MatchType.ANCESTOR;
+                                       matchType = resourceMatcher != null ? 
resourceMatcher.getMatchType(request.getResource(), request.getContext()) : 
RangerPolicyResourceMatcher.MatchType.NONE;
                                }
 
+                               final boolean isMatched = matchType != 
RangerPolicyResourceMatcher.MatchType.NONE;;
+
                                if (isMatched) {
                                        if 
(RangerTagAccessRequest.class.isInstance(request)) {
-                                               matchType = 
((RangerTagAccessRequest) request).getMatchType();
+                                               if (matchType == 
RangerPolicyResourceMatcher.MatchType.DESCENDANT
+                                                               && 
!request.isAccessTypeAny()
+                                                               && 
request.getResourceMatchingScope() == 
RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS) {
+                                                       if 
(LOG.isDebugEnabled()) {
+                                                               
LOG.debug("Setting matchType from DESCENDANT to SELF, so that any DENY 
policy-items will take effect.");
+                                                       }
+                                                       matchType = 
RangerPolicyResourceMatcher.MatchType.SELF;
+                                               }
                                        }
                                        if (!result.getIsAuditedDetermined()) {
                                                if (isAuditEnabled()) {
@@ -301,17 +307,15 @@ public class RangerDefaultPolicyEvaluator extends 
RangerAbstractPolicyEvaluator
                if(LOG.isDebugEnabled()) {
                        LOG.debug("==> 
RangerDefaultPolicyEvaluator.getResourceAccessInfo(" + request + ", " + result 
+ ")");
                }
-               RangerPolicyResourceMatcher.MatchType matchType = 
resourceMatcher != null ? resourceMatcher.getMatchType(request.getResource(), 
request.getContext()) : RangerPolicyResourceMatcher.MatchType.NONE;
-
-               final boolean isMatched;
-               if (request.isAccessTypeAny()) {
-                               isMatched = matchType != 
RangerPolicyResourceMatcher.MatchType.NONE;
-                       } else if (request.getResourceMatchingScope() == 
RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS) {
-                               isMatched = matchType == 
RangerPolicyResourceMatcher.MatchType.SELF || matchType == 
RangerPolicyResourceMatcher.MatchType.DESCENDANT;
-                       } else {
-                       isMatched = matchType == 
RangerPolicyResourceMatcher.MatchType.SELF || matchType == 
RangerPolicyResourceMatcher.MatchType.ANCESTOR;
+               RangerPolicyResourceMatcher.MatchType matchType;
+               if (RangerTagAccessRequest.class.isInstance(request)) {
+                       matchType = ((RangerTagAccessRequest) 
request).getMatchType();
+               } else {
+                       matchType = resourceMatcher != null ? 
resourceMatcher.getMatchType(request.getResource(), request.getContext()) : 
RangerPolicyResourceMatcher.MatchType.NONE;
                }
 
+               final boolean isMatched = matchType != 
RangerPolicyResourceMatcher.MatchType.NONE;;
+
                if (isMatched) {
 
                        if (CollectionUtils.isNotEmpty(allowEvaluators)) {

http://git-wip-us.apache.org/repos/asf/ranger/blob/ded33518/agents-common/src/test/resources/policyengine/test_policyengine_tag_hdfs.json
----------------------------------------------------------------------
diff --git 
a/agents-common/src/test/resources/policyengine/test_policyengine_tag_hdfs.json 
b/agents-common/src/test/resources/policyengine/test_policyengine_tag_hdfs.json
index b4941cd..eb2251c 100644
--- 
a/agents-common/src/test/resources/policyengine/test_policyengine_tag_hdfs.json
+++ 
b/agents-common/src/test/resources/policyengine/test_policyengine_tag_hdfs.json
@@ -215,7 +215,7 @@
         "userGroups": [ ],
         "requestData": "read /resource",
         "context": {
-          "TAGS": "[{\"type\":\"PII\", \"matchType\":1}]"
+          "TAGS": "[{\"type\":\"PII\", \"matchType\": \"SELF\"}]"
         }
       },
       "result": { "isAudited": true, "isAllowed": false, "policyId": 101 }
@@ -371,7 +371,7 @@
         "userGroups": [ ],
         "requestData": "read /resource",
         "context": {
-          "TAGS": "[{\"type\":\"Unaudited-TAG\", \"matchType\":1}]"
+          "TAGS": "[{\"type\":\"Unaudited-TAG\", \"matchType\": \"SELF\"}]"
         }
       },
       "result": { "isAudited": true, "isAllowed": false, "policyId": 1 }

http://git-wip-us.apache.org/repos/asf/ranger/blob/ded33518/hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java
----------------------------------------------------------------------
diff --git 
a/hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java
 
b/hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java
index 058168f..12b675b 100644
--- 
a/hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java
+++ 
b/hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java
@@ -398,47 +398,33 @@ public class RangerAuthorizationCoprocessor extends 
RangerAuthorizationCoprocess
                        if (columns == null || columns.isEmpty()) {
                                LOG.debug("evaluateAccess: columns collection 
null or empty, ok.  Family level access is desired.");
                                session.column(null) // zap stale column from 
prior iteration of this loop, if any
-                                       .buildRequest()
-                                       .authorize();
+                                               .buildRequest()
+                                               .authorize();
                                AuthzAuditEvent auditEvent = 
auditHandler.getAndDiscardMostRecentEvent(); // capture it only for success
                                if (session.isAuthorized()) {
-                                       if (LOG.isDebugEnabled()) {
-                                               LOG.debug("evaluateAccess: has 
family level access [" + family + "]");
-                                       }
-                                       // we need to do 3 things: 
housekeeping, decide about audit events, building the results cache for filter
                                        somethingIsAccessible = true;
-                                       familesAccessAllowed.add(family);
-                                       if (auditEvent != null) {
-                                               LOG.debug("evaluateAccess: 
adding to family-level-access-granted-event-set");
-                                               
familyLevelAccessEvents.add(auditEvent);
-                                       }
-                               } else {
-                                       everythingIsAccessible = false;
-                                       if (auditEvent != null && deniedEvent 
== null) { // we need to capture just one denial event
-                                               LOG.debug("evaluateAccess: 
Setting denied access audit event with last auth failure audit event.");
-                                               deniedEvent = auditEvent;
-                                       }
                                        if (LOG.isDebugEnabled()) {
-                                               LOG.debug("evaluateAccess: no 
family level access [" + family + "].  Checking if has partial access (of any 
type)...");
+                                               LOG.debug("evaluateAccess: has 
family level access [" + family + "]. Checking if [" + family + "] descendants 
have access.");
                                        }
-
                                        
session.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS)
                                                        .buildRequest()
                                                        .authorize();
                                        auditEvent = 
auditHandler.getAndDiscardMostRecentEvent(); // capture it only for failure
                                        if (session.isAuthorized()) {
                                                if (LOG.isDebugEnabled()) {
-                                                       
LOG.debug("evaluateAccess: has partial access (of some type) in family [" + 
family + "]");
+                                                       
LOG.debug("evaluateAccess: [" + family + "] descendants have access");
+                                               }
+                                               
familesAccessAllowed.add(family);
+                                               if (auditEvent != null) {
+                                                       
LOG.debug("evaluateAccess: adding to family-level-access-granted-event-set");
+                                                       
familyLevelAccessEvents.add(auditEvent);
                                                }
-                                               // we need to do 3 things: 
housekeeping, decide about audit events, building the results cache for filter
-                                               somethingIsAccessible = true;
-                                               
familesAccessIndeterminate.add(family);
                                        } else {
                                                if (LOG.isDebugEnabled()) {
-                                                       
LOG.debug("evaluateAccess: has no access of ["+ access + "] type in family [" + 
family + "]");
+                                                       
LOG.debug("evaluateAccess: has partial access (of some type) in family [" + 
family + "]");
                                                }
-                                               familesAccessDenied.add(family);
-                                               denialReason = 
String.format("Insufficient permissions for user ‘%s',action: %s, 
tableName:%s, family:%s.", user.getName(), operation, table, family);
+                                               everythingIsAccessible = false;
+                                               
familesAccessIndeterminate.add(family);
                                                if (auditEvent != null && 
deniedEvent == null) { // we need to capture just one denial event
                                                        
LOG.debug("evaluateAccess: Setting denied access audit event with last auth 
failure audit event.");
                                                        deniedEvent = 
auditEvent;
@@ -446,6 +432,17 @@ public class RangerAuthorizationCoprocessor extends 
RangerAuthorizationCoprocess
                                        }
                                        // Restore the headMatch setting
                                        
session.resourceMatchingScope(RangerAccessRequest.ResourceMatchingScope.SELF);
+                               } else {
+                                       if (LOG.isDebugEnabled()) {
+                                               LOG.debug("evaluateAccess: has 
no access of [" + access + "] type in family [" + family + "]");
+                                       }
+                                       everythingIsAccessible = false;
+                                       familesAccessDenied.add(family);
+                                       denialReason = 
String.format("Insufficient permissions for user ‘%s',action: %s, 
tableName:%s, family:%s.", user.getName(), operation, table, family);
+                                       if (auditEvent != null && deniedEvent 
== null) { // we need to capture just one denial event
+                                               LOG.debug("evaluateAccess: 
Setting denied access audit event with last auth failure audit event.");
+                                               deniedEvent = auditEvent;
+                                       }
                                }
                        } else {
                                LOG.debug("evaluateAccess: columns collection 
not empty.  Skipping Family level check, will do finer level access check.");

http://git-wip-us.apache.org/repos/asf/ranger/blob/ded33518/hbase-agent/src/test/java/org/apache/ranger/authorization/hbase/HBaseRangerAuthorizationTest.java
----------------------------------------------------------------------
diff --git 
a/hbase-agent/src/test/java/org/apache/ranger/authorization/hbase/HBaseRangerAuthorizationTest.java
 
b/hbase-agent/src/test/java/org/apache/ranger/authorization/hbase/HBaseRangerAuthorizationTest.java
index 198aad9..665640f 100644
--- 
a/hbase-agent/src/test/java/org/apache/ranger/authorization/hbase/HBaseRangerAuthorizationTest.java
+++ 
b/hbase-agent/src/test/java/org/apache/ranger/authorization/hbase/HBaseRangerAuthorizationTest.java
@@ -324,8 +324,9 @@ public class HBaseRangerAuthorizationTest {
                 // Read a row
                 try {
                     Get get = new Get(Bytes.toBytes("row1"));
-                    table.get(get);
-                    Assert.fail("Failure expected on an unauthorized user");
+                    Result result = table.get(get);
+                    byte[] valResult = 
result.getValue(Bytes.toBytes("colfam1"), Bytes.toBytes("col1"));
+                    Assert.assertNull("Failure expected on an unauthorized 
user", valResult);
                 } catch (IOException ex) {
                     // expected
                 }
@@ -533,8 +534,9 @@ public class HBaseRangerAuthorizationTest {
                 // Read a row
                 try {
                     Get get = new Get(Bytes.toBytes("row1"));
-                    table.get(get);
-                    Assert.fail("Failure expected on an unauthorized user");
+                    Result result = table.get(get);
+                    byte[] valResult = 
result.getValue(Bytes.toBytes("colfam2"), Bytes.toBytes("col1"));
+                    Assert.assertNull("Failure expected on an unauthorized 
user", valResult);
                 } catch (IOException ex) {
                     // expected
                 }
@@ -846,8 +848,9 @@ public class HBaseRangerAuthorizationTest {
 
                 Get get = new Get(Bytes.toBytes("row1"));
                 try {
-                    table.get(get);
-                    Assert.fail("Failure expected on an unauthorized user");
+                    Result result = table.get(get);
+                    byte[] valResult = 
result.getValue(Bytes.toBytes("colfam2"), Bytes.toBytes("col1"));
+                    Assert.assertNull("Failure expected on an unauthorized 
user", valResult);
                 } catch (IOException ex) {
                     // expected
                 }

Reply via email to