HBase
  1. HBase
  2. HBASE-5352 ACL improvements
  3. HBASE-6061

Fix ACL "Admin" Table inconsistent permission check

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.92.1, 0.94.0, 0.95.2
    • Fix Version/s: 0.94.1, 0.95.0
    • Component/s: security
    • Labels:
    • Hadoop Flags:
      Reviewed

      Description

      the requirePermission() check for "admin" operation on a table is currently inconsistent.

      Table Owner with CREATE rights (that means, the owner has created that table) can enable/disable and delete the table but needs ADMIN rights to add/remove/modify a column.

      1. HBASE-6061-0.92.patch
        4 kB
        Matteo Bertozzi
      2. HBASE-6061-v0.patch
        4 kB
        Matteo Bertozzi
      3. HBASE-6061-v1.patch
        4 kB
        Matteo Bertozzi

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in HBase-0.92-security #108 (See https://builds.apache.org/job/HBase-0.92-security/108/)
          HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341268)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/branches/0.92/CHANGES.txt
          • /hbase/branches/0.92/security/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Show
          Hudson added a comment - Integrated in HBase-0.92-security #108 (See https://builds.apache.org/job/HBase-0.92-security/108/ ) HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341268) Result = FAILURE tedyu : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/security/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Hide
          Laxman added a comment -

          Thanks Ted. Filed under HBASE-6086.

          Show
          Laxman added a comment - Thanks Ted. Filed under HBASE-6086 .
          Hide
          Matteo Bertozzi added a comment -

          @Laxman

          Still some inconsistency exists after patch. We actually need to check for table permissions instead of global permissions here.

          Is a different permission check not an inconsistent state...
          HBASE-5372 was opened to change from global rights to table rights.

          Show
          Matteo Bertozzi added a comment - @Laxman Still some inconsistency exists after patch. We actually need to check for table permissions instead of global permissions here. Is a different permission check not an inconsistent state... HBASE-5372 was opened to change from global rights to table rights.
          Hide
          Ted Yu added a comment -

          Opening another issue is fine.

          Show
          Ted Yu added a comment - Opening another issue is fine.
          Hide
          Laxman added a comment -

          Sorry for late reporting.

          Still some inconsistency exists after patch. We actually need to check for table permissions instead of global permissions here.

          +  private void requireTableAdminPermission(MasterCoprocessorEnvironment e,
          +      byte[] tableName) throws IOException {
          +    if (isActiveUserTableOwner(e, tableName)) {
          +      requirePermission(Permission.Action.CREATE);
          +    } else {
          +      requirePermission(Permission.Action.ADMIN);
          +    }
          +  }
          

          I think this needs to be handled as separate jira.

          Show
          Laxman added a comment - Sorry for late reporting. Still some inconsistency exists after patch. We actually need to check for table permissions instead of global permissions here. + private void requireTableAdminPermission(MasterCoprocessorEnvironment e, + byte [] tableName) throws IOException { + if (isActiveUserTableOwner(e, tableName)) { + requirePermission(Permission.Action.CREATE); + } else { + requirePermission(Permission.Action.ADMIN); + } + } I think this needs to be handled as separate jira.
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94-security #28 (See https://builds.apache.org/job/HBase-0.94-security/28/)
          HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341267)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/branches/0.94/security/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Show
          Hudson added a comment - Integrated in HBase-0.94-security #28 (See https://builds.apache.org/job/HBase-0.94-security/28/ ) HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341267) Result = FAILURE tedyu : Files : /hbase/branches/0.94/security/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Hide
          Hudson added a comment -

          Integrated in HBase-0.92 #416 (See https://builds.apache.org/job/HBase-0.92/416/)
          HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341268)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/branches/0.92/CHANGES.txt
          • /hbase/branches/0.92/security/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Show
          Hudson added a comment - Integrated in HBase-0.92 #416 (See https://builds.apache.org/job/HBase-0.92/416/ ) HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341268) Result = FAILURE tedyu : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/security/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94 #207 (See https://builds.apache.org/job/HBase-0.94/207/)
          HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341267)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/branches/0.94/security/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Show
          Hudson added a comment - Integrated in HBase-0.94 #207 (See https://builds.apache.org/job/HBase-0.94/207/ ) HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341267) Result = FAILURE tedyu : Files : /hbase/branches/0.94/security/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #2914 (See https://builds.apache.org/job/HBase-TRUNK/2914/)
          HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341265)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #2914 (See https://builds.apache.org/job/HBase-TRUNK/2914/ ) HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341265) Result = FAILURE tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #13 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/13/)
          HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341265)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #13 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/13/ ) HBASE-6061 Fix ACL "Admin" Table inconsistent permission check (Matteo Bertozzi) (Revision 1341265) Result = FAILURE tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          Hide
          Ted Yu added a comment -

          Integrated to 0.92, 0.94 and trunk.

          Thanks for the patch, Matteo.

          Thanks for the review, Andy.

          Show
          Ted Yu added a comment - Integrated to 0.92, 0.94 and trunk. Thanks for the patch, Matteo. Thanks for the review, Andy.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12528508/HBASE-6061-0.92.patch
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 hadoop23. The patch compiles against the hadoop 0.23.x 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 appears to introduce 33 new Findbugs (version 1.3.9) warnings.

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

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.replication.TestReplication
          org.apache.hadoop.hbase.regionserver.TestSplitTransactionOnCluster
          org.apache.hadoop.hbase.replication.TestMultiSlaveReplication
          org.apache.hadoop.hbase.replication.TestMasterReplication

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1951//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1951//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1951//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/12528508/HBASE-6061-0.92.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop23. The patch compiles against the hadoop 0.23.x 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 appears to introduce 33 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.replication.TestReplication org.apache.hadoop.hbase.regionserver.TestSplitTransactionOnCluster org.apache.hadoop.hbase.replication.TestMultiSlaveReplication org.apache.hadoop.hbase.replication.TestMasterReplication Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1951//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1951//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1951//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12528491/HBASE-6061-v1.patch
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 hadoop23. The patch compiles against the hadoop 0.23.x 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 appears to introduce 33 new Findbugs (version 1.3.9) warnings.

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

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.replication.TestReplication
          org.apache.hadoop.hbase.replication.TestMultiSlaveReplication
          org.apache.hadoop.hbase.replication.TestMasterReplication
          org.apache.hadoop.hbase.master.TestSplitLogManager

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1950//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1950//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1950//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/12528491/HBASE-6061-v1.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop23. The patch compiles against the hadoop 0.23.x 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 appears to introduce 33 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.replication.TestReplication org.apache.hadoop.hbase.replication.TestMultiSlaveReplication org.apache.hadoop.hbase.replication.TestMasterReplication org.apache.hadoop.hbase.master.TestSplitLogManager Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1950//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1950//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1950//console This message is automatically generated.
          Hide
          Matteo Bertozzi added a comment -

          Attached the 0.92 patch, also good for 0.94

          Show
          Matteo Bertozzi added a comment - Attached the 0.92 patch, also good for 0.94
          Hide
          Ted Yu added a comment -

          @Matteo:
          Do you mind providing patch for 0.92 / 0.94 ?
          The directory structure has changed.

          Show
          Ted Yu added a comment - @Matteo: Do you mind providing patch for 0.92 / 0.94 ? The directory structure has changed.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12528475/HBASE-6061-v0.patch
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 hadoop23. The patch compiles against the hadoop 0.23.x 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 appears to introduce 33 new Findbugs (version 1.3.9) warnings.

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

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1947//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1947//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1947//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/12528475/HBASE-6061-v0.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop23. The patch compiles against the hadoop 0.23.x 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 appears to introduce 33 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.replication.TestReplication org.apache.hadoop.hbase.replication.TestMultiSlaveReplication org.apache.hadoop.hbase.replication.TestMasterReplication Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1947//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1947//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1947//console This message is automatically generated.
          Hide
          Andrew Purtell added a comment -

          Not related but maybe we can squeeze into this one... preCheckAndPut() and preCheckAndDelete() checks for READ when they also want to WRITE

          Yes, new jira.

          Show
          Andrew Purtell added a comment - Not related but maybe we can squeeze into this one... preCheckAndPut() and preCheckAndDelete() checks for READ when they also want to WRITE Yes, new jira.
          Hide
          Matteo Bertozzi added a comment -

          Not related but maybe we can squeeze into this one... preCheckAndPut() and preCheckAndDelete() checks for READ when they also want to WRITE...
          for me checking for WRITE permission is the right thing... what do you say about that? keep READ, replace with WRITE.. open new jira?

          Show
          Matteo Bertozzi added a comment - Not related but maybe we can squeeze into this one... preCheckAndPut() and preCheckAndDelete() checks for READ when they also want to WRITE... for me checking for WRITE permission is the right thing... what do you say about that? keep READ, replace with WRITE.. open new jira?
          Hide
          Andrew Purtell added a comment -

          +1 yes, this is better, since the direction here is to let the creator take any action on the table, pulling up the logic to a small helper method is cleaner, fixes the issue, and will avoid error going forward.

          Show
          Andrew Purtell added a comment - +1 yes, this is better, since the direction here is to let the creator take any action on the table, pulling up the logic to a small helper method is cleaner, fixes the issue, and will avoid error going forward.
          Hide
          Ted Yu added a comment -

          Minor comment:

          +   * If current user is the table owner, and has CREATE permission is a table admin,
          

          ', and has CREATE permission is a table admin' -> ' and has CREATE permission, then he/she has table admin permission.' (wrap if line is too long)

          Show
          Ted Yu added a comment - Minor comment: + * If current user is the table owner, and has CREATE permission is a table admin, ', and has CREATE permission is a table admin' -> ' and has CREATE permission, then he/she has table admin permission.' (wrap if line is too long)

            People

            • Assignee:
              Matteo Bertozzi
              Reporter:
              Matteo Bertozzi
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development