[HBASE-10326] Super user should be able scan all the cells irrespective of the visibility labels - ASF JIRA

Details

Type: Bug
Status: Resolved
Priority: Critical
Resolution: Fixed
Affects Version/s: 0.98.0
Fix Version/s: 0.98.0, 0.99.0
Component/s: security
Labels:
- security

Hadoop Flags:

Reviewed
Release Note:
HBase super user can (any user who is having system visibility label) read back all the cells irrespective of visibility expression applied for cells.
Tags:
visibility

Description

This issue is in lieu with ~~HBASE-10322~~. In case of export tool, when the cells with visibility labels are exported using a super user we should be able to export the data. But with the current implementation, the super user would also be able to view cells that has visibility labels associated with the superuser. The idea of ~~HBASE-10322~~ is to strip out tags based on user and if so this change is necessary for export tool to work with Visibility. ACL already has a concept of global admins.

Options
- Sort By Name
- Sort By Date
- Ascending
- Descending

Attachments

HBASE-10326_1.patch

13/Jan/14 08:51

23 kB

ramkrishna.s.vasudevan
HBASE-10326.patch

13/Jan/14 05:39

23 kB

ramkrishna.s.vasudevan

Issue Links

is related to

HBASE-10322 Strip tags from KV while sending back to client on reads

Resolved

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

ramkrishna.s.vasudevan added a comment - 13/Jan/14 05:39

Running testcases. Tests involving Visibility controller passes.

Show

ramkrishna.s.vasudevan added a comment - 13/Jan/14 05:39 Running testcases. Tests involving Visibility controller passes.

Hide

Permalink

Hadoop QA added a comment - 13/Jan/14 07:03

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12622582/HBASE-10326.patch
against trunk revision .
ATTACHMENT ID: 12622582

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 6 new or modified tests.

+1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

+1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 lineLengths. The patch introduces the following lines longer than 100:
+ // If a super user issues a scan, he should be able to scan the cells irrespective of the Visibility labels
+ // If a super user issues a get, he should be able to scan the cells irrespective of the Visibility labels
+ PrivilegedExceptionAction<VisibilityLabelsResponse> action = new PrivilegedExceptionAction<VisibilityLabelsResponse>() {

-1 site. The patch appears to cause mvn site goal to fail.

+1 core tests. The patch passed unit tests in .

This message is automatically generated.

Show

Hadoop QA added a comment - 13/Jan/14 07:03 -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12622582/HBASE-10326.patch against trunk revision . ATTACHMENT ID: 12622582 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + // If a super user issues a scan, he should be able to scan the cells irrespective of the Visibility labels + // If a super user issues a get, he should be able to scan the cells irrespective of the Visibility labels + PrivilegedExceptionAction<VisibilityLabelsResponse> action = new PrivilegedExceptionAction<VisibilityLabelsResponse>() { -1 site . The patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8400//console This message is automatically generated.

Hide

Permalink

Hadoop QA added a comment - 13/Jan/14 10:23

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12622599/HBASE-10326_1.patch
against trunk revision .
ATTACHMENT ID: 12622599

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 6 new or modified tests.

+1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

+1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

-1 site. The patch appears to cause mvn site goal to fail.

+1 core tests. The patch passed unit tests in .

This message is automatically generated.

Show

Hadoop QA added a comment - 13/Jan/14 10:23 -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12622599/HBASE-10326_1.patch against trunk revision . ATTACHMENT ID: 12622599 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 -1 site . The patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8401//console This message is automatically generated.

Hide

Permalink

Anoop Sam John added a comment - 13/Jan/14 10:27

Patch looks good Ram.
Pls correct the white spaces introduced after checkIfScanOrGetFromSuperUser private method.

+    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    try {
+      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
+      AccessControlService.BlockingInterface protocol = AccessControlService
+          .newBlockingStub(service);
+      ProtobufUtil.grant(protocol, NORMAL_USER2.getShortName(), tableName, null, null,
+          Permission.Action.READ);
+    } finally {
+      acl.close();
+    }

Instead can use AccessControlClient#grant ? This code is repeated in tests..

Thanks for the patch.

Show

Anoop Sam John added a comment - 13/Jan/14 10:27 Patch looks good Ram. Pls correct the white spaces introduced after checkIfScanOrGetFromSuperUser private method. + HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME); + try { + BlockingRpcChannel service = acl.coprocessorService(tableName.getName()); + AccessControlService.BlockingInterface protocol = AccessControlService + .newBlockingStub(service); + ProtobufUtil.grant(protocol, NORMAL_USER2.getShortName(), tableName, null , null , + Permission.Action.READ); + } finally { + acl.close(); + } Instead can use AccessControlClient#grant ? This code is repeated in tests.. Thanks for the patch.

Hide

Permalink

Andrew Purtell added a comment - 13/Jan/14 17:18 - edited

Instead can use AccessControlClient#grant ? This code is repeated in tests..

Or use the new grant/revoke methods in SecureTestUtils, which are designed for granting or revoking in tests. They do things only possible in miniclusters to insure the AC has propagated the grant to all caches first, to avoid flapping tests.

Are the changes to TestVisibilityLabels needed? The test runs under the superuser implicitly right? There is no functional change though, would be fine to keep them.

What do the new tests in TestVisibilityLabelsWithACL do? Comment, please.

Show

Andrew Purtell added a comment - 13/Jan/14 17:18 - edited Instead can use AccessControlClient#grant ? This code is repeated in tests.. Or use the new grant/revoke methods in SecureTestUtils, which are designed for granting or revoking in tests. They do things only possible in miniclusters to insure the AC has propagated the grant to all caches first, to avoid flapping tests. Are the changes to TestVisibilityLabels needed? The test runs under the superuser implicitly right? There is no functional change though, would be fine to keep them. What do the new tests in TestVisibilityLabelsWithACL do? Comment, please.

Hide

Permalink

Andrew Purtell added a comment - 13/Jan/14 17:20

Anoop Sam John, Ram mailed me that he is away this evening. I would be +1 for a commit of this patch without the test changes. What do you think? We can add the test changes later as an addendum or new JIRA.

Show

Andrew Purtell added a comment - 13/Jan/14 17:20 Anoop Sam John , Ram mailed me that he is away this evening. I would be +1 for a commit of this patch without the test changes. What do you think? We can add the test changes later as an addendum or new JIRA.

Hide

Permalink

Anoop Sam John added a comment - 13/Jan/14 17:41 - edited

I will commit patch as it is now.. We can improve the tests later as you suggested.

Show

Anoop Sam John added a comment - 13/Jan/14 17:41 - edited I will commit patch as it is now.. We can improve the tests later as you suggested.

Hide

Permalink

Anoop Sam John added a comment - 13/Jan/14 17:54

Committed to Trunk and 0.98. Thanks for the patch Ram. Thanks for the review Andy.

Show

Anoop Sam John added a comment - 13/Jan/14 17:54 Committed to Trunk and 0.98. Thanks for the patch Ram. Thanks for the review Andy.

Hide

Permalink

Andrew Purtell added a comment - 13/Jan/14 18:02

Then I will fix the tests now. ~~HBASE-10331~~

Show

Andrew Purtell added a comment - 13/Jan/14 18:02 Then I will fix the tests now. HBASE-10331

Hide

Permalink

Hudson added a comment - 13/Jan/14 20:15

SUCCESS: Integrated in HBase-0.98 #73 (See https://builds.apache.org/job/HBase-0.98/73/)
~~HBASE-10326~~ Super user should be able scan all the cells irrespective of the visibility labels(Ram) (anoopsamjohn: rev 1557791)

/hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
/hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java
/hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithACL.java

Show

Hudson added a comment - 13/Jan/14 20:15 SUCCESS: Integrated in HBase-0.98 #73 (See https://builds.apache.org/job/HBase-0.98/73/ ) HBASE-10326 Super user should be able scan all the cells irrespective of the visibility labels(Ram) (anoopsamjohn: rev 1557791) /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithACL.java

Hide

Permalink

Hudson added a comment - 13/Jan/14 20:15

SUCCESS: Integrated in HBase-TRUNK #4810 (See https://builds.apache.org/job/HBase-TRUNK/4810/)
~~HBASE-10326~~ Super user should be able scan all the cells irrespective of the visibility labels(Ram) (anoopsamjohn: rev 1557792)

/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java
/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithACL.java

Show

Hudson added a comment - 13/Jan/14 20:15 SUCCESS: Integrated in HBase-TRUNK #4810 (See https://builds.apache.org/job/HBase-TRUNK/4810/ ) HBASE-10326 Super user should be able scan all the cells irrespective of the visibility labels(Ram) (anoopsamjohn: rev 1557792) /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithACL.java

Hide

Permalink

Hudson added a comment - 13/Jan/14 20:39

SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #68 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/68/)
~~HBASE-10326~~ Super user should be able scan all the cells irrespective of the visibility labels(Ram) (anoopsamjohn: rev 1557791)

/hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
/hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java
/hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithACL.java

Show

Hudson added a comment - 13/Jan/14 20:39 SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #68 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/68/ ) HBASE-10326 Super user should be able scan all the cells irrespective of the visibility labels(Ram) (anoopsamjohn: rev 1557791) /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithACL.java

Hide

Permalink

Hudson added a comment - 14/Jan/14 01:13

SUCCESS: Integrated in HBase-TRUNK-on-Hadoop-1.1 #52 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-1.1/52/)
~~HBASE-10326~~ Super user should be able scan all the cells irrespective of the visibility labels(Ram) (anoopsamjohn: rev 1557792)

/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java
/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithACL.java

Show

Hudson added a comment - 14/Jan/14 01:13 SUCCESS: Integrated in HBase-TRUNK-on-Hadoop-1.1 #52 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-1.1/52/ ) HBASE-10326 Super user should be able scan all the cells irrespective of the visibility labels(Ram) (anoopsamjohn: rev 1557792) /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithACL.java

People

Assignee:

ramkrishna.s.vasudevan

Reporter:

ramkrishna.s.vasudevan

Votes:

0 Vote for this issue

Watchers:

5 Start watching this issue

Dates

Created:

13/Jan/14 04:29

Updated:

14/Jan/14 01:13

Resolved:

13/Jan/14 17:54

Development

Agile

View on Board