You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "anmolnar (via GitHub)" <gi...@apache.org> on 2024/11/06 22:12:08 UTC
[PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
anmolnar opened a new pull request, #6447:
URL: https://github.com/apache/hbase/pull/6447
Added new coproc hook to `ServerRpcConnection` right after authorization is done. Support both Master and RS client connections. Coprocs are allowed to throw `AccessDeniedException` if connection should not be established. Targeted primarly for X509 cert based authentication.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1837076315
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RpcObserver.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.coprocessor;
+
+import java.io.IOException;
+import java.security.cert.X509Certificate;
+import org.apache.hadoop.hbase.HBaseInterfaceAudience;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.yetus.audience.InterfaceStability;
+
+/**
+ * Coprocessors implement this interface to observe and mediate RPC events in Master and RS
+ * instances.
+ * <p>
+ * Since most implementations will be interested in only a subset of hooks, this class uses
+ * 'default' functions to avoid having to add unnecessary overrides. When the functions are
+ * non-empty, it's simply to satisfy the compiler by returning value of expected (non-void) type. It
+ * is done in a way that these default definitions act as no-op. So our suggestion to implementation
+ * would be to not call these 'default' methods from overrides.
+ * <p>
+ * <h3>Exception Handling</h3><br>
+ * For all functions, exception handling is done as follows:
+ * <ul>
+ * <li>Exceptions of type {@link IOException} are reported back to client.</li>
+ * <li>For any other kind of exception:
+ * <ul>
+ * <li>Be aware that this coprocessor doesn't support abortion. If the configuration
+ * {@link CoprocessorHost#ABORT_ON_ERROR_KEY} is set to true, the event will be logged, but the RPC
+ * server won't be aborted.</li>
+ * <li>Otherwise, coprocessor is removed from the server.</li>
+ * </ul>
+ * </li>
+ * </ul>
+ */
+@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.COPROC)
+@InterfaceStability.Evolving
+public interface RpcObserver {
+ /**
+ * Called after successfully authorizing connection
+ * @param ctx the coprocessor instance's environment
+ * @param userName the user name
+ * @param clientCertificateChain list of peer certificates from SSL connection
+ */
+ default void postAuthorizeConnection(ObserverContext<RpcCoprocessorEnvironment> ctx,
Review Comment:
Trying to keep it simple, not needed atm. Though should be easy to add the `preHook` call before `authorizeConnection()`. Shall I?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2550336890
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 38s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 42s | | master passed |
| +1 :green_heart: | compile | 1m 11s | | master passed |
| +1 :green_heart: | javadoc | 0m 35s | | master passed |
| +1 :green_heart: | shadedjars | 6m 25s | | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | patch | 6m 34s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 44s | | the patch passed |
| +1 :green_heart: | compile | 1m 17s | | the patch passed |
| +1 :green_heart: | javac | 1m 17s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 42s | | the patch passed |
| +1 :green_heart: | shadedjars | 7m 18s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 218m 20s | | hbase-server in the patch passed. |
| | | 248m 8s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/8/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 6c39e8ab15b5 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dd5f2042a892d10b5c4e4f98f2d90ba1de5a5483 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/8/testReport/ |
| Max. process+thread count | 4853 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/8/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2501943317
ping @Apache9 @bbeaudreault
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1837072478
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java:
##########
@@ -898,4 +911,14 @@ protected boolean needAuthorization() {
public List<BlockingServiceAndInterface> getServices() {
return services;
}
+
+ private void initializeCoprocessorHost(Configuration conf) {
+ // initialize master side coprocessors before we start handling requests
Review Comment:
Nice catch, copy-paste error.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2465809003
@Apache9 Check it out.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2537275392
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 33s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 28s | | master passed |
| +1 :green_heart: | compile | 3m 19s | | master passed |
| +1 :green_heart: | checkstyle | 0m 41s | | master passed |
| +1 :green_heart: | spotbugs | 1m 40s | | master passed |
| +1 :green_heart: | spotless | 0m 46s | | branch has no errors when running spotless:check. |
| -0 :warning: | patch | 0m 53s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 25s | | the patch passed |
| +1 :green_heart: | compile | 4m 3s | | the patch passed |
| +1 :green_heart: | javac | 4m 3s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 39s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 57s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 47s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 46s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | | The patch does not generate ASF License warnings. |
| | | 41m 47s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 4727bebff9f0 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 634047b5a9d7fc4b2a89388f6bd67a2217cb6f91 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/6/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2540151094
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 44s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 57s | | master passed |
| +1 :green_heart: | compile | 2m 51s | | master passed |
| +1 :green_heart: | checkstyle | 0m 36s | | master passed |
| +1 :green_heart: | spotbugs | 1m 34s | | master passed |
| +1 :green_heart: | spotless | 0m 43s | | branch has no errors when running spotless:check. |
| -0 :warning: | patch | 0m 50s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 52s | | the patch passed |
| +1 :green_heart: | compile | 2m 48s | | the patch passed |
| +1 :green_heart: | javac | 2m 48s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 34s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 48s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 10m 43s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 55s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | | The patch does not generate ASF License warnings. |
| | | 35m 54s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux d82ac380988e 5.4.0-200-generic #220-Ubuntu SMP Fri Sep 27 13:19:16 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / fd7d07ea15a078a4f45a801cff1f3dbe4fb48c23 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/7/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2550064151
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 41s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 46s | | master passed |
| +1 :green_heart: | compile | 2m 39s | | master passed |
| +1 :green_heart: | checkstyle | 0m 35s | | master passed |
| +1 :green_heart: | spotbugs | 1m 26s | | master passed |
| +1 :green_heart: | spotless | 0m 42s | | branch has no errors when running spotless:check. |
| -0 :warning: | patch | 0m 49s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 38s | | the patch passed |
| +1 :green_heart: | compile | 2m 37s | | the patch passed |
| +1 :green_heart: | javac | 2m 37s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 33s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 31s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 9m 35s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 40s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | | The patch does not generate ASF License warnings. |
| | | 32m 40s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 5c69561358aa 5.4.0-200-generic #220-Ubuntu SMP Fri Sep 27 13:19:16 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dd5f2042a892d10b5c4e4f98f2d90ba1de5a5483 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/8/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2462731951
> Maybe we could introduce something like a RpcObserver? So we do not need to do the instance of MasterObserver and RegionServerObserver.
Makes sense to me. Let me give it a try.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2537854100
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 47s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 2s | | master passed |
| +1 :green_heart: | compile | 1m 16s | | master passed |
| +1 :green_heart: | javadoc | 0m 40s | | master passed |
| +1 :green_heart: | shadedjars | 7m 21s | | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | patch | 7m 29s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 23s | | the patch passed |
| +1 :green_heart: | compile | 1m 9s | | the patch passed |
| +1 :green_heart: | javac | 1m 9s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 32s | | the patch passed |
| +1 :green_heart: | shadedjars | 7m 5s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 467m 49s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/6/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 498m 58s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux b30439184ab1 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 634047b5a9d7fc4b2a89388f6bd67a2217cb6f91 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/6/testReport/ |
| Max. process+thread count | 5009 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/6/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1846909017
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java:
##########
@@ -366,13 +366,16 @@ public void processOneRpc(ByteBuff buf) throws IOException, InterruptedException
processConnectionHeader(buf);
callCleanupIfNeeded();
this.connectionHeaderRead = true;
+ this.rpcServer.getRpcCoprocessorHost().preAuthorizeConnection();
if (rpcServer.needAuthorization() && !authorizeConnection()) {
// Throw FatalConnectionException wrapping ACE so client does right thing and closes
// down the connection instead of trying to read non-existent retun.
throw new AccessDeniedException("Connection from " + this + " for service "
+ connectionHeader.getServiceName() + " is unauthorized for user: " + ugi);
}
this.user = this.rpcServer.userProvider.create(this.ugi);
+ this.rpcServer.getRpcCoprocessorHost().postAuthorizeConnection(
+ this.user != null ? this.user.getName() : null, this.clientCertificateChain);
Review Comment:
That makes sense too to me. I'll make this change too.
But:
Isn't that a problem to introduce too many dependencies required to pull in by coprocessors?
`User` is in `hbase-common`, `ConnectionHeader` is in `hbase-protocol-shaded`, the implementor has to reference all these to compile the co-proc, but since they're generic hbase projects, it might not be a problem at all. I'm not sure.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2486670638
@Apache9 what do you think of the current state? see anything outstanding?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2465994784
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 41s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 14s | | master passed |
| +1 :green_heart: | compile | 1m 9s | | master passed |
| +1 :green_heart: | javadoc | 0m 36s | | master passed |
| +1 :green_heart: | shadedjars | 5m 36s | | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | patch | 5m 46s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 55s | | the patch passed |
| +1 :green_heart: | compile | 1m 1s | | the patch passed |
| +1 :green_heart: | javac | 1m 1s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 29s | | the patch passed |
| +1 :green_heart: | shadedjars | 5m 20s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 227m 58s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/2/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 253m 55s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 656c573c8994 5.4.0-195-generic #215-Ubuntu SMP Fri Aug 2 18:28:05 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 81b70ae00ef1da331169fb0376edb28f9a184c79 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/2/testReport/ |
| Max. process+thread count | 4474 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/2/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2465859787
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 39s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 7s | | master passed |
| +1 :green_heart: | compile | 2m 56s | | master passed |
| +1 :green_heart: | checkstyle | 0m 38s | | master passed |
| +1 :green_heart: | spotbugs | 1m 34s | | master passed |
| +1 :green_heart: | spotless | 0m 44s | | branch has no errors when running spotless:check. |
| -0 :warning: | patch | 0m 52s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 54s | | the patch passed |
| +1 :green_heart: | compile | 3m 4s | | the patch passed |
| +1 :green_heart: | javac | 3m 4s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 37s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 39s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 10m 16s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 44s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | | The patch does not generate ASF License warnings. |
| | | 35m 30s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 35923e3f5070 5.4.0-195-generic #215-Ubuntu SMP Fri Aug 2 18:28:05 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 81b70ae00ef1da331169fb0376edb28f9a184c79 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/2/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2471755950
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 58s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 35s | | master passed |
| +1 :green_heart: | compile | 3m 31s | | master passed |
| +1 :green_heart: | checkstyle | 0m 40s | | master passed |
| +1 :green_heart: | spotbugs | 1m 58s | | master passed |
| +1 :green_heart: | spotless | 1m 6s | | branch has no errors when running spotless:check. |
| -0 :warning: | patch | 1m 15s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 6s | | the patch passed |
| +1 :green_heart: | compile | 3m 14s | | the patch passed |
| +1 :green_heart: | javac | 3m 14s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 47s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 53s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 11m 26s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| -1 :x: | spotless | 0m 39s | | patch has 23 errors when running spotless:check, run spotless:apply to fix. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | | The patch does not generate ASF License warnings. |
| | | 41m 59s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 5bc60de8af5e 5.4.0-195-generic #215-Ubuntu SMP Fri Aug 2 18:28:05 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 056fb4c7a50605fd3f3adff14529d0601eee1a4e |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/3/artifact/yetus-general-check/output/patch-spotless.txt |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/3/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2461249721
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 54s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 47s | | master passed |
| +1 :green_heart: | compile | 1m 12s | | master passed |
| +1 :green_heart: | javadoc | 0m 36s | | master passed |
| +1 :green_heart: | shadedjars | 6m 16s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 38s | | the patch passed |
| +1 :green_heart: | compile | 1m 7s | | the patch passed |
| +1 :green_heart: | javac | 1m 7s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 31s | | the patch passed |
| +1 :green_heart: | shadedjars | 5m 47s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 285m 37s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/1/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 314m 4s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 1fcd963bf9f9 5.4.0-195-generic #215-Ubuntu SMP Fri Aug 2 18:28:05 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 287deec24c67332069cd2b34e9cdc778356ec3f6 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/1/testReport/ |
| Max. process+thread count | 4756 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/1/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1843763802
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java:
##########
@@ -366,13 +366,16 @@ public void processOneRpc(ByteBuff buf) throws IOException, InterruptedException
processConnectionHeader(buf);
callCleanupIfNeeded();
this.connectionHeaderRead = true;
+ this.rpcServer.getRpcCoprocessorHost().preAuthorizeConnection();
Review Comment:
i wonder if there's anything else we can send in here? We have the RequestHeader at this point, and maybe the IP address?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2554880884
Ping @apurtell
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2484067524
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 45s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 0s | | master passed |
| +1 :green_heart: | compile | 3m 3s | | master passed |
| +1 :green_heart: | checkstyle | 0m 38s | | master passed |
| +1 :green_heart: | spotbugs | 1m 32s | | master passed |
| +1 :green_heart: | spotless | 0m 45s | | branch has no errors when running spotless:check. |
| -0 :warning: | patch | 0m 52s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 55s | | the patch passed |
| +1 :green_heart: | compile | 2m 57s | | the patch passed |
| +1 :green_heart: | javac | 2m 57s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 38s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 38s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 10m 28s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 44s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | | The patch does not generate ASF License warnings. |
| | | 35m 44s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 981b077f58eb 5.4.0-195-generic #215-Ubuntu SMP Fri Aug 2 18:28:05 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 5d4f959dff09e4d2aedec8208db14bf30cdd5026 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 83 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/5/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1835317777
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RpcObserver.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.coprocessor;
+
+import java.io.IOException;
+import java.security.cert.X509Certificate;
+import org.apache.hadoop.hbase.HBaseInterfaceAudience;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.yetus.audience.InterfaceStability;
+
+/**
+ * Coprocessors implement this interface to observe and mediate RPC events in Master and RS
+ * instances.
+ * <p>
+ * Since most implementations will be interested in only a subset of hooks, this class uses
+ * 'default' functions to avoid having to add unnecessary overrides. When the functions are
+ * non-empty, it's simply to satisfy the compiler by returning value of expected (non-void) type. It
+ * is done in a way that these default definitions act as no-op. So our suggestion to implementation
+ * would be to not call these 'default' methods from overrides.
+ * <p>
+ * <h3>Exception Handling</h3><br>
+ * For all functions, exception handling is done as follows:
+ * <ul>
+ * <li>Exceptions of type {@link IOException} are reported back to client.</li>
+ * <li>For any other kind of exception:
+ * <ul>
+ * <li>Be aware that this coprocessor doesn't support abortion. If the configuration
+ * {@link CoprocessorHost#ABORT_ON_ERROR_KEY} is set to true, the event will be logged, but the RPC
+ * server won't be aborted.</li>
+ * <li>Otherwise, coprocessor is removed from the server.</li>
+ * </ul>
+ * </li>
+ * </ul>
+ */
+@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.COPROC)
+@InterfaceStability.Evolving
+public interface RpcObserver {
+ /**
+ * Called after successfully authorizing connection
+ * @param ctx the coprocessor instance's environment
+ * @param userName the user name
+ * @param clientCertificateChain list of peer certificates from SSL connection
+ */
+ default void postAuthorizeConnection(ObserverContext<RpcCoprocessorEnvironment> ctx,
Review Comment:
Only post, no pre?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java:
##########
@@ -324,6 +330,13 @@ public void onConfigurationChange(Configuration newConf) {
refreshAuthManager(newConf, new HBasePolicyProvider());
}
refreshSlowLogConfiguration(newConf);
+ if (
+ CoprocessorConfigurationUtil.checkConfigurationChange(getConf(), newConf,
+ CoprocessorHost.RPC_COPROCESSOR_CONF_KEY)
+ ) {
+ LOG.info("Update the RPC coprocessor(s) because the configuration has changed");
+ initializeCoprocessorHost(newConf);
Review Comment:
Is a Configuration object enough for your usage? I see the RpcCoprossesorEnvironment is also empty?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java:
##########
@@ -898,4 +911,14 @@ protected boolean needAuthorization() {
public List<BlockingServiceAndInterface> getServices() {
return services;
}
+
+ private void initializeCoprocessorHost(Configuration conf) {
+ // initialize master side coprocessors before we start handling requests
Review Comment:
Not only for master?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1838830331
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RpcObserver.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.coprocessor;
+
+import java.io.IOException;
+import java.security.cert.X509Certificate;
+import org.apache.hadoop.hbase.HBaseInterfaceAudience;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.yetus.audience.InterfaceStability;
+
+/**
+ * Coprocessors implement this interface to observe and mediate RPC events in Master and RS
+ * instances.
+ * <p>
+ * Since most implementations will be interested in only a subset of hooks, this class uses
+ * 'default' functions to avoid having to add unnecessary overrides. When the functions are
+ * non-empty, it's simply to satisfy the compiler by returning value of expected (non-void) type. It
+ * is done in a way that these default definitions act as no-op. So our suggestion to implementation
+ * would be to not call these 'default' methods from overrides.
+ * <p>
+ * <h3>Exception Handling</h3><br>
+ * For all functions, exception handling is done as follows:
+ * <ul>
+ * <li>Exceptions of type {@link IOException} are reported back to client.</li>
+ * <li>For any other kind of exception:
+ * <ul>
+ * <li>Be aware that this coprocessor doesn't support abortion. If the configuration
+ * {@link CoprocessorHost#ABORT_ON_ERROR_KEY} is set to true, the event will be logged, but the RPC
+ * server won't be aborted.</li>
+ * <li>Otherwise, coprocessor is removed from the server.</li>
+ * </ul>
+ * </li>
+ * </ul>
+ */
+@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.COPROC)
+@InterfaceStability.Evolving
+public interface RpcObserver {
+ /**
+ * Called after successfully authorizing connection
+ * @param ctx the coprocessor instance's environment
+ * @param userName the user name
+ * @param clientCertificateChain list of peer certificates from SSL connection
+ */
+ default void postAuthorizeConnection(ObserverContext<RpcCoprocessorEnvironment> ctx,
Review Comment:
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1843764405
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java:
##########
@@ -366,13 +366,16 @@ public void processOneRpc(ByteBuff buf) throws IOException, InterruptedException
processConnectionHeader(buf);
callCleanupIfNeeded();
this.connectionHeaderRead = true;
+ this.rpcServer.getRpcCoprocessorHost().preAuthorizeConnection();
if (rpcServer.needAuthorization() && !authorizeConnection()) {
// Throw FatalConnectionException wrapping ACE so client does right thing and closes
// down the connection instead of trying to read non-existent retun.
throw new AccessDeniedException("Connection from " + this + " for service "
+ connectionHeader.getServiceName() + " is unauthorized for user: " + ugi);
}
this.user = this.rpcServer.userProvider.create(this.ugi);
+ this.rpcServer.getRpcCoprocessorHost().postAuthorizeConnection(
+ this.user != null ? this.user.getName() : null, this.clientCertificateChain);
Review Comment:
should we send the user in directly and let the implementer decide if they want the name or something else?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2590964458
Another - final - nudge to @apurtell and @Apache9 before submitting 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1846909017
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java:
##########
@@ -366,13 +366,16 @@ public void processOneRpc(ByteBuff buf) throws IOException, InterruptedException
processConnectionHeader(buf);
callCleanupIfNeeded();
this.connectionHeaderRead = true;
+ this.rpcServer.getRpcCoprocessorHost().preAuthorizeConnection();
if (rpcServer.needAuthorization() && !authorizeConnection()) {
// Throw FatalConnectionException wrapping ACE so client does right thing and closes
// down the connection instead of trying to read non-existent retun.
throw new AccessDeniedException("Connection from " + this + " for service "
+ connectionHeader.getServiceName() + " is unauthorized for user: " + ugi);
}
this.user = this.rpcServer.userProvider.create(this.ugi);
+ this.rpcServer.getRpcCoprocessorHost().postAuthorizeConnection(
+ this.user != null ? this.user.getName() : null, this.clientCertificateChain);
Review Comment:
That makes sense too to me. I'll make this change too.
But:
Isn't that a problem to introduce too many dependencies required to pull in by coprocessors?
`User` is in `hbase-common`, `ConnectionHeader` is in `hbase-protocol-shaded`, the implementor has to reference all these when compiling the co-proc, but since they're generic hbase projects, it might not be a problem at all. I'm not sure.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2474890575
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 46s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 56s | | master passed |
| +1 :green_heart: | compile | 3m 53s | | master passed |
| +1 :green_heart: | checkstyle | 0m 48s | | master passed |
| +1 :green_heart: | spotbugs | 1m 54s | | master passed |
| +1 :green_heart: | spotless | 0m 53s | | branch has no errors when running spotless:check. |
| -0 :warning: | patch | 1m 0s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 55s | | the patch passed |
| +1 :green_heart: | compile | 3m 42s | | the patch passed |
| +1 :green_heart: | javac | 3m 42s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 45s | | the patch passed |
| +1 :green_heart: | spotbugs | 2m 5s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 11m 55s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 52s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | | The patch does not generate ASF License warnings. |
| | | 43m 27s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux a79be0503378 5.4.0-195-generic #215-Ubuntu SMP Fri Aug 2 18:28:05 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d02e12550fd7416358c51d9be9559adfc0445f68 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/4/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2472186358
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 27s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 2s | | master passed |
| +1 :green_heart: | compile | 0m 54s | | master passed |
| +1 :green_heart: | javadoc | 0m 27s | | master passed |
| +1 :green_heart: | shadedjars | 5m 42s | | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | patch | 5m 50s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 54s | | the patch passed |
| +1 :green_heart: | compile | 0m 58s | | the patch passed |
| +1 :green_heart: | javac | 0m 58s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 26s | | the patch passed |
| +1 :green_heart: | shadedjars | 5m 39s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 214m 12s | | hbase-server in the patch passed. |
| | | 239m 51s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/3/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux e33e24bbb222 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 056fb4c7a50605fd3f3adff14529d0601eee1a4e |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/3/testReport/ |
| Max. process+thread count | 5360 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/3/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1837073713
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java:
##########
@@ -324,6 +330,13 @@ public void onConfigurationChange(Configuration newConf) {
refreshAuthManager(newConf, new HBasePolicyProvider());
}
refreshSlowLogConfiguration(newConf);
+ if (
+ CoprocessorConfigurationUtil.checkConfigurationChange(getConf(), newConf,
+ CoprocessorHost.RPC_COPROCESSOR_CONF_KEY)
+ ) {
+ LOG.info("Update the RPC coprocessor(s) because the configuration has changed");
+ initializeCoprocessorHost(newConf);
Review Comment:
Strictly for this particular hook we don't anything else. Configuration object is needed to instantiate Observers, the rest is passed in method arguments.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java:
##########
@@ -324,6 +330,13 @@ public void onConfigurationChange(Configuration newConf) {
refreshAuthManager(newConf, new HBasePolicyProvider());
}
refreshSlowLogConfiguration(newConf);
+ if (
+ CoprocessorConfigurationUtil.checkConfigurationChange(getConf(), newConf,
+ CoprocessorHost.RPC_COPROCESSOR_CONF_KEY)
+ ) {
+ LOG.info("Update the RPC coprocessor(s) because the configuration has changed");
+ initializeCoprocessorHost(newConf);
Review Comment:
Strictly for this particular hook we don't need anything else. Configuration object is needed to instantiate Observers, the rest is passed in method arguments.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2461401599
Maybe we could introduce something like a RpcObserver? So we do not need to do the instance of MasterObserver and RegionServerObserver.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2463586986
I guess we need to introduce all these things.
You can have a try first, if we think introducing so many new classes does not make sense, we could discuss again.
Thanks.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2484485422
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 42s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 8s | | master passed |
| +1 :green_heart: | compile | 1m 17s | | master passed |
| +1 :green_heart: | javadoc | 0m 38s | | master passed |
| +1 :green_heart: | shadedjars | 7m 44s | | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | patch | 7m 54s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 49s | | the patch passed |
| +1 :green_heart: | compile | 1m 28s | | the patch passed |
| +1 :green_heart: | javac | 1m 28s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed |
| +1 :green_heart: | shadedjars | 7m 12s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 267m 14s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/5/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 300m 0s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/5/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux d30bd7b7b715 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 5d4f959dff09e4d2aedec8208db14bf30cdd5026 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/5/testReport/ |
| Max. process+thread count | 4409 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/5/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1846912071
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java:
##########
@@ -366,13 +366,16 @@ public void processOneRpc(ByteBuff buf) throws IOException, InterruptedException
processConnectionHeader(buf);
callCleanupIfNeeded();
this.connectionHeaderRead = true;
+ this.rpcServer.getRpcCoprocessorHost().preAuthorizeConnection();
if (rpcServer.needAuthorization() && !authorizeConnection()) {
// Throw FatalConnectionException wrapping ACE so client does right thing and closes
// down the connection instead of trying to read non-existent retun.
throw new AccessDeniedException("Connection from " + this + " for service "
+ connectionHeader.getServiceName() + " is unauthorized for user: " + ugi);
}
this.user = this.rpcServer.userProvider.create(this.ugi);
+ this.rpcServer.getRpcCoprocessorHost().postAuthorizeConnection(
+ this.user != null ? this.user.getName() : null, this.clientCertificateChain);
Review Comment:
In `MasterObserver` only the username is passed as `String` to coprocessor.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1846903419
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java:
##########
@@ -366,13 +366,16 @@ public void processOneRpc(ByteBuff buf) throws IOException, InterruptedException
processConnectionHeader(buf);
callCleanupIfNeeded();
this.connectionHeaderRead = true;
+ this.rpcServer.getRpcCoprocessorHost().preAuthorizeConnection();
Review Comment:
Good point, thanks for the idea.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2540343754
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 43s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 49s | | master passed |
| +1 :green_heart: | compile | 0m 52s | | master passed |
| +1 :green_heart: | javadoc | 0m 29s | | master passed |
| +1 :green_heart: | shadedjars | 4m 55s | | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | patch | 5m 4s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 42s | | the patch passed |
| +1 :green_heart: | compile | 0m 53s | | the patch passed |
| +1 :green_heart: | javac | 0m 53s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed |
| +1 :green_heart: | shadedjars | 4m 55s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 190m 1s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/7/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 212m 49s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/7/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux d8e39acf80b7 5.4.0-200-generic #220-Ubuntu SMP Fri Sep 27 13:19:16 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / fd7d07ea15a078a4f45a801cff1f3dbe4fb48c23 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/7/testReport/ |
| Max. process+thread count | 5544 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/7/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2460968935
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 57s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 43s | | master passed |
| +1 :green_heart: | compile | 3m 48s | | master passed |
| +1 :green_heart: | checkstyle | 0m 46s | | master passed |
| +1 :green_heart: | spotbugs | 2m 5s | | master passed |
| +1 :green_heart: | spotless | 0m 54s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 1s | | the patch passed |
| +1 :green_heart: | compile | 3m 24s | | the patch passed |
| +1 :green_heart: | javac | 3m 24s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 48s | | the patch passed |
| +1 :green_heart: | spotbugs | 2m 5s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 30s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 54s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | | The patch does not generate ASF License warnings. |
| | | 42m 57s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 4adab3a06626 5.4.0-195-generic #215-Ubuntu SMP Fri Aug 2 18:28:05 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 287deec24c67332069cd2b34e9cdc778356ec3f6 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/1/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2463385043
@Apache9 Does that need the entire new co-proc infra with: **RpcCoprocessor, RpcCoprocessorEnvironment, RpcObserver, RpcCoprocessorHost**, etc.?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2475156177
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 39s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 8s | | master passed |
| +1 :green_heart: | compile | 0m 58s | | master passed |
| +1 :green_heart: | javadoc | 0m 31s | | master passed |
| +1 :green_heart: | shadedjars | 5m 26s | | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | patch | 5m 36s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 1s | | the patch passed |
| +1 :green_heart: | compile | 1m 1s | | the patch passed |
| +1 :green_heart: | javac | 1m 1s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 31s | | the patch passed |
| +1 :green_heart: | shadedjars | 5m 24s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 231m 44s | | hbase-server in the patch passed. |
| | | 256m 58s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/6447 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux ae1ad96df64b 5.4.0-200-generic #220-Ubuntu SMP Fri Sep 27 13:19:16 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d02e12550fd7416358c51d9be9559adfc0445f68 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/4/testReport/ |
| Max. process+thread count | 5198 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6447/4/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on PR #6447:
URL: https://github.com/apache/hbase/pull/6447#issuecomment-2551628203
Thanks @bbeaudreault . Build is now green. I'm waiting for your approval @Apache9 .
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #6447:
URL: https://github.com/apache/hbase/pull/6447#discussion_r1846912071
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java:
##########
@@ -366,13 +366,16 @@ public void processOneRpc(ByteBuff buf) throws IOException, InterruptedException
processConnectionHeader(buf);
callCleanupIfNeeded();
this.connectionHeaderRead = true;
+ this.rpcServer.getRpcCoprocessorHost().preAuthorizeConnection();
if (rpcServer.needAuthorization() && !authorizeConnection()) {
// Throw FatalConnectionException wrapping ACE so client does right thing and closes
// down the connection instead of trying to read non-existent retun.
throw new AccessDeniedException("Connection from " + this + " for service "
+ connectionHeader.getServiceName() + " is unauthorized for user: " + ugi);
}
this.user = this.rpcServer.userProvider.create(this.ugi);
+ this.rpcServer.getRpcCoprocessorHost().postAuthorizeConnection(
+ this.user != null ? this.user.getName() : null, this.clientCertificateChain);
Review Comment:
In `MasterObserver` only to username is passed as `String` to coprocessor.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain [hbase]
Posted by "stoty (via GitHub)" <gi...@apache.org>.
stoty merged PR #6447:
URL: https://github.com/apache/hbase/pull/6447
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org