Re: [PR] HBASE-27118 Add security headers to Thrift/HTTP server [hbase]
Apache9 commented on PR #5864: URL: https://github.com/apache/hbase/pull/5864#issuecomment-2152453974 @anmolnar You can merge this by yourself since you are a committer now. -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-27118 Add security headers to Thrift/HTTP server [hbase]
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-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] HBASE-27118 Add security headers to Thrift/HTTP server [hbase]
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-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] HBASE-27118 Add security headers to Thrift/HTTP server [hbase]
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 3) | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-27118 Add security headers to Thrift/HTTP server [hbase]
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 3) | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-27118 Add security headers to Thrift/HTTP server [hbase]
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 3) | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-27118 Add security headers to Thrift/HTTP server [hbase]
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 3) | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
