You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "sarutak (via GitHub)" <gi...@apache.org> on 2026/06/22 15:32:37 UTC
[PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
sarutak opened a new pull request, #56669:
URL: https://github.com/apache/spark/pull/56669
### What changes were proposed in this pull request?
This PR fixes the `spark.ui.allowFramingFrom` configuration to use CSP `frame-ancestors` directive instead of the deprecated `X-Frame-Options: ALLOW-FROM`.
According to [MDN Web Docs](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-Frame-Options#allow-from_origin):
> **`ALLOW-FROM origin`** — This is an obsolete directive. Modern browsers that encounter response headers with this directive will ignore the header completely. The `Content-Security-Policy` HTTP header has a `frame-ancestors` directive which you should use instead.
As a result, users who set `spark.ui.allowFramingFrom` to allow framing from a specific origin find that **the setting has no effect** in any current browser.
The fix:
- Uses CSP `frame-ancestors 'self' <uri>` as the primary clickjacking protection mechanism
- Always sets `X-Frame-Options: SAMEORIGIN` as a fallback for legacy clients
- When `spark.ui.contentSecurityPolicy.enabled=false` (the default), emits a minimal CSP header containing only `frame-ancestors` so that the `allowFramingFrom` setting works regardless of the full CSP toggle
- Sanitizes the config value to prevent CSP directive injection via semicolons or newlines
- Filters out legacy `X-Frame-Options` values (`SAMEORIGIN`, `DENY`) if explicitly set in `spark.ui.allowFramingFrom` for backward compatibility
- Moves `X-Frame-Options` header before access control checks so that even 403 error responses include clickjacking protection
### Why are the changes needed?
The `spark.ui.allowFramingFrom` config has been non-functional in all modern browsers since `ALLOW-FROM` was obsoleted. Users relying on this setting for embedding Spark UI in iframes from specific origins have no working clickjacking protection. CSP `frame-ancestors` is the W3C-recommended replacement.
### Does this PR introduce _any_ user-facing change?
Yes.
- `spark.ui.allowFramingFrom` now works correctly in modern browsers via CSP `frame-ancestors`.
- `X-Frame-Options` header is always `SAMEORIGIN` (previously it was `ALLOW-FROM <uri>` which browsers ignored).
- A `Content-Security-Policy` header with `frame-ancestors` is now always emitted, even when `spark.ui.contentSecurityPolicy.enabled=false`.
### How was this patch tested?
- Updated unit tests in `HttpSecurityFilterSuite` to verify the CSP `frame-ancestors` directive contains the configured URI
- Updated unit tests to verify `X-Frame-Options` is always `SAMEORIGIN`
- Updated integration test in `UISuite` to verify Jetty sanitizes newlines in the CSP header
- Added test for CSP directive injection prevention via semicolons
- Added test verifying `X-Frame-Options` is set even when access is denied (403 responses)
- Added test coverage for CSP-disabled mode (verifies minimal `frame-ancestors` header is still emitted)
### Was this patch authored or co-authored using generative AI tooling?
Kiro CLI / Claude
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4801045268
Exactly! That's the reason why this PR description proposes `Always sets X-Frame-Options: SAMEORIGIN as a fallback for legacy clients`, right? IIUC, you proposed that `spark.ui.allowFramingFrom` works with the fallback legacy behavior without `contentSecurityPolicy`.
> However, spark.ui.allowFramingFrom is not a sub-config of contentSecurityPolicy. It's an independent config that has existed since Spark 1.6.0, long before CSP support was added. If we enforce the parent-child rule strictly here, allowFramingFrom becomes non-functional unless users explicitly enable CSP, which changes the originally intended default behavior of this config.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #56669:
URL: https://github.com/apache/spark/pull/56669#discussion_r3453596803
##########
core/src/main/scala/org/apache/spark/ui/HttpSecurityFilter.scala:
##########
@@ -51,13 +51,36 @@ private class HttpSecurityFilter(
val cspNonce = CspNonce.generate()
try {
+ // SPARK-10589 avoid frame-related click-jacking vulnerability.
+ // Use CSP frame-ancestors as the primary mechanism (supported by all modern browsers),
+ // with X-Frame-Options: SAMEORIGIN as a fallback for legacy clients.
+ // Note: X-Frame-Options ALLOW-FROM is deprecated and ignored by modern browsers
+ // (Chrome, Firefox, Edge, Safari), so frame-ancestors is used instead.
+ val frameAncestors = conf.get(UI_ALLOW_FRAMING_FROM)
+ .filterNot(v => v.equalsIgnoreCase("SAMEORIGIN") || v.equalsIgnoreCase("DENY"))
+ .map { uri =>
+ // Sanitize the URI: truncate at semicolons to prevent CSP directive injection,
+ // and strip newlines to prevent header injection.
+ val sanitized = uri.replaceAll("[\\r\\n]+", "").split(";", 2)(0).trim
+ s"frame-ancestors 'self' $sanitized"
+ }
+ .getOrElse("frame-ancestors 'self'")
+
if (conf.get(UI_CONTENT_SECURITY_POLICY_ENABLED)) {
hres.setHeader("Content-Security-Policy",
s"default-src 'self'; script-src 'self' 'nonce-$cspNonce'; " +
s"style-src 'self' 'unsafe-inline'; img-src 'self' data:; " +
- s"object-src 'none'; base-uri 'self';")
+ s"object-src 'none'; base-uri 'self'; $frameAncestors;")
+ } else {
+ // Even when the full CSP is disabled, set frame-ancestors to enforce
+ // the allowFramingFrom setting in browsers that support CSP.
+ hres.setHeader("Content-Security-Policy", s"$frameAncestors;")
Review Comment:
May I ask if there is a way to have this feature without `CSP` header?
Although I agree with your intention, this looks like a conflict to our CSP config option. `CSP` header should not be used when the CSP configuration is off. In other words, this is a sub feature of `CSP` parent configuration.
> Even when the full CSP is disabled, set frame-ancestors to enforce the allowFramingFrom setting in browsers that support CSP.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4915962444
Thank you @dongjoon-hyun for reviewing!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4776185632
> Are you targeting this to deliver to Apache Spark 4.2.0? I saw that you put this under [SPARK-55556 Improve Security](https://issues.apache.org/jira/browse/SPARK-55556) which is resolved for Apache Spark 4.2.0 already.
I was just about to comment on that very thing. I intentionally added this as a sub-task of SPARK-55556. It's already marked as resolved but some other sub-tasks seems added recently.
I thought it's better to target this to 4.2.0 if we can make it in time because this is a security issue.
cc: @huaxingao
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4781567755
I agree with the general principle that when `spark.dynamicAllocation.enabled=false`, sub-configs like `maxExecutors` are ignored. That makes sense because those sub-configs were designed as part of the dynamic allocation feature from the start.
I also understand your intent with the new sub-config: `frameAncestors.enabled` lives under `contentSecurityPolicy.*`, so when the parent (`contentSecurityPolicy.enabled`) is off, no CSP header should be emitted.
However, `spark.ui.allowFramingFrom` is not a sub-config of `contentSecurityPolicy`. It's an independent config that has existed since Spark 1.6.0, long before CSP support was added. If we enforce the parent-child rule strictly here, `allowFramingFrom` becomes non-functional unless users explicitly enable CSP, which changes the originally intended default behavior of this config.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #56669:
URL: https://github.com/apache/spark/pull/56669#discussion_r3461199891
##########
core/src/main/scala/org/apache/spark/ui/HttpSecurityFilter.scala:
##########
@@ -51,13 +51,42 @@ private class HttpSecurityFilter(
val cspNonce = CspNonce.generate()
try {
- if (conf.get(UI_CONTENT_SECURITY_POLICY_ENABLED)) {
- hres.setHeader("Content-Security-Policy",
- s"default-src 'self'; script-src 'self' 'nonce-$cspNonce'; " +
- s"style-src 'self' 'unsafe-inline'; img-src 'self' data:; " +
- s"object-src 'none'; base-uri 'self';")
+ val cspEnabled = conf.get(UI_CONTENT_SECURITY_POLICY_ENABLED)
+ val frameAncestorsEnabled = conf.get(UI_CONTENT_SECURITY_POLICY_FRAME_ANCESTORS_ENABLED)
+
+ if (cspEnabled || frameAncestorsEnabled) {
+ // Use CSP frame-ancestors as the primary clickjacking protection mechanism.
+ // X-Frame-Options ALLOW-FROM is deprecated and ignored by modern browsers
+ // (Chrome, Firefox, Edge, Safari), so frame-ancestors is used instead.
+ val frameAncestors = conf.get(UI_ALLOW_FRAMING_FROM)
+ .filterNot(v => v.equalsIgnoreCase("SAMEORIGIN"))
+ .map { uri =>
+ if (uri.equalsIgnoreCase("DENY")) {
+ "frame-ancestors 'none'"
+ } else {
+ // Sanitize the URI: truncate at semicolons to prevent CSP directive injection,
+ // and strip newlines to prevent header injection.
+ val sanitized = uri.replaceAll("[\\r\\n]+", "").split(";", 2)(0).trim
+ if (sanitized.isEmpty) "frame-ancestors 'self'"
+ else s"frame-ancestors 'self' $sanitized"
+ }
+ }
+ .getOrElse("frame-ancestors 'self'")
+
+ if (cspEnabled) {
+ hres.setHeader("Content-Security-Policy",
+ s"default-src 'self'; script-src 'self' 'nonce-$cspNonce'; " +
+ s"style-src 'self' 'unsafe-inline'; img-src 'self' data:; " +
+ s"object-src 'none'; base-uri 'self'; $frameAncestors;")
+ } else {
+ hres.setHeader("Content-Security-Policy", s"$frameAncestors;")
Review Comment:
Well, I don't think this is correct.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #56669:
URL: https://github.com/apache/spark/pull/56669#discussion_r3453561402
##########
core/src/main/scala/org/apache/spark/ui/HttpSecurityFilter.scala:
##########
@@ -51,13 +51,36 @@ private class HttpSecurityFilter(
val cspNonce = CspNonce.generate()
try {
+ // SPARK-10589 avoid frame-related click-jacking vulnerability.
Review Comment:
I believe we can remove this line.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4781308289
@dongjoon-hyun
When you suggested adding `spark.ui.contentSecurityPolicy.frameAncestors.enabled`, I interpreted "fallback for this transition" as: since the full CSP is too strong right now, this sub-config allows emitting just `frame-ancestors` without the full CSP so that `allowFramingFrom` works during the transition to full CSP.
If your intent is that no CSP header should be emitted at all when `contentSecurityPolicy.enabled=false`, I can make that change. However, I'd like to point out that these two goals are mutually exclusive:
- **Design consistency**: CSP disabled = no CSP header at all
- **`allowFramingFrom` working as intended**: requires `frame-ancestors`, which can only exist in a CSP header
There is no browser mechanism to allow framing from a specific origin without a CSP header. If we choose design consistency, `allowFramingFrom` becomes non-functional unless users explicitly enable CSP. Since `ALLOW-FROM` was already broken in browsers, this doesn't cause a regression in practice, but it does mean the originally intended behavior of `allowFramingFrom` cannot work by default.
Which trade-off do you prefer?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #56669:
URL: https://github.com/apache/spark/pull/56669#discussion_r3456681742
##########
core/src/main/scala/org/apache/spark/ui/HttpSecurityFilter.scala:
##########
@@ -51,13 +51,36 @@ private class HttpSecurityFilter(
val cspNonce = CspNonce.generate()
try {
+ // SPARK-10589 avoid frame-related click-jacking vulnerability.
+ // Use CSP frame-ancestors as the primary mechanism (supported by all modern browsers),
+ // with X-Frame-Options: SAMEORIGIN as a fallback for legacy clients.
+ // Note: X-Frame-Options ALLOW-FROM is deprecated and ignored by modern browsers
+ // (Chrome, Firefox, Edge, Safari), so frame-ancestors is used instead.
+ val frameAncestors = conf.get(UI_ALLOW_FRAMING_FROM)
+ .filterNot(v => v.equalsIgnoreCase("SAMEORIGIN") || v.equalsIgnoreCase("DENY"))
+ .map { uri =>
+ // Sanitize the URI: truncate at semicolons to prevent CSP directive injection,
+ // and strip newlines to prevent header injection.
+ val sanitized = uri.replaceAll("[\\r\\n]+", "").split(";", 2)(0).trim
+ s"frame-ancestors 'self' $sanitized"
+ }
+ .getOrElse("frame-ancestors 'self'")
+
if (conf.get(UI_CONTENT_SECURITY_POLICY_ENABLED)) {
hres.setHeader("Content-Security-Policy",
s"default-src 'self'; script-src 'self' 'nonce-$cspNonce'; " +
s"style-src 'self' 'unsafe-inline'; img-src 'self' data:; " +
- s"object-src 'none'; base-uri 'self';")
+ s"object-src 'none'; base-uri 'self'; $frameAncestors;")
+ } else {
+ // Even when the full CSP is disabled, set frame-ancestors to enforce
+ // the allowFramingFrom setting in browsers that support CSP.
+ hres.setHeader("Content-Security-Policy", s"$frameAncestors;")
Review Comment:
`spark.ui.contentSecurityPolicy.enabled` means (2).
For this new feature, shall we add new sub-config like `spark.ui.contentSecurityPolicy.frameAncestors.enabled' because we need a fallback for this transition.
BTW, while testing 4.2.0, I found that CSP feature is too strong and Spark UIs are still immature. I need to revisit one by one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #56669:
URL: https://github.com/apache/spark/pull/56669#discussion_r3456681742
##########
core/src/main/scala/org/apache/spark/ui/HttpSecurityFilter.scala:
##########
@@ -51,13 +51,36 @@ private class HttpSecurityFilter(
val cspNonce = CspNonce.generate()
try {
+ // SPARK-10589 avoid frame-related click-jacking vulnerability.
+ // Use CSP frame-ancestors as the primary mechanism (supported by all modern browsers),
+ // with X-Frame-Options: SAMEORIGIN as a fallback for legacy clients.
+ // Note: X-Frame-Options ALLOW-FROM is deprecated and ignored by modern browsers
+ // (Chrome, Firefox, Edge, Safari), so frame-ancestors is used instead.
+ val frameAncestors = conf.get(UI_ALLOW_FRAMING_FROM)
+ .filterNot(v => v.equalsIgnoreCase("SAMEORIGIN") || v.equalsIgnoreCase("DENY"))
+ .map { uri =>
+ // Sanitize the URI: truncate at semicolons to prevent CSP directive injection,
+ // and strip newlines to prevent header injection.
+ val sanitized = uri.replaceAll("[\\r\\n]+", "").split(";", 2)(0).trim
+ s"frame-ancestors 'self' $sanitized"
+ }
+ .getOrElse("frame-ancestors 'self'")
+
if (conf.get(UI_CONTENT_SECURITY_POLICY_ENABLED)) {
hres.setHeader("Content-Security-Policy",
s"default-src 'self'; script-src 'self' 'nonce-$cspNonce'; " +
s"style-src 'self' 'unsafe-inline'; img-src 'self' data:; " +
- s"object-src 'none'; base-uri 'self';")
+ s"object-src 'none'; base-uri 'self'; $frameAncestors;")
+ } else {
+ // Even when the full CSP is disabled, set frame-ancestors to enforce
+ // the allowFramingFrom setting in browsers that support CSP.
+ hres.setHeader("Content-Security-Policy", s"$frameAncestors;")
Review Comment:
`spark.ui.contentSecurityPolicy.enabled` means (2).
For this new feature, shall we add new sub-config like `spark.ui.contentSecurityPolicy.frameAncestors.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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4776120542
@sarutak ? Are you targeting this to deliver to Apache Spark 4.2.0? I saw that you put this under [SPARK-55556 Improve Security](https://issues.apache.org/jira/browse/SPARK-55556) which is resolved for Apache Spark 4.2.0 already.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #56669:
URL: https://github.com/apache/spark/pull/56669#discussion_r3453973041
##########
core/src/main/scala/org/apache/spark/ui/HttpSecurityFilter.scala:
##########
@@ -51,13 +51,36 @@ private class HttpSecurityFilter(
val cspNonce = CspNonce.generate()
try {
+ // SPARK-10589 avoid frame-related click-jacking vulnerability.
+ // Use CSP frame-ancestors as the primary mechanism (supported by all modern browsers),
+ // with X-Frame-Options: SAMEORIGIN as a fallback for legacy clients.
+ // Note: X-Frame-Options ALLOW-FROM is deprecated and ignored by modern browsers
+ // (Chrome, Firefox, Edge, Safari), so frame-ancestors is used instead.
+ val frameAncestors = conf.get(UI_ALLOW_FRAMING_FROM)
+ .filterNot(v => v.equalsIgnoreCase("SAMEORIGIN") || v.equalsIgnoreCase("DENY"))
+ .map { uri =>
+ // Sanitize the URI: truncate at semicolons to prevent CSP directive injection,
+ // and strip newlines to prevent header injection.
+ val sanitized = uri.replaceAll("[\\r\\n]+", "").split(";", 2)(0).trim
+ s"frame-ancestors 'self' $sanitized"
+ }
+ .getOrElse("frame-ancestors 'self'")
+
if (conf.get(UI_CONTENT_SECURITY_POLICY_ENABLED)) {
hres.setHeader("Content-Security-Policy",
s"default-src 'self'; script-src 'self' 'nonce-$cspNonce'; " +
s"style-src 'self' 'unsafe-inline'; img-src 'self' data:; " +
- s"object-src 'none'; base-uri 'self';")
+ s"object-src 'none'; base-uri 'self'; $frameAncestors;")
+ } else {
+ // Even when the full CSP is disabled, set frame-ancestors to enforce
+ // the allowFramingFrom setting in browsers that support CSP.
+ hres.setHeader("Content-Security-Policy", s"$frameAncestors;")
Review Comment:
> May I ask if there is a way to have this feature without CSP header?
There is no browser mechanism to allow framing from a specific origin without `Content-Security-Policy: frame-ancestors`. `X-Frame-Options` only supports `DENY` and `SAMEORIGIN` in modern browsers (`ALLOW-FROM` is obsolete and ignored).
So we have two options:
1. **Emit a minimal CSP header (`frame-ancestors` only) when CSP is disabled**
`allowFramingFrom` works regardless of CSP config, but it's inconsistent with the CSP toggle.
2. **No CSP header when CSP is disabled**
Consistent with the CSP config, but `allowFramingFrom` only takes effect when `spark.ui.contentSecurityPolicy.enabled=true`. When CSP is disabled, `X-Frame-Options: SAMEORIGIN` is always used regardless of the `allowFramingFrom` value.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4890506280
Any other feedbacks, @dongjoon-hyun ?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4781114405
Thank you for updating.
I'm also okay for the targeting 4.2.0 as long as Huaxin allows it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4781419256
Before going further, let's clarify this.
Apache Spark configuration namespace is organized in this way, @sarutak . Do you agree with this, @sarutak ?
- `spark.dynamicAllocation.enabled` controls all dynamic allocation features including subfeatures.
- When `spark.dynamicAllocation.enabled=false`, the followings are ignored completely.
- spark.dynamicAllocation.minExecutors
- spark.dynamicAllocation.maxExecutors
- spark.dynamicAllocation.initialExecutors
- spark.dynamicAllocation.executorAllocationRatio
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #56669:
URL: https://github.com/apache/spark/pull/56669#issuecomment-4915955676
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/575dfb2afabbb181e4d51f68451bedcfa57f5191
- merged into branch-4.x https://github.com/apache/spark/commit/9272286dd91813b16af34995ace77b96170271ad
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM` [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak closed pull request #56669: [SPARK-57614][WEBUI] Use CSP `frame-ancestors` instead of deprecated `X-Frame-Options: ALLOW-FROM`
URL: https://github.com/apache/spark/pull/56669
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org