You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@celeborn.apache.org by re...@apache.org on 2024/07/12 03:51:49 UTC
(celeborn) branch main updated: [CELEBORN-1493] Check admin privileges for http mutative requests
This is an automated email from the ASF dual-hosted git repository.
rexxiong pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/celeborn.git
The following commit(s) were added to refs/heads/main by this push:
new 09d3a3b05 [CELEBORN-1493] Check admin privileges for http mutative requests
09d3a3b05 is described below
commit 09d3a3b05fcec32ee1e10da748d5ebba4dde195f
Author: Fei Wang <fw...@ebay.com>
AuthorDate: Fri Jul 12 11:51:35 2024 +0800
[CELEBORN-1493] Check admin privileges for http mutative requests
### What changes were proposed in this pull request?
If authentication enabled, check admin privileges for http mutative requests.
Likes:
```
POST /api/v1/workers/exclude
POST /api/v1/workers/events
POST /api/v1/workers/exit
```
### Why are the changes needed?
For security requirement.
### Does this PR introduce _any_ user-facing change?
Yes, after this pr, if http authentication enabled, for all mutative http requests, it will check the admin privileges.
Before this PR, if an API is not defined and the method is `POST/PUT/DELETE/PATCH`, the response status code is `404`.
After this PR, if the admin privileges check failed, the response status code will be `403`.
### How was this patch tested?
UT.
Closes #2601 from turboFei/admin_check.
Authored-by: Fei Wang <fw...@ebay.com>
Signed-off-by: Shuang <lv...@alibaba-inc.com>
---
.../org/apache/celeborn/common/CelebornConf.scala | 22 ++++++++++++++++
docs/configuration/master.md | 1 +
docs/configuration/worker.md | 1 +
.../http/authentication/AuthenticationFilter.scala | 26 +++++++++++++++++--
.../http/ApiBaseResourceAuthenticationSuite.scala | 30 ++++++++++++++++++++++
5 files changed, 78 insertions(+), 2 deletions(-)
diff --git a/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala b/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
index a2b2e284f..5d89945ae 100644
--- a/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
+++ b/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
@@ -2271,6 +2271,17 @@ object CelebornConf extends Logging {
.stringConf
.createWithDefault(classOf[AnonymousAuthenticationProviderImpl].getName)
+ val MASTER_HTTP_AUTH_ADMINISTERS: ConfigEntry[Seq[String]] =
+ buildConf("celeborn.master.http.auth.administers")
+ .categories("master")
+ .version("0.6.0")
+ .doc("A comma-separated list of users who have admin privileges," +
+ s" Note, when ${MASTER_HTTP_AUTH_SUPPORTED_SCHEMES.key} is not set," +
+ s" everyone is treated as administrator.")
+ .stringConf
+ .toSequence
+ .createWithDefault(Seq.empty)
+
val HA_ENABLED: ConfigEntry[Boolean] =
buildConf("celeborn.master.ha.enabled")
.withAlternative("celeborn.ha.enabled")
@@ -2968,6 +2979,17 @@ object CelebornConf extends Logging {
.stringConf
.createWithDefault(classOf[AnonymousAuthenticationProviderImpl].getName)
+ val WORKER_HTTP_AUTH_ADMINISTERS: ConfigEntry[Seq[String]] =
+ buildConf("celeborn.worker.http.auth.administers")
+ .categories("worker")
+ .version("0.6.0")
+ .doc("A comma-separated list of users who have admin privileges," +
+ s" Note, when ${WORKER_HTTP_AUTH_SUPPORTED_SCHEMES.key} is not set," +
+ s" everyone is treated as administrator.")
+ .stringConf
+ .toSequence
+ .createWithDefault(Seq.empty)
+
val WORKER_RPC_PORT: ConfigEntry[Int] =
buildConf("celeborn.worker.rpc.port")
.categories("worker")
diff --git a/docs/configuration/master.md b/docs/configuration/master.md
index efa455451..5c5bd8ac2 100644
--- a/docs/configuration/master.md
+++ b/docs/configuration/master.md
@@ -43,6 +43,7 @@ license: |
| celeborn.master.heartbeat.application.timeout | 300s | false | Application heartbeat timeout. | 0.3.0 | celeborn.application.heartbeat.timeout |
| celeborn.master.heartbeat.worker.timeout | 120s | false | Worker heartbeat timeout. | 0.3.0 | celeborn.worker.heartbeat.timeout |
| celeborn.master.host | <localhost> | false | Hostname for master to bind. | 0.2.0 | |
+| celeborn.master.http.auth.administers | | false | A comma-separated list of users who have admin privileges, Note, when celeborn.master.http.auth.supportedSchemes is not set, everyone is treated as administrator. | 0.6.0 | |
| celeborn.master.http.auth.basic.provider | org.apache.celeborn.common.authentication.AnonymousAuthenticationProviderImpl | false | User-defined password authentication implementation of org.apache.celeborn.common.authentication.PasswdAuthenticationProvider | 0.6.0 | |
| celeborn.master.http.auth.bearer.provider | org.apache.celeborn.common.authentication.AnonymousAuthenticationProviderImpl | false | User-defined token authentication implementation of org.apache.celeborn.common.authentication.TokenAuthenticationProvider | 0.6.0 | |
| celeborn.master.http.auth.supportedSchemes | | false | A comma-separated list of master http auth supported schemes.<ul> <li>SPNEGO: Kerberos/GSSAPI authentication.</li> <li>BASIC: User-defined password authentication, the concreted implementation is configurable via `celeborn.master.http.auth.basic.provider`.</li> <li>BEARER: User-defined bearer token authentication, the concreted implementation is configurable via `celeborn.master.http.auth.bearer.provider`.</li></ul> | 0.6.0 | |
diff --git a/docs/configuration/worker.md b/docs/configuration/worker.md
index 53938ccc0..14addd604 100644
--- a/docs/configuration/worker.md
+++ b/docs/configuration/worker.md
@@ -86,6 +86,7 @@ license: |
| celeborn.worker.graceful.shutdown.saveCommittedFileInfo.interval | 5s | false | Interval for a Celeborn worker to flush committed file infos into Level DB. | 0.3.1 | |
| celeborn.worker.graceful.shutdown.saveCommittedFileInfo.sync | false | false | Whether to call sync method to save committed file infos into Level DB to handle OS crash. | 0.3.1 | |
| celeborn.worker.graceful.shutdown.timeout | 600s | false | The worker's graceful shutdown timeout time. | 0.2.0 | |
+| celeborn.worker.http.auth.administers | | false | A comma-separated list of users who have admin privileges, Note, when celeborn.worker.http.auth.supportedSchemes is not set, everyone is treated as administrator. | 0.6.0 | |
| celeborn.worker.http.auth.basic.provider | org.apache.celeborn.common.authentication.AnonymousAuthenticationProviderImpl | false | User-defined password authentication implementation of org.apache.celeborn.common.authentication.PasswdAuthenticationProvider | 0.6.0 | |
| celeborn.worker.http.auth.bearer.provider | org.apache.celeborn.common.authentication.AnonymousAuthenticationProviderImpl | false | User-defined token authentication implementation of org.apache.celeborn.common.authentication.TokenAuthenticationProvider | 0.6.0 | |
| celeborn.worker.http.auth.supportedSchemes | | false | A comma-separated list of worker http auth supported schemes.<ul> <li>SPNEGO: Kerberos/GSSAPI authentication.</li> <li>BASIC: User-defined password authentication, the concreted implementation is configurable via `celeborn.worker.http.auth.basic.provider`.</li> <li>BEARER: User-defined bearer token authentication, the concreted implementation is configurable via `celeborn.worker.http.auth.bearer.provider`.</li></ul> | 0.6.0 | |
diff --git a/service/src/main/scala/org/apache/celeborn/server/common/http/authentication/AuthenticationFilter.scala b/service/src/main/scala/org/apache/celeborn/server/common/http/authentication/AuthenticationFilter.scala
index 650ad268e..b90412ff4 100644
--- a/service/src/main/scala/org/apache/celeborn/server/common/http/authentication/AuthenticationFilter.scala
+++ b/service/src/main/scala/org/apache/celeborn/server/common/http/authentication/AuthenticationFilter.scala
@@ -21,6 +21,7 @@ import java.io.IOException
import javax.security.sasl.AuthenticationException
import javax.servlet.{Filter, FilterChain, FilterConfig, ServletException, ServletRequest, ServletResponse}
import javax.servlet.http.{HttpServletRequest, HttpServletResponse}
+import javax.ws.rs.HttpMethod
import scala.collection.mutable
@@ -64,6 +65,13 @@ class AuthenticationFilter(conf: CelebornConf, serviceName: String) extends Filt
case Service.WORKER => conf.get(CelebornConf.WORKER_HTTP_PROXY_CLIENT_IP_HEADER)
}
+ private val administrators: Set[String] = serviceName match {
+ case Service.MASTER =>
+ conf.get(CelebornConf.MASTER_HTTP_AUTH_ADMINISTERS).toSet
+ case Service.WORKER =>
+ conf.get(CelebornConf.WORKER_HTTP_AUTH_ADMINISTERS).toSet
+ }
+
private def initAuthHandlers(): Unit = {
if (authSchemes.contains(HttpAuthSchemes.NEGOTIATE)) {
serviceName match {
@@ -160,8 +168,15 @@ class AuthenticationFilter(conf: CelebornConf, serviceName: String) extends Filt
}
}
+ private def isMutativeRequest(httpRequest: HttpServletRequest): Boolean = {
+ HttpMethod.POST.equalsIgnoreCase(httpRequest.getMethod) ||
+ HttpMethod.PUT.equalsIgnoreCase(httpRequest.getMethod) ||
+ HttpMethod.DELETE.equalsIgnoreCase(httpRequest.getMethod) ||
+ HttpMethod.PATCH.equalsIgnoreCase(httpRequest.getMethod)
+ }
+
/**
- * Delegates call to the servlet filter chain. Sub-classes my override this
+ * Delegates call to the servlet filter chain. Sub-classes may override this
* method to perform pre and post tasks.
*
* @param filterChain the filter chain object.
@@ -176,7 +191,14 @@ class AuthenticationFilter(conf: CelebornConf, serviceName: String) extends Filt
filterChain: FilterChain,
request: HttpServletRequest,
response: HttpServletResponse): Unit = {
- filterChain.doFilter(request, response)
+ if (isMutativeRequest(request) && !administrators.contains(HTTP_CLIENT_IDENTIFIER.get())) {
+ response.setStatus(HttpServletResponse.SC_FORBIDDEN)
+ response.sendError(
+ HttpServletResponse.SC_FORBIDDEN,
+ s"${HTTP_CLIENT_IDENTIFIER.get()} does not have admin privilege to perform ${request.getMethod} action")
+ } else {
+ filterChain.doFilter(request, response)
+ }
}
override def destroy(): Unit = {
diff --git a/service/src/test/scala/org/apache/celeborn/server/common/http/ApiBaseResourceAuthenticationSuite.scala b/service/src/test/scala/org/apache/celeborn/server/common/http/ApiBaseResourceAuthenticationSuite.scala
index 938454707..41aa3269c 100644
--- a/service/src/test/scala/org/apache/celeborn/server/common/http/ApiBaseResourceAuthenticationSuite.scala
+++ b/service/src/test/scala/org/apache/celeborn/server/common/http/ApiBaseResourceAuthenticationSuite.scala
@@ -29,6 +29,7 @@ import org.apache.celeborn.server.common.http.HttpAuthUtils.AUTHORIZATION_HEADER
import org.apache.celeborn.server.common.http.authentication.{UserDefinePasswordAuthenticationProviderImpl, UserDefineTokenAuthenticationProviderImpl}
abstract class ApiBaseResourceAuthenticationSuite extends HttpTestHelper {
+ val administers = Seq("celeborn", "celeborn2")
celebornConf
.set(CelebornConf.METRICS_ENABLED.key, "true")
.set(
@@ -48,6 +49,8 @@ abstract class ApiBaseResourceAuthenticationSuite extends HttpTestHelper {
.set(
CelebornConf.WORKER_HTTP_AUTH_BEARER_PROVIDER,
classOf[UserDefineTokenAuthenticationProviderImpl].getName)
+ .set(CelebornConf.MASTER_HTTP_AUTH_ADMINISTERS, administers)
+ .set(CelebornConf.WORKER_HTTP_AUTH_ADMINISTERS, administers)
def basicAuthorizationHeader(user: String, password: String): String =
HttpAuthSchemes.BASIC + " " + new String(
@@ -117,4 +120,31 @@ abstract class ApiBaseResourceAuthenticationSuite extends HttpTestHelper {
assert(200 == response.getStatus)
assert(response.readEntity(classOf[String]).contains("\"name\" : \"jvm.memory.heap.max\""))
}
+
+ test("check admin privilege for mutative request") {
+ Seq("/any_api", "/api/v1/any_api").foreach { api =>
+ var response = webTarget.path(api)
+ .request(MediaType.TEXT_PLAIN)
+ .header(
+ AUTHORIZATION_HEADER,
+ basicAuthorizationHeader(
+ "no_admin",
+ UserDefinePasswordAuthenticationProviderImpl.VALID_PASSWORD))
+ .post(null)
+ assert(HttpServletResponse.SC_FORBIDDEN == response.getStatus)
+
+ administers.foreach { admin =>
+ response = webTarget.path(api)
+ .request(MediaType.TEXT_PLAIN)
+ .header(
+ AUTHORIZATION_HEADER,
+ basicAuthorizationHeader(
+ admin,
+ UserDefinePasswordAuthenticationProviderImpl.VALID_PASSWORD))
+ .post(null)
+ // pass the admin privilege check, but the api is not found
+ assert(HttpServletResponse.SC_NOT_FOUND == response.getStatus)
+ }
+ }
+ }
}