You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/12/23 12:46:09 UTC

[GitHub] [hbase] lujiefsi opened a new pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

lujiefsi opened a new pull request #2809:
URL: https://github.com/apache/hbase/pull/2809


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750906261


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  3s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 28s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 35s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 33s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 147m 17s |  hbase-server in the patch passed.  |
   |  |   | 173m 46s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 022bb7c0b8c4 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/4/testReport/ |
   | Max. process+thread count | 4218 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/4/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750875108


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 17s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 12s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 13s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 11s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 45s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 10s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  19m  6s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   2m 20s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  43m  8s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux fd19efdb05d8 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Max. process+thread count | 84 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/4/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750359871


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 27s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 28s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 52s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 43s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  7s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  7s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 56s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 154m 54s |  hbase-server in the patch passed.  |
   |  |   | 184m 36s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 7fab2fa76707 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/1/testReport/ |
   | Max. process+thread count | 3783 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] virajjasani commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750383118


   > Above code is the test code, but i found that RPCServe still use admin user
   > 
   > _protected void requirePermission(String request, Permission.Action perm) throws IOException { if (accessChecker != null) { accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm); } }_
   > RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user, this would not happen in real clauster.
   > If someone could point the root cause, plese leave a comment.
   
   In real cluster, if user is not Admin, call `requirePermission(requestName, Permission.Action.ADMIN)` will fail anyways.
   If you are worried about using some other user as part of unit test, yeah maybe we need some help.
   
   Let's wait for some time, if we don't find a way to repro our scenario using non-admin user, we can commit core changes in Rpc implementation as is.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750798217


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 48s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 32s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 40s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 30s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 36s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 34s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 138m  7s |  hbase-server in the patch failed.  |
   |  |   | 164m  8s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 03227aeed62c 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/testReport/ |
   | Max. process+thread count | 5002 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750802183


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 51s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 42s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 35s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   9m 21s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m  6s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 31s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 31s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 28s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 40s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 142m  5s |  hbase-server in the patch passed.  |
   |  |   | 178m  5s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux b732c40b3386 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/testReport/ |
   | Max. process+thread count | 3678 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751447508


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  2s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 23s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 58s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 35s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 32s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 33s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 34s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 145m 48s |  hbase-server in the patch passed.  |
   |  |   | 172m 11s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/7/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux d5e94ed2dd44 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/7/testReport/ |
   | Max. process+thread count | 4492 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/7/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751339719


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m 10s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 22s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 20s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 25s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 51s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 12s |  hbase-server: The patch generated 9 new + 19 unchanged - 0 fixed = 28 total (was 19)  |
   | -0 :warning: |  whitespace  |   0m  0s |  The patch 1 line(s) with tabs.  |
   | +1 :green_heart: |  hadoopcheck  |  19m  8s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   2m 20s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 11s |  The patch does not generate ASF License warnings.  |
   |  |   |  44m 52s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux c7c9b7384887 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | whitespace | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/artifact/yetus-general-check/output/whitespace-tabs.txt |
   | Max. process+thread count | 84 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test.
   
   _public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }_
   Above code is the test code, but i found that RPCServe still use admin user 
   _protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }_
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user.
   If someone could point the reason, plese leave a comment.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi removed a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi removed a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750340911


   > TestAccessController
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test.
   
   _public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }_
   
   Above code is the test code, but i found that RPCServe still use admin user 
   
   _protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }_
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user.
   If someone could point the root cause, plese leave a comment.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750903308


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 36s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  9s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  4s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 52s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 44s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  8s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 47s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 41s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 132m 11s |  hbase-server in the patch passed.  |
   |  |   | 160m 38s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 2cb90dc48f0b 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/4/testReport/ |
   | Max. process+thread count | 3641 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/4/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test, i will figure it out and push later.
   
   `  public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
   
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }`
   Above code is the test code, but i found that RPCServe still use admin user 
   ` protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }`
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751437198


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m 42s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 47s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 10s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 44s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 10s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 48s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   2m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 11s |  The patch does not generate ASF License warnings.  |
   |  |   |  43m 41s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/7/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 59bfbccf8d26 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Max. process+thread count | 84 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/7/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750306245


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   4m 30s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 49s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 25s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 33s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m  0s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  25m 46s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   2m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  56m 58s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 2cad9f8d5946 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Max. process+thread count | 84 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/1/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751634379


   > @lujiefsi Can you please raise PRs for branch-2, branch-2.4, branch-2.3 and branch-1? Although this PR can be backported directly, I just want to make sure your changes in `SecureTestUtil` doesn't cause regression for any existing tests.
   > Once we have positive QA result, all PRs can be merged together.
   
   Done:
   https://github.com/apache/hbase/pull/2817
   https://github.com/apache/hbase/pull/2818
   https://github.com/apache/hbase/pull/2819
   https://github.com/apache/hbase/pull/2820


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751347915


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 31s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  5s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 45s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 44s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 14s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  6s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  6s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m  0s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 41s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 131m 39s |  hbase-server in the patch passed.  |
   |  |   | 160m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux c70c4185c99a 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/testReport/ |
   | Max. process+thread count | 3797 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi closed pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi closed pull request #2809:
URL: https://github.com/apache/hbase/pull/2809


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750731066


   > > @virajjasani
   > > I try to use code "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster.
   > > I do not know why we get the wrong user at serve side, may be it is a bug in mini-cluster, but if that, we need a new pull request to fix it. Anyway, i attach the test code, if someone figures out the root cause, plese let me konw, thanks.
   > > @test
   > > public void testUnauthorizedSetTableStateInMeta() throws Exception {
   > > AccessTestAction action = new AccessTestAction() {
   > > @override public Object run() throws Exception {
   > > Hbck hbck = TEST_UTIL.getHbck();
   > > hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
   > > return null;
   > > }
   > > };
   > > verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
   > > USER_GROUP_WRITE, USER_GROUP_CREATE);
   > > }
   > > @test
   > > public void testUnauthorizedFixMeta() throws Exception {
   > > AccessTestAction action = new AccessTestAction() {
   > > @override public Object run() throws Exception {
   > > Hbck hbck = TEST_UTIL.getHbck();
   > > hbck.fixMeta();
   > > return null;
   > > }
   > > };
   > > verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
   > > USER_GROUP_WRITE, USER_GROUP_CREATE);
   > > }
   > 
   > Try creating the Connection by your own without using TEST_UTIL?
   > 
   > ```
   > try (Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()) {
   > } 
   > ```
   > 
   > Something like this.
   
   do not work


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test, i will figure it out and push later.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751451896


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 16s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 49s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 10s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  6s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 46s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 11s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 48s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   2m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 11s |  The patch does not generate ASF License warnings.  |
   |  |   |  42m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/8/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux b8dc74a1920f 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Max. process+thread count | 84 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/8/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751138386


   It seems that every is ok, The only question is can we add the fix for [HBASE-25441](https://issues.apache.org/jira/browse/HBASE-25441) in this pull reqeust?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750758632


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 44s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  8s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  3s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 25s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m  4s |  hbase-server: The patch generated 1 new + 16 unchanged - 0 fixed = 17 total (was 16)  |
   | -0 :warning: |  whitespace  |   0m  0s |  The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply  |
   | +1 :green_heart: |  hadoopcheck  |  17m 14s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   2m 10s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  38m 39s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 0a72e776c41b 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | whitespace | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/artifact/yetus-general-check/output/whitespace-eol.txt |
   | Max. process+thread count | 94 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/2/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on a change in pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#discussion_r548928637



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
##########
@@ -379,6 +381,36 @@ public void testUnauthorizedStopMaster() throws Exception {
         USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
+  @Test
+  public void testUnauthorizedSetTableStateInMeta() throws Exception {
+    AccessTestAction action = new AccessTestAction() {
+      @Override public Object run() throws Exception {
+        Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()) ;

Review comment:
       Please use try-with-resources to close the connection.

##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
##########
@@ -379,6 +381,36 @@ public void testUnauthorizedStopMaster() throws Exception {
         USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
+  @Test
+  public void testUnauthorizedSetTableStateInMeta() throws Exception {
+    AccessTestAction action = new AccessTestAction() {
+      @Override public Object run() throws Exception {
+        Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()) ;
+        Hbck hbck = conn.getHbck();
+        hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
+        return null;
+      }
+    };
+
+    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
+        USER_GROUP_WRITE, USER_GROUP_CREATE);
+  }
+
+  @Test
+  public void testUnauthorizedFixMeta() throws Exception {
+    AccessTestAction action = new AccessTestAction() {
+      @Override public Object run() throws Exception {
+        Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()) ;

Review comment:
       Ditto.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750747992


   Due to verifyDenied do not handle the RemoteWithExtrasException, so the AccessDeniedException are warpped and could not be seen by test. I have fixed it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750731066


   > > @virajjasani
   > > I try to use code "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster.
   > > I do not know why we get the wrong user at serve side, may be it is a bug in mini-cluster, but if that, we need a new pull request to fix it. Anyway, i attach the test code, if someone figures out the root cause, plese let me konw, thanks.
   > > @test
   > > public void testUnauthorizedSetTableStateInMeta() throws Exception {
   > > AccessTestAction action = new AccessTestAction() {
   > > @override public Object run() throws Exception {
   > > Hbck hbck = TEST_UTIL.getHbck();
   > > hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
   > > return null;
   > > }
   > > };
   > > verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
   > > USER_GROUP_WRITE, USER_GROUP_CREATE);
   > > }
   > > @test
   > > public void testUnauthorizedFixMeta() throws Exception {
   > > AccessTestAction action = new AccessTestAction() {
   > > @override public Object run() throws Exception {
   > > Hbck hbck = TEST_UTIL.getHbck();
   > > hbck.fixMeta();
   > > return null;
   > > }
   > > };
   > > verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
   > > USER_GROUP_WRITE, USER_GROUP_CREATE);
   > > }
   > 
   > Try creating the Connection by your own without using TEST_UTIL?
   > 
   > ```
   > try (Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()) {
   > } 
   > ```
   > 
   > Something like this.
   
   it works!!!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] virajjasani commented on a change in pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#discussion_r548946432



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
##########
@@ -379,6 +381,36 @@ public void testUnauthorizedStopMaster() throws Exception {
         USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
+  @Test
+  public void testUnauthorizedSetTableStateInMeta() throws Exception {
+    AccessTestAction action = new AccessTestAction() {
+      @Override public Object run() throws Exception {

Review comment:
       Let's use lambda here?
   ```
       AccessTestAction action = () -> {
   ...
   ...
   ...
         return null;
       };
   ```

##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
##########
@@ -379,6 +381,36 @@ public void testUnauthorizedStopMaster() throws Exception {
         USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
+  @Test
+  public void testUnauthorizedSetTableStateInMeta() throws Exception {
+    AccessTestAction action = new AccessTestAction() {
+      @Override public Object run() throws Exception {
+        Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()) ;
+        Hbck hbck = conn.getHbck();
+        hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
+        return null;
+      }
+    };
+
+    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
+        USER_GROUP_WRITE, USER_GROUP_CREATE);
+  }
+
+  @Test
+  public void testUnauthorizedFixMeta() throws Exception {
+    AccessTestAction action = new AccessTestAction() {
+      @Override public Object run() throws Exception {

Review comment:
       Same here




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test, i will figure it out and push later.
   
   `  public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
   
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }`
   Above code is the test code, but i found that RPCServe still use admin user 
   `  protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }`
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751634379


   > @lujiefsi Can you please raise PRs for branch-2, branch-2.4, branch-2.3 and branch-1? Although this PR can be backported directly, I just want to make sure your changes in `SecureTestUtil` doesn't cause regression for any existing tests.
   > Once we have positive QA result, all PRs can be merged together.
   
   YES, I will do it!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750815577


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 30s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  7s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 14s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 18s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 48s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 10s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  1s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  19m  6s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   2m 21s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 12s |  The patch does not generate ASF License warnings.  |
   |  |   |  43m 27s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux e3f55488a35e 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Max. process+thread count | 84 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/3/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750850478


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 50s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 30s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 22s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   9m  3s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 48s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m  2s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 27s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 39s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 44s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 132m 57s |  hbase-server in the patch failed.  |
   |  |   | 167m 25s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 665237e0f6e3 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/3/testReport/ |
   | Max. process+thread count | 4045 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/3/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] saintstack merged pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
saintstack merged pull request #2809:
URL: https://github.com/apache/hbase/pull/2809


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751446707


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 19s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 22s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 40s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 44s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  0s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  6s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  6s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 41s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 41s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 133m 56s |  hbase-server in the patch passed.  |
   |  |   | 162m 52s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/7/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 578f01da86c5 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/7/testReport/ |
   | Max. process+thread count | 4420 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/7/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test, i will figure it out and push later.
   
   public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }
   Above code is the test code, but i found that RPCServe still use admin user 
   protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751362399


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 18s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  9s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 51s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  6s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 42s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 131m 45s |  hbase-server in the patch failed.  |
   |  |   | 160m  3s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 63262eb96cea 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/6/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/6/testReport/ |
   | Max. process+thread count | 3684 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/6/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test, i will figure it out and push later.
   
   _public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }_
   Above code is the test code, but i found that RPCServe still use admin user 
   _protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }_
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user.
   If someone could point the reason, plese leave a comment.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751138386


   It seems that everything is ok, The only question is can we add the fix for [HBASE-25441](https://issues.apache.org/jira/browse/HBASE-25441) in this pull reqeust?
   
   Then I want to summay the reason   why previous UT fails, hope this can help others who write UT about AccessController.
   1. In the first version, I use _'TEST_UTIL.getHbck'_ to get client.   _'getHbck'_ use  '_getConnection_' to obtain the connection(**be careful here**). This connection is _systemUserConnection_(see TestAccessController#278), that means if you use this  connection, the user passed to RPCServer are always the system user, i.e. who run the UT, hence permission check are always passed.
   2. In the second version, i use _ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()_, to obtain the connection and use this connection to gain the hbck, i can pass the  user who really call the command to RPCServer. Checking the test log we can see that _AccessDeniedException_ is thrown, But UT still can not see it. After checking the code, i found that the exception **are warped into _RemoteWithExtrasException_** who are not handled in method _SecureTestUtil@verifyDenied_..   Other UT call the command on RPCServer directly, not through the RPC, so they do not meet this problem. In future, if we want to add more test about RPC security, we need address this problem. I have fixed this bug in this pull request by unwaping the _RemoteWithExtrasException_.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750732297


   > (hahaha, sorry. I'm overlapping with Duo :) )
   > 
   > > I try to use code "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster.
   > 
   > Look at `getAsyncConnection` in `HBaseTestingUtility.java`. Can you verify what `User` is here?
   > 
   > You could also just instantiate your own `Connection` and call `getHbck()` on that `Connection`, to avoid whatever HBaseTestingUtility is doing. The `TestAccessController#testRead()` method has an example of creating a new `Connection`.
   > 
   > Could you try that, please?
   
   The user is the user who run t
   
   > I think the fix is OK for now. Checked HBASE-19400, we want to move the permission check out of a CP but it is only half done...
   
   As you mention HBASE-19400, I check the patches and find that you add "requirePermission" in RS, but the API like stopServer do not call it for security check. I think there are must some other APIs on RSRpcServices need security check! I have created a issuse to track it.  https://issues.apache.org/jira/browse/HBASE-25441


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751352374


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 17s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 48s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 11s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  6s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 51s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 10s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 46s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   2m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 11s |  The patch does not generate ASF License warnings.  |
   |  |   |  42m 26s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/6/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux dd0c16bbb3dd 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Max. process+thread count | 84 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/6/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case,  ther still a problem  for unit test, i will figure it out.
   
   _public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }_
   
   Above code is the test code, but i found that RPCServe still use admin user 
   
   _protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }_
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user, this would not happen in real clauster.
   If someone could point the root cause, plese leave a comment.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750731066


   > > @virajjasani
   > > I try to use code "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster.
   > > I do not know why we get the wrong user at serve side, may be it is a bug in mini-cluster, but if that, we need a new pull request to fix it. Anyway, i attach the test code, if someone figures out the root cause, plese let me konw, thanks.
   > > @test
   > > public void testUnauthorizedSetTableStateInMeta() throws Exception {
   > > AccessTestAction action = new AccessTestAction() {
   > > @override public Object run() throws Exception {
   > > Hbck hbck = TEST_UTIL.getHbck();
   > > hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
   > > return null;
   > > }
   > > };
   > > verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
   > > USER_GROUP_WRITE, USER_GROUP_CREATE);
   > > }
   > > @test
   > > public void testUnauthorizedFixMeta() throws Exception {
   > > AccessTestAction action = new AccessTestAction() {
   > > @override public Object run() throws Exception {
   > > Hbck hbck = TEST_UTIL.getHbck();
   > > hbck.fixMeta();
   > > return null;
   > > }
   > > };
   > > verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
   > > USER_GROUP_WRITE, USER_GROUP_CREATE);
   > > }
   > 
   > Try creating the Connection by your own without using TEST_UTIL?
   > 
   > ```
   > try (Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()) {
   > } 
   > ```
   > 
   > Something like this.
   
   it works!!!But it seems that exception are not propagated to client. I will check it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test, i will figure it out and push later.
   
   _public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }_
   Above code is the test code, but i found that RPCServe still use admin user 
   _protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }_
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750849732


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 27s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 51s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 58s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 39s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 31s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 35s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 138m 47s |  hbase-server in the patch passed.  |
   |  |   | 164m 56s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux b2fc4813d340 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/3/testReport/ |
   | Max. process+thread count | 4886 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/3/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750363141


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 28s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 52s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 29s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 39s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 39s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 37s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 164m 59s |  hbase-server in the patch passed.  |
   |  |   | 192m  9s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 4ef8513f9549 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcb38f47db |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/1/testReport/ |
   | Max. process+thread count | 4042 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751363332


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 24s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 31s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 28s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 32s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 145m 22s |  hbase-server in the patch passed.  |
   |  |   | 171m 38s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 93ff4a1891a4 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/6/testReport/ |
   | Max. process+thread count | 3971 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/6/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751138386


   It seems that everything is ok, The only question is can we add the fix for [HBASE-25441](https://issues.apache.org/jira/browse/HBASE-25441) in this pull reqeust?
   
   Then I want to summay the reason   why previous UT fails, hope this can help others who write UT about AccessController.
   1. In the first version, I use _'TEST_UTIL.getHbck'_ to get client:  _'getHbck'_, then use its  '_getConnection_' to obtain the connection(**be careful here**). This connection is _systemUserConnection_(see TestAccessController#278), that means if you use this  connection, the user passed to RPCServer are always the system user, i.e. who run the UT, hence permission check are always passed.
   2. In the second version, i use _ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()_ to obtain the connection and use this connection to gain the hbck, i can pass the  user who really call the command to RPCServer. Checking the test log we can see that _AccessDeniedException_ is thrown, But UT still can not see it. After checking the code, i found that the exception **are warped into _RemoteWithExtrasException_** who are not handled in method _SecureTestUtil@verifyDenied_..   Other UT call the command on RPCServer directly, not through the RPC, so they do not meet this problem. In future, if we want to add more test about RPC security, we need address this problem. I have fixed this bug in this pull request by unwaping the _RemoteWithExtrasException_.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750380940


   @virajjasani 
   I try to use  code  "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster. 
   I do not know why we get the wrong user at serve side, may be it is a bug in mini-cluster, but if that, we need a new pull request to fix it. Anyway, i attach the test code,  if someone figures out the root cause, plese let me konw, thanks.
   
     @Test
     public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }
   
     @Test
     public void testUnauthorizedFixMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.fixMeta();
           return null;
         }
       };
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750710595


   > @virajjasani
   > I try to use code "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster.
   > I do not know why we get the wrong user at serve side, may be it is a bug in mini-cluster, but if that, we need a new pull request to fix it. Anyway, i attach the test code, if someone figures out the root cause, plese let me konw, thanks.
   > 
   > @test
   > public void testUnauthorizedSetTableStateInMeta() throws Exception {
   > AccessTestAction action = new AccessTestAction() {
   > @override public Object run() throws Exception {
   > Hbck hbck = TEST_UTIL.getHbck();
   > hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
   > return null;
   > }
   > };
   > verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
   > USER_GROUP_WRITE, USER_GROUP_CREATE);
   > }
   > 
   > @test
   > public void testUnauthorizedFixMeta() throws Exception {
   > AccessTestAction action = new AccessTestAction() {
   > @override public Object run() throws Exception {
   > Hbck hbck = TEST_UTIL.getHbck();
   > hbck.fixMeta();
   > return null;
   > }
   > };
   > verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
   > USER_GROUP_WRITE, USER_GROUP_CREATE);
   > }
   
   Try creating the Connection by your own without using TEST_UTIL?
   
   ```
   try (Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()) {
   } 
   ```
   
   Something like this.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750866026


   rebase the commit


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750732797


   > (hahaha, sorry. I'm overlapping with Duo :) )
   > 
   > > I try to use code "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster.
   > 
   > Look at `getAsyncConnection` in `HBaseTestingUtility.java`. Can you verify what `User` is here?
   > 
   > You could also just instantiate your own `Connection` and call `getHbck()` on that `Connection`, to avoid whatever HBaseTestingUtility is doing. The `TestAccessController#testRead()` method has an example of creating a new `Connection`.
   > 
   > Could you try that, please?
   
   I have inserted a log to find what User is here. I found that the user is 'lujie' who runs the test.
   
   2020-12-24 12:07:00,160 INFO  [RpcServer.default.FPBQ.Fifo.handler=2,queue=0,port=46723] regionserver.RSRpcServices(1301): the user name is lujie (auth:SIMPLE)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751138386


   It seems that everything is ok, The only question is can we add the fix for [HBASE-25441](https://issues.apache.org/jira/browse/HBASE-25441) in this pull reqeust?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750710121


   I think the fix is OK for now. Checked HBASE-19400, we want to move the permission check out of a CP but it is only half done...


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] joshelser edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
joshelser edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750710601


   (hahaha, sorry. I'm overlapping with Duo :) )
   
   > I try to use code "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster.
   
   Look at `getAsyncConnection` in `HBaseTestingUtility.java`. Can you verify what `User` is here?
   
   You could also just instantiate your own `Connection` and call `getHbck()` on that `Connection`, to avoid whatever HBaseTestingUtility is doing. The `TestAccessController#testRead()` method has an example of creating a new `Connection`.
   
   Could you try that, please?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751464390


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  3s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 24s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 55s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 37s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 34s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 39s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 145m 26s |  hbase-server in the patch passed.  |
   |  |   | 171m 59s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 7781eb9b6f02 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/8/testReport/ |
   | Max. process+thread count | 4504 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/8/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750340911


   > TestAccessController
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750382791


   By the way, i do not find any UT about rpcPreCheck, we need one.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751138386


   It seems that everything is ok, The only question is can we add the fix for [HBASE-25441](https://issues.apache.org/jira/browse/HBASE-25441) in this pull reqeust?
   
   Then I want to summay the reason   why previous UT fails, hope this can help others who write UT about AccessController.
   1. In the first version, I use _'TEST_UTIL.getHbck'_ to get client:  _'getHbck'_, then use its  '_getConnection_' to obtain the connection(**be careful here**). This connection is _systemUserConnection_(see TestAccessController#278), that means if you use this  connection, the user passed to RPCServer are always the system user, i.e. who run the UT, hence permission check are always passed.
   2. In the second version, i use _ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()_, to obtain the connection and use this connection to gain the hbck, i can pass the  user who really call the command to RPCServer. Checking the test log we can see that _AccessDeniedException_ is thrown, But UT still can not see it. After checking the code, i found that the exception **are warped into _RemoteWithExtrasException_** who are not handled in method _SecureTestUtil@verifyDenied_..   Other UT call the command on RPCServer directly, not through the RPC, so they do not meet this problem. In future, if we want to add more test about RPC security, we need address this problem. I have fixed this bug in this pull request by unwaping the _RemoteWithExtrasException_.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750380940


   @virajjasani 
   I try to use  code  "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster. 
   I do not know why we get the wrong user at serve side, may be it is a bug in mini-cluster, but if that, we need a new pull request to fix it. Anyway, i attach the test code,  if someone figures out the root cause, plese let me konw, thanks.
   
     @Test
     public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
   
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
      
     }
   
     @Test
     public void testUnauthorizedFixMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.fixMeta();
           return null;
         }
       };
   
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] virajjasani commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751631979


   @lujiefsi Can you please raise PRs for branch-2, branch-2.4, branch-2.3 and branch-1? Although this PR can be backported directly, I just want to make sure your changes in `SecureTestUtil` doesn't cause regression for any existing tests. 
   Once we have positive QA result, all PRs can be merged together.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751463191


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 31s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 47s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 40s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 36s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  8s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 44s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 130m 40s |  hbase-server in the patch passed.  |
   |  |   | 159m 35s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 72844166c612 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/8/testReport/ |
   | Max. process+thread count | 3712 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/8/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751138386


   It seems that everything is ok, The only question is can we add the fix for [HBASE-25441](https://issues.apache.org/jira/browse/HBASE-25441) in this pull reqeust?
   
   Then I want to summay the reason   why previous UT fails, hope this can help others who write UT about AccessController.
   1. In the first version, I use _'TEST_UTIL.getHbck'_ to get client.   _'getHbck'_ and use its  '_getConnection_' to obtain the connection(**be careful here**). This connection is _systemUserConnection_(see TestAccessController#278), that means if you use this  connection, the user passed to RPCServer are always the system user, i.e. who run the UT, hence permission check are always passed.
   2. In the second version, i use _ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()_, to obtain the connection and use this connection to gain the hbck, i can pass the  user who really call the command to RPCServer. Checking the test log we can see that _AccessDeniedException_ is thrown, But UT still can not see it. After checking the code, i found that the exception **are warped into _RemoteWithExtrasException_** who are not handled in method _SecureTestUtil@verifyDenied_..   Other UT call the command on RPCServer directly, not through the RPC, so they do not meet this problem. In future, if we want to add more test about RPC security, we need address this problem. I have fixed this bug in this pull request by unwaping the _RemoteWithExtrasException_.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test, i will figure it out and push later.
   
   `public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
   
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }`
   Above code is the test code, but i found that RPCServe still use admin user 
   ` protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }`
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-751348855


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  2s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 49s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 58s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 34s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 39s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 30s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 37s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 145m 53s |  hbase-server in the patch passed.  |
   |  |   | 172m 48s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2809 |
   | JIRA Issue | HBASE-25432 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 244a89b0c253 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0f868da05d |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/testReport/ |
   | Max. process+thread count | 4323 (vs. ulimit of 30000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2809/5/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] joshelser commented on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
joshelser commented on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750710601


   > I try to use code "hbck = TEST_UTIL.getConnection(USER_NONE).getHbck()" to gain the hbck, but UT still failed to pass. So I perfer to commit this PR without UT as its patch works fine in a real cluster.
   
   Look at `getAsyncConnection` in `HBaseTestingUtility.java`. Can you verify what `User` is here?
   
   You could also just instantiate your own `Connection` and call `getHbck()` on that `Connection`, to avoid whatever HBaseTestingUtility is doing. The `TestAccessController#testRead()` method has an example of creating a new `Connection`.
   
   Could you try that, please?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lujiefsi edited a comment on pull request #2809: HBASE-25432:add security checks for setTableStateInMeta and fixMeta

Posted by GitBox <gi...@apache.org>.
lujiefsi edited a comment on pull request #2809:
URL: https://github.com/apache/hbase/pull/2809#issuecomment-750342006


   @virajjasani 
   I will add a test case, but i find that there maybe a bug in RPC for test.
   
   _public void testUnauthorizedSetTableStateInMeta() throws Exception {
       AccessTestAction action = new AccessTestAction() {
         @Override public Object run() throws Exception {
           Hbck hbck = TEST_UTIL.getHbck();
           hbck.setTableStateInMeta(new TableState(TEST_TABLE, TableState.State.DISABLED));
           return null;
         }
       };
       verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ,
           USER_GROUP_WRITE, USER_GROUP_CREATE);
     }_
   
   Above code is the test code, but i found that RPCServe still use admin user 
   
   _protected void requirePermission(String request, Permission.Action perm) throws IOException {
       if (accessChecker != null) {
         accessChecker.requirePermission(RpcServer.getRequestUser().orElse(null), request, null, perm);
       }
     }_
   RpcServer.getRequestUser().orElse(null) returns admin user, not non-admin user.
   If someone could point the reason, plese leave a comment.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org