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/04/30 22:55:45 UTC
[PR] HBASE-27118 Add security headers to Thrift/HTTP server [hbase]
anmolnar opened a new pull request, #5864:
URL: https://github.com/apache/hbase/pull/5864
Line up Thrift server with HTTP and REST by adding HTTP security headers if SSL is enabled.
Includes a new unit test class specific for HTTP+SSL based Thrift server + some refactoring.
--
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-27118 Add security headers to Thrift/HTTP server [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5864:
URL: https://github.com/apache/hbase/pull/5864#issuecomment-2087724371
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 1s | master passed |
| +1 :green_heart: | compile | 1m 2s | master passed |
| +1 :green_heart: | shadedjars | 5m 16s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 50s | the patch passed |
| +1 :green_heart: | compile | 1m 1s | the patch passed |
| +1 :green_heart: | javac | 1m 1s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 17s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 1s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 59s | hbase-http in the patch passed. |
| +1 :green_heart: | unit | 6m 44s | hbase-thrift in the patch passed. |
| +1 :green_heart: | unit | 3m 31s | hbase-rest in the patch passed. |
| | | 34m 10s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5864 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c47bae970cfa 5.4.0-174-generic #193-Ubuntu SMP Thu Mar 7 14:29:28 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 8a2f3ef793 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/testReport/ |
| Max. process+thread count | 1678 (vs. ulimit of 30000) |
| modules | C: hbase-http hbase-thrift hbase-rest U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
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-27118 Add security headers to Thrift/HTTP server [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #5864:
URL: https://github.com/apache/hbase/pull/5864#discussion_r1587596463
##########
hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServer.java:
##########
@@ -387,9 +388,12 @@ protected void setupHTTPServer() throws IOException {
httpServer = new Server(threadPool);
// Context handler
+ boolean isSecure = conf.getBoolean(THRIFT_SSL_ENABLED_KEY, false);
ServletContextHandler ctxHandler =
new ServletContextHandler(httpServer, "/", ServletContextHandler.SESSIONS);
- ctxHandler.addServlet(new ServletHolder(thriftHttpServlet), "/*");
+ HttpServerUtil.addClickjackingPreventionFilter(ctxHandler, conf, PATH_SPEC_ANY);
Review Comment:
Thanks, I've updated the ticket.
--
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-27118 Add security headers to Thrift/HTTP server [hbase]
Posted by "stoty (via GitHub)" <gi...@apache.org>.
stoty commented on code in PR #5864:
URL: https://github.com/apache/hbase/pull/5864#discussion_r1587032854
##########
hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServer.java:
##########
@@ -387,9 +388,12 @@ protected void setupHTTPServer() throws IOException {
httpServer = new Server(threadPool);
// Context handler
+ boolean isSecure = conf.getBoolean(THRIFT_SSL_ENABLED_KEY, false);
ServletContextHandler ctxHandler =
new ServletContextHandler(httpServer, "/", ServletContextHandler.SESSIONS);
- ctxHandler.addServlet(new ServletHolder(thriftHttpServlet), "/*");
+ HttpServerUtil.addClickjackingPreventionFilter(ctxHandler, conf, PATH_SPEC_ANY);
Review Comment:
nit:
You mention in the ticket description that we add this when "HTTP support is enabled".
While I understand that this refers to the HTTP frontend as opposed to the binary, it can still cause misundestandings.
I suggest updating the ticket text to "HTTP/HTTPS support is enabled"
--
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-27118 Add security headers to Thrift/HTTP server [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5864:
URL: https://github.com/apache/hbase/pull/5864#issuecomment-2087723689
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | 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 _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 56s | master passed |
| +1 :green_heart: | compile | 0m 54s | master passed |
| +1 :green_heart: | shadedjars | 5m 27s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 53s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 49s | the patch passed |
| +1 :green_heart: | compile | 0m 58s | the patch passed |
| +1 :green_heart: | javac | 0m 58s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 26s | 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 | 0m 49s | hbase-http in the patch passed. |
| +1 :green_heart: | unit | 6m 28s | hbase-thrift in the patch passed. |
| +1 :green_heart: | unit | 3m 27s | hbase-rest in the patch passed. |
| | | 33m 11s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5864 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 928347fcbaf6 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 / 8a2f3ef793 |
| Default Java | Eclipse Adoptium-17.0.10+7 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/testReport/ |
| Max. process+thread count | 1640 (vs. ulimit of 30000) |
| modules | C: hbase-http hbase-thrift hbase-rest U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
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-27118 Add security headers to Thrift/HTTP server [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5864:
URL: https://github.com/apache/hbase/pull/5864#issuecomment-2087727999
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 51s | 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 _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 31s | master passed |
| +1 :green_heart: | compile | 1m 57s | master passed |
| +1 :green_heart: | checkstyle | 0m 59s | master passed |
| +1 :green_heart: | spotless | 0m 52s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 2m 15s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 59s | the patch passed |
| +1 :green_heart: | compile | 1m 55s | the patch passed |
| +1 :green_heart: | javac | 1m 55s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 6s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 2s | The patch has no ill-formed XML file. |
| +1 :green_heart: | hadoopcheck | 6m 16s | Patch does not cause any errors with Hadoop 3.3.6. |
| +1 :green_heart: | spotless | 0m 51s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 1s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 44s | The patch does not generate ASF License warnings. |
| | | 38m 37s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5864 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile xml |
| uname | Linux 14a06661c5cd 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 / 8a2f3ef793 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 81 (vs. ulimit of 30000) |
| modules | C: hbase-http hbase-thrift hbase-rest U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
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-27118 Add security headers to Thrift/HTTP server [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5864:
URL: https://github.com/apache/hbase/pull/5864#issuecomment-2087729410
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 56s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 57s | master passed |
| +1 :green_heart: | compile | 1m 8s | master passed |
| +1 :green_heart: | shadedjars | 6m 54s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 15s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 19s | 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 18s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 53s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 8s | hbase-http in the patch passed. |
| +1 :green_heart: | unit | 6m 45s | hbase-thrift in the patch passed. |
| +1 :green_heart: | unit | 4m 56s | hbase-rest in the patch passed. |
| | | 40m 38s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5864 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 5989d0f5beb1 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 / 8a2f3ef793 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/testReport/ |
| Max. process+thread count | 1638 (vs. ulimit of 30000) |
| modules | C: hbase-http hbase-thrift hbase-rest U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5864/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org