[
https://issues.apache.org/jira/browse/SPARK-58658?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
L. C. Hsieh updated SPARK-58658:
--------------------------------
Description:
The Spark Connect Config RPC hands back any configuration key the session
holds, including keys that Spark treats as sensitive everywhere else. For
example the pre-shared authentication token:
spark.conf.get("spark.connect.authenticate.token") // returns the token
Keys reach the session config because SQLConf.mergeSparkConf copies every
SparkConf entry -- static ones included -- into it, and
SparkConnectConfigHandler's read paths (handleGet, handleGetOption,
handleGetWithDefault, handleGetAll) apply no denylist. RuntimeConfig guards
writes to static configs with requireNonStaticConf, but reads are unguarded. So
any secret that ends up in the driver's SparkConf is readable by a Connect
client, both keyed and through GetAll.
This is broader than the authentication token, and a secret can end up in
SparkConf in two unrelated ways:
- Placed there deliberately. spark.connect.authenticate.token is set on the
server so it can authenticate clients. In a standalone server that is between
the server and the client that already holds it. Behind a proxy that terminates
end-user authentication (JWT/OIDC) and reuses the token as a proxy-to-server
credential, the token is infrastructure the end user is not supposed to have --
yet the Config RPC hands it back to any client the proxy admits, who can then
reach a server directly.
- Carried there as a side effect of config passing. Apache Kyuubi passes its
engine configuration through --conf and prefixes non-spark. keys with spark.,
so deployment-wide secrets -- the ZooKeeper digest used for discovery-namespace
registration, and the pre-shared secret behind its internal engine tokens --
land in the driver's SparkConf without anyone intending them to be readable
over the wire. There is no proxy here; Kyuubi runs a Spark engine per end user,
and the Config RPC lets a client of one engine read secrets that are shared
across the whole deployment.
Proposed change: have the Config RPC read paths withhold any key whose name
matches the existing spark.redaction.regex
(default(?i)secret|password|token|access[.]?key), the same pattern Spark
already uses to redact configuration in the UI and logs. A withheld key is
reported the way an unset key is: Get and GetOption return no value,
GetWithDefault returns the caller's default, and GetAll omits it.
spark.connect.authenticate.token matches this pattern, so the token case is
covered as one instance of the general rule. Set/Unset already reject static
configs via requireNonStaticConf, and IsModifiable returns false for one
without disclosing anything.
Note for whoever implements this: handleGetAll strips the requested prefix from
the keys it returns, so the match has to run on the full key before the prefix
is removed.
was:
The Spark Connect Config RPC hands back any configuration key the session
holds, including keys that Spark treats as sensitive everywhere else. For
example the pre-shared authentication token:
spark.conf.get("spark.connect.authenticate.token") // returns the token
Keys reach the session config because SQLConf.mergeSparkConf copies every
SparkConf entry -- static ones included -- into it, and
SparkConnectConfigHandler's read paths (handleGet, handleGetOption,
handleGetWithDefault, handleGetAll) apply no denylist. RuntimeConfig guards
writes to static configs with requireNonStaticConf, but reads are unguarded. So
any secret that ends up in the driver's SparkConf is readable by a Connect
client, both keyed and through GetAll.
This is broader than the authentication token. Deployments that layer their
own configuration into SparkConf expose their own secrets the same way. Apache
Kyuubi, for instance, passes its engine configuration through --conf and
prefixes non-spark. keys with spark., so deployment-wide secrets -- the
ZooKeeper digest used for discovery-namespace registration, and the pre-shared
secret behind its internal engine tokens -- land in the driver's SparkConf and
are readable through the Config RPC (verified on 4.0.3 and 4.2.0, with the keys
Kyuubi produces and its redaction pattern set).
None of this is a vulnerability in the client-to-server model Spark documents:
a client that can call the Config RPC has already authenticated, and the
authentication token is a credential it had to present to connect. It matters
to deployments that reuse the token, or other secrets, as a value shared
between components while end users authenticate by other means -- a proxy in
front of Spark Connect, or the Kyuubi engine model.
There, any client the deployment admits can read a secret out of the driver and
reach a component directly. Spark does not promise otherwise, so this is
defense in depth rather than a fix for a Spark vulnerability; a server not
disclosing configuration it already considers sensitive is the better default
regardless.
Proposed change: have the Config RPC read paths withhold any key whose name
matches the existing spark.redaction.regex
(default(?i)secret|password|token|access[.]?key), the same pattern Spark
already uses to redact configuration in the UI and logs. A withheld key is
reported the way an unset key is: Get and GetOption return no value,
GetWithDefault returns the caller's default, and GetAll omits it.
spark.connect.authenticate.token matches this pattern, so the token case is
covered as one instance of the general rule. Set/Unset already reject static
configs via requireNonStaticConf, and IsModifiable returns false for one
without disclosing anything.
Note for whoever implements this: handleGetAll strips the requested prefix from
the keys it returns, so the match has to run on the full key before the prefix
is removed.
> Do not return redacted configurations from the Config RPC
> ---------------------------------------------------------
>
> Key: SPARK-58658
> URL: https://issues.apache.org/jira/browse/SPARK-58658
> Project: Spark
> Issue Type: Improvement
> Components: Connect
> Affects Versions: 4.2.0, 4.1.2, 4.3.0
> Reporter: L. C. Hsieh
> Assignee: L. C. Hsieh
> Priority: Major
> Labels: pull-request-available
>
> The Spark Connect Config RPC hands back any configuration key the session
> holds, including keys that Spark treats as sensitive everywhere else. For
> example the pre-shared authentication token:
> spark.conf.get("spark.connect.authenticate.token") // returns the token
> Keys reach the session config because SQLConf.mergeSparkConf copies every
> SparkConf entry -- static ones included -- into it, and
> SparkConnectConfigHandler's read paths (handleGet, handleGetOption,
> handleGetWithDefault, handleGetAll) apply no denylist. RuntimeConfig guards
> writes to static configs with requireNonStaticConf, but reads are unguarded.
> So any secret that ends up in the driver's SparkConf is readable by a Connect
> client, both keyed and through GetAll.
> This is broader than the authentication token, and a secret can end up in
> SparkConf in two unrelated ways:
> - Placed there deliberately. spark.connect.authenticate.token is set on the
> server so it can authenticate clients. In a standalone server that is between
> the server and the client that already holds it. Behind a proxy that
> terminates end-user authentication (JWT/OIDC) and reuses the token as a
> proxy-to-server credential, the token is infrastructure the end user is not
> supposed to have -- yet the Config RPC hands it back to any client the proxy
> admits, who can then reach a server directly.
> - Carried there as a side effect of config passing. Apache Kyuubi passes its
> engine configuration through --conf and prefixes non-spark. keys with spark.,
> so deployment-wide secrets -- the ZooKeeper digest used for
> discovery-namespace registration, and the pre-shared secret behind its
> internal engine tokens -- land in the driver's SparkConf without anyone
> intending them to be readable over the wire. There is no proxy here; Kyuubi
> runs a Spark engine per end user, and the Config RPC lets a client of one
> engine read secrets that are shared across the whole deployment.
> Proposed change: have the Config RPC read paths withhold any key whose name
> matches the existing spark.redaction.regex
> (default(?i)secret|password|token|access[.]?key), the same pattern Spark
> already uses to redact configuration in the UI and logs. A withheld key is
> reported the way an unset key is: Get and GetOption return no value,
> GetWithDefault returns the caller's default, and GetAll omits it.
> spark.connect.authenticate.token matches this pattern, so the token case is
> covered as one instance of the general rule. Set/Unset already reject static
> configs via requireNonStaticConf, and IsModifiable returns false for one
> without disclosing anything.
> Note for whoever implements this: handleGetAll strips the requested prefix
> from the keys it returns, so the match has to run on the full key before the
> prefix is removed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
