-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-49090][CORE] Support JWSFilter
#47575
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| <groupId>io.jsonwebtoken</groupId> | ||
| <artifactId>jjwt-jackson</artifactId> | ||
| <version>0.12.6</version> | ||
| <scope>test</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this as a test dependency for now because the user may want to use GSON instead of this.
|
Could you review this PR about Spark UI (including Spark Cluster), @viirya ? |
| val claims = Jwts.parser().verifyWith(key).build().parseSignedClaims(token) | ||
| chain.doFilter(req, res) | ||
| case _ => | ||
| hres.sendError(HttpServletResponse.SC_FORBIDDEN, s"Malformed ${AUTHORIZATION} header.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| hres.sendError(HttpServletResponse.SC_FORBIDDEN, s"Malformed ${AUTHORIZATION} header.") | |
| hres.sendError(HttpServletResponse.SC_FORBIDDEN, s"Malformed JWT ${AUTHORIZATION} header.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, but actually, the previous one is better because Bearer is one of type~
Authorization: <type> <credentials> pattern is W3C in HTTP 1.0 spec, instead of a specific to JTW.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, but the current one also doesn't have Bearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and, the missing Bearer is an issue of Authorization header, not a JWT token. Here, JWT token itself doesn't exist yet.
| <dependency> | ||
| <groupId>io.jsonwebtoken</groupId> | ||
| <artifactId>jjwt-api</artifactId> | ||
| <version>0.12.6</version> | ||
| </dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If users don't use the JWSFilter feature, we still need to include this new dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes for now. Of course, we can make this as a profile.
Co-authored-by: Liang-Chi Hsieh <[email protected]>
|
Thank you, @viirya ! |
| jettison/1.5.4//jettison-1.5.4.jar | ||
| jetty-util-ajax/11.0.21//jetty-util-ajax-11.0.21.jar | ||
| jetty-util/11.0.21//jetty-util-11.0.21.jar | ||
| jjwt-api/0.12.6//jjwt-api-0.12.6.jar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update our NOTICE-binary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Sure, @yaooqinn ! It's Apache License. Let me add this item.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the update! @dongjoon-hyun
|
Thank you, @yaooqinn ! |
|
Merged to master for Apache Spark 4.0.0-preview2. |
### What changes were proposed in this pull request? This PR aims to support `spark.master.rest.filters` configuration like the existing `spark.ui.filters` configuration. Recently, Apache Spark starts to support `JWSFilter`. We can take advantage of `JWSFilter` to protect Spark Master REST API. - #47575 ### Why are the changes needed? Like `Spark UI`, we had better provide the same capability to Apache Spark Master REST API . For example, we can protect `JWSFilter` to `Spark Master REST API` like the following. **MASTER REST API WITH JWSFilter** ``` $ build/sbt package $ cp jjwt-impl-0.12.6.jar assembly/target/scala-2.13/jars $ cp jjwt-jackson-0.12.6.jar assembly/target/scala-2.13/jars $ SPARK_NO_DAEMONIZE=1 \ SPARK_MASTER_OPTS="-Dspark.master.rest.enabled=true -Dspark.master.rest.filters=org.apache.spark.ui.JWSFilter -Dspark.org.apache.spark.ui.JWSFilter.param.key=VmlzaXQgaHR0cHM6Ly9zcGFyay5hcGFjaGUub3JnIHRvIGRvd25sb2FkIEFwYWNoZSBTcGFyay4=" \ sbin/start-master.sh ``` **AUTHORIZATION FAILURE** ``` $ curl -v -XPOST http://localhost:6066/v1/submissions/clear * Host localhost:6066 was resolved. * IPv6: ::1 * IPv4: 127.0.0.1 * Trying [::1]:6066... * connect to ::1 port 6066 from ::1 port 51705 failed: Connection refused * Trying 127.0.0.1:6066... * Connected to localhost (127.0.0.1) port 6066 > POST /v1/submissions/clear HTTP/1.1 > Host: localhost:6066 > User-Agent: curl/8.7.1 > Accept: */* > * Request completely sent off < HTTP/1.1 403 Forbidden < Date: Sat, 03 Aug 2024 22:18:03 GMT < Cache-Control: must-revalidate,no-cache,no-store < Content-Type: text/html;charset=iso-8859-1 < Content-Length: 590 < Server: Jetty(11.0.21) < <html> <head> <meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/> <title>Error 403 Authorization header is missing.</title> </head> <body><h2>HTTP ERROR 403 Authorization header is missing.</h2> <table> <tr><th>URI:</th><td>/v1/submissions/clear</td></tr> <tr><th>STATUS:</th><td>403</td></tr> <tr><th>MESSAGE:</th><td>Authorization header is missing.</td></tr> <tr><th>SERVLET:</th><td>org.apache.spark.deploy.rest.StandaloneClearRequestServlet-7f171159</td></tr> </table> <hr/><a href="https://eclipse.org/jetty">Powered by Jetty:// 11.0.21</a><hr/> </body> </html> * Connection #0 to host localhost left intact ``` **SUCCESS** ``` $ curl -v -H "Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.e30.4EKWlOkobpaAPR0J4BE0cPQ-ZD1tRQKLZp1vtE7upPw" -XPOST http://localhost:6066/v1/submissions/clear * Host localhost:6066 was resolved. * IPv6: ::1 * IPv4: 127.0.0.1 * Trying [::1]:6066... * connect to ::1 port 6066 from ::1 port 51697 failed: Connection refused * Trying 127.0.0.1:6066... * Connected to localhost (127.0.0.1) port 6066 > POST /v1/submissions/clear HTTP/1.1 > Host: localhost:6066 > User-Agent: curl/8.7.1 > Accept: */* > Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.e30.4EKWlOkobpaAPR0J4BE0cPQ-ZD1tRQKLZp1vtE7upPw > * Request completely sent off < HTTP/1.1 200 OK < Date: Sat, 03 Aug 2024 22:16:51 GMT < Content-Type: application/json;charset=utf-8 < Content-Length: 113 < Server: Jetty(11.0.21) < { "action" : "ClearResponse", "message" : "", "serverSparkVersion" : "4.0.0-SNAPSHOT", "success" : true * Connection #0 to host localhost left intact }% ``` ### Does this PR introduce _any_ user-facing change? No, this is a new feature which is not loaded by default. ### How was this patch tested? Pass the CIs with newly added test case. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #47595 from dongjoon-hyun/SPARK-49103. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
…REST API and rename parameter to `secretKey`
### What changes were proposed in this pull request?
This PR aims the following.
- Document `JWSFilter` and its usage in `Spark UI` and `REST API`
- `Spark UI` section of `Configuration` page
- `Spark Security` page
- `Spark Standalone` page
- Rename the parameter `key` to `secretKey` to redact it in Spark Driver UI and Spark Master UI.
### Why are the changes needed?
To apply recent new security features
- #47575
- #47595
### Does this PR introduce _any_ user-facing change?
No because this is a new feature of Apache Spark 4.0.0.
### How was this patch tested?
Pass the CIs and manual review.
- `spark-standalone.html`

- `security.html`


- `configuration.html`

### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #47596 from dongjoon-hyun/SPARK-49104.
Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
What changes were proposed in this pull request?
This PR aims to support
JWSFilterwhich is a servlet filter that requiresJWS, a cryptographically signed JSON Web Token, in the header viaspark.ui.filtersconfiguration.To simply put,
JWSFilterwill check the following for all requests.Authorization: Bearer <jws>header.<jws>is a string with three fields,<header>.<payload>.<signature>.<header>is supposed to be a base64url-encoded string of{"alg":"HS256","typ":"JWT"}.<payload>is a base64url-encoded string of fully-user-defined content.<signature>is a signature based on<header>.<payload>and a user-provided key parameter.For example, the value of
<header>will beeyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9always and the value ofpayloadcan bee30if the payload is empty,{}. The<signature>part is changed by the shared value ofspark.org.apache.spark.ui.JWSFilter.param.keybetween the server and client.Why are the changes needed?
To provide a little better security on WebUI consistently including Spark Standalone Clusters.
For example,
SETTING
SPARK-SHELL
Without JWS (ErrorCode: 403 Forbidden)
With JWS,
SPARK MASTER
Without JWS (ErrorCode: 403 Forbidden)
With JWS
Does this PR introduce any user-facing change?
No, this is a new filter.
How was this patch tested?
Pass the CIs.
Was this patch authored or co-authored using generative AI tooling?
No.