sunchao commented on code in PR #57525:
URL: https://github.com/apache/spark/pull/57525#discussion_r3722739845
##########
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala:
##########
@@ -62,6 +63,10 @@ class CoarseGrainedSchedulerBackend(scheduler:
TaskSchedulerImpl, val rpcEnv: Rp
// Total number of executors that are currently registered
protected val totalRegisteredExecutors = new AtomicInteger(0)
protected val conf = scheduler.sc.conf
+ private[spark] val driverInstanceToken = UUID.randomUUID().toString
+ // Propagate the token as an environment variable to executor processes
(YARN, Kubernetes).
+ // Standalone mode adds it to Command.environment directly in
StandaloneSchedulerBackend.
+ conf.setExecutorEnv(EXECUTOR_DRIVER_INSTANCE_TOKEN, driverInstanceToken)
Review Comment:
[P1] Keep the driver bearer token out of Kubernetes Pod specifications
`setExecutorEnv` sends this UUID through
`KubernetesExecutorConf.environment` into `BasicExecutorFeatureStep`, where
`KubernetesUtils.buildEnvVars` writes it as a literal `EnvVar.value` beside
`SPARK_DRIVER_URL`. The driver's service account needs Pod `get`/`list`/`watch`
permissions, and executors inherit that service account by default, so an
executor or another application sharing the namespace can read application B's
executor Pod and recover both B's driver address and token. With the default
`spark.authenticate=false`, it can replay `RetrieveSparkAppConfigWithIdentity`
to obtain B's I/O encryption key and Hadoop delegation tokens, or submit
`RegisterExecutor`; thus this proposed per-driver identity check provides no
isolation in that deployment. The existing documentation about Pod exposure of
`_SPARK_AUTH_SECRET` applies when RPC authentication is enabled and does not
cover the default unauthenticated case. Please distribute the token through an
application-isolated channel
instead of a literal Pod environment value and add a generated-Pod
non-disclosure regression.
##########
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala:
##########
@@ -149,7 +154,9 @@ class CoarseGrainedSchedulerBackend(scheduler:
TaskSchedulerImpl, val rpcEnv: Rp
// Spark configuration sent to executors. This is a lazy val so that
subclasses of the
// scheduler can modify the SparkConf object before this view is created.
private lazy val sparkProperties = scheduler.sc.conf.getAll
- .filter { case (k, _) => k.startsWith("spark.") }
+ .filter { case (k, _) =>
+ k.startsWith("spark.") && k !=
s"spark.executorEnv.$EXECUTOR_DRIVER_INSTANCE_TOKEN"
Review Comment:
[P1] Keep the driver bearer token out of user-visible session configuration
Filtering the `SparkAppConfig` reply does not remove the token already
stored in `scheduler.sc.conf`. `SparkSession` copies `sparkContext.conf.getAll`
into every session's `SQLConf`, and
`SparkConnectConfigHandler.handleGet`/`handleGetAll` return those values
without redaction, so any Spark Connect client can directly call
`spark.conf.get("spark.executorEnv.SPARK_EXECUTOR_DRIVER_INSTANCE_TOKEN")` and
recover the raw per-driver bearer token. SQL-only Thrift/JDBC clients can also
retrieve it with `SET spark.sql.legacy.setCommandRejectsSparkCoreConfs=false;
SET spark.redaction.regex=^$; SET
spark.executorEnv.SPARK_EXECUTOR_DRIVER_INSTANCE_TOKEN;`; I reproduced this
sequence and confirmed it changes only session-local redaction. When the driver
RPC endpoint is reachable with default authentication disabled, that token
permits credential-bearing bootstrap requests or executor registration, despite
Spark Connect's documented isolation from driver/static configuration. Please
keep this b
earer secret out of shared `SparkConf`/SQL session state, or prevent its
extraction across every remote configuration boundary, and add Connect plus SQL
non-disclosure regressions.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
