HBase
  1. HBase
  2. HBASE-11434

[AccessController] Disallow inbound cells with reserved tags

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.99.0, 0.98.4
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The AccessController allows users to store cells with ACL tags encoded by the client. This isn't a security issue currently, because in order to store the cell the user must have a relevant WRITE grant, and the user is allowed to specify whatever ACL for the cell they'd like. However it could become a correctness problem in the future, if we introduce format sanity checking or the like, so let's disallow inbound mutations containing cells with reserved tags like the VisibilityController does.

      The check is skipped if the active user is a superuser. First, superusers are allowed to do anything. Second, replication (as superuser) must be able to store incoming cells with ACL tags.

      1. HBASE-11434.patch
        9 kB
        Andrew Purtell
      2. HBASE-11434.patch
        9 kB
        Andrew Purtell
      3. HBASE-11434.patch
        11 kB
        Andrew Purtell
      4. HBASE-11434.patch
        11 kB
        Andrew Purtell

        Activity

        Hide
        Enis Soztutar added a comment -

        Closing this issue after 0.99.0 release.

        Show
        Enis Soztutar added a comment - Closing this issue after 0.99.0 release.
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #354 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/354/)
        HBASE-11434 [AccessController] Disallow inbound cells with reserved tags (apurtell: rev 54c186b42c3c77248783d2c3d9181c12e7b06802)

        • hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #354 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/354/ ) HBASE-11434 [AccessController] Disallow inbound cells with reserved tags (apurtell: rev 54c186b42c3c77248783d2c3d9181c12e7b06802) hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98 #374 (See https://builds.apache.org/job/HBase-0.98/374/)
        HBASE-11434 [AccessController] Disallow inbound cells with reserved tags (apurtell: rev 54c186b42c3c77248783d2c3d9181c12e7b06802)

        • hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #374 (See https://builds.apache.org/job/HBase-0.98/374/ ) HBASE-11434 [AccessController] Disallow inbound cells with reserved tags (apurtell: rev 54c186b42c3c77248783d2c3d9181c12e7b06802) hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-TRUNK #5270 (See https://builds.apache.org/job/HBase-TRUNK/5270/)
        HBASE-11434 [AccessController] Disallow inbound cells with reserved tags (apurtell: rev 80f1271ee5eb8957ff67040e95b2880257bda822)

        • hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-TRUNK #5270 (See https://builds.apache.org/job/HBase-TRUNK/5270/ ) HBASE-11434 [AccessController] Disallow inbound cells with reserved tags (apurtell: rev 80f1271ee5eb8957ff67040e95b2880257bda822) hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-1.0 #10 (See https://builds.apache.org/job/HBase-1.0/10/)
        HBASE-11434 [AccessController] Disallow inbound cells with reserved tags (apurtell: rev a8cbf2988b9153ade4d4ebf8b5eefe5420c5b41a)

        • hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-1.0 #10 (See https://builds.apache.org/job/HBase-1.0/10/ ) HBASE-11434 [AccessController] Disallow inbound cells with reserved tags (apurtell: rev a8cbf2988b9153ade4d4ebf8b5eefe5420c5b41a) hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
        Hide
        Andrew Purtell added a comment -

        Committed to 0.98+

        Show
        Andrew Purtell added a comment - Committed to 0.98+
        Hide
        Andrew Purtell added a comment -

        Looks like this is good to go, committing soon.

        Show
        Andrew Purtell added a comment - Looks like this is good to go, committing soon.
        Hide
        Enis Soztutar added a comment -

        +1 for branch-1.

        Show
        Enis Soztutar added a comment - +1 for branch-1.
        Hide
        Andrew Purtell added a comment -

        Replication tests in precommit were broken due to HBASE-11440. Resubmitting for QA anyhow.

        Show
        Andrew Purtell added a comment - Replication tests in precommit were broken due to HBASE-11440 . Resubmitting for QA anyhow.
        Hide
        Andrew Purtell added a comment -

        Ping Enis Soztutar for branch-1

        Show
        Andrew Purtell added a comment - Ping Enis Soztutar for branch-1
        Hide
        Anoop Sam John added a comment -

        +1

        Show
        Anoop Sam John added a comment - +1
        Hide
        Hadoop QA added a comment -

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

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

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

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

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

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

        -1 findbugs. The patch appears to introduce 13 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 mvn site goal succeeds with this patch.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.replication.TestReplicationDisableInactivePeer
        org.apache.hadoop.hbase.replication.TestMasterReplication

        -1 core zombie tests. There are 1 zombie test(s): at org.apache.hadoop.hbase.client.TestReplicaWithCluster.testChangeTable(TestReplicaWithCluster.java:217)

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12653463/HBASE-11434.patch against trunk revision . ATTACHMENT ID: 12653463 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 findbugs . The patch appears to introduce 13 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 mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.replication.TestReplicationDisableInactivePeer org.apache.hadoop.hbase.replication.TestMasterReplication -1 core zombie tests . There are 1 zombie test(s): at org.apache.hadoop.hbase.client.TestReplicaWithCluster.testChangeTable(TestReplicaWithCluster.java:217) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9927//console This message is automatically generated.
        Hide
        Andrew Purtell added a comment -

        Rebase patch on master, looks like a whitespace difference, maybe CRLF

        Show
        Andrew Purtell added a comment - Rebase patch on master, looks like a whitespace difference, maybe CRLF
        Hide
        Hadoop QA added a comment -

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

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

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

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9925//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12653456/HBASE-11434.patch against trunk revision . ATTACHMENT ID: 12653456 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified tests. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9925//console This message is automatically generated.
        Hide
        Andrew Purtell added a comment -

        Yes. New patch, with unit test.

        Show
        Andrew Purtell added a comment - Yes. New patch, with unit test.
        Hide
        Anoop Sam John added a comment -
        +    // set up the list of users with superuser privilege
        +    superusers = Lists.asList(userProvider.getCurrentUserName(),
        +      conf.getStrings(AccessControlLists.SUPERUSER_CONF_KEY, new String[0]));
        

        And UserProvider#getCurrentUserName() returns user.getName().
        We should use user.getShortName()

        And previously this List of users were creating by

        -    String currentUser = user.getShortName();
        -    List<String> superusers = Lists.asList(currentUser, conf.getStrings(
        -      AccessControlLists.SUPERUSER_CONF_KEY, new String[0]));
        

        So we used getShortName().

        Show
        Anoop Sam John added a comment - + // set up the list of users with superuser privilege + superusers = Lists.asList(userProvider.getCurrentUserName(), + conf.getStrings(AccessControlLists.SUPERUSER_CONF_KEY, new String [0])); And UserProvider#getCurrentUserName() returns user.getName(). We should use user.getShortName() And previously this List of users were creating by - String currentUser = user.getShortName(); - List< String > superusers = Lists.asList(currentUser, conf.getStrings( - AccessControlLists.SUPERUSER_CONF_KEY, new String [0])); So we used getShortName().
        Hide
        Andrew Purtell added a comment -

        Resubmit for QA

        Show
        Andrew Purtell added a comment - Resubmit for QA
        Hide
        Andrew Purtell added a comment -

        Thanks Anoop. Paste-o. New patch

        Show
        Andrew Purtell added a comment - Thanks Anoop. Paste-o. New patch
        Hide
        Anoop Sam John added a comment -
        +    // Superusers are allowed to store cells unconditionally
        +    if (!(superusers.contains(user.getShortName()))) {
        +      return;
        +    }
        

        Should be superusers.containc() check not the ! check right Andy?

        Show
        Anoop Sam John added a comment - + // Superusers are allowed to store cells unconditionally + if (!(superusers.contains(user.getShortName()))) { + return ; + } Should be superusers.containc() check not the ! check right Andy?
        Hide
        Andrew Purtell added a comment -

        Attached patch passes security unit tests locally.

        Show
        Andrew Purtell added a comment - Attached patch passes security unit tests locally.

          People

          • Assignee:
            Andrew Purtell
            Reporter:
            Andrew Purtell
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development