sarutak commented on code in PR #59185:
URL: https://github.com/apache/spark/pull/59185#discussion_r4163304871
##########
core/src/main/scala/org/apache/spark/deploy/security/UserCredentialManager.scala:
##########
@@ -519,6 +527,44 @@ private[spark] object UserCredentialManager extends
Logging {
}
}
+ /**
+ * Returns true if Spark's RPC channel encryption is configured: either
SSL-based RPC encryption
+ * (`spark.ssl.rpc.enabled`), or authentication (`spark.authenticate`)
together with either
+ * AES-based encryption (`spark.network.crypto.enabled`) or the separate
(deprecated) SASL-based
+ * encryption (`spark.authenticate.enableSaslEncryption`).
+ *
+ * This is shared with [[HadoopDelegationTokenManager]]'s
direct-credential-provider check so the
+ * two credential-propagation paths cannot drift apart: that path fails
startup via `require()`,
+ * while OIDC propagation only warns (see [[warnIfRpcEncryptionDisabled]]).
+ */
+ private[security] def isRpcEncryptionEnabled(sparkConf: SparkConf): Boolean
= {
+ sparkConf.getBoolean("spark.ssl.rpc.enabled", false) ||
+ (sparkConf.get(NETWORK_AUTH_ENABLED) &&
+ (sparkConf.get(NETWORK_CRYPTO_ENABLED) ||
sparkConf.get(SASL_ENCRYPTION_ENABLED)))
+ }
+
+ /**
+ * Logs a warning if OIDC credential propagation is enabled but RPC channel
encryption is not
+ * configured, since credentials would then be transmitted over the RPC
channel in cleartext.
+ * Unlike [[HadoopDelegationTokenManager]]'s direct-credential-provider path
(which fails fast),
+ * this only warns, per the SPIP's default behavior.
+ */
+ private[security] def warnIfRpcEncryptionDisabled(sparkConf: SparkConf):
Unit = {
+ if (!isRpcEncryptionEnabled(sparkConf)) {
+ // Reference config keys via `.key` (rather than hardcoded strings) so
the message stays in
+ // sync if a key is renamed. `spark.ssl.rpc.enabled` has no Scala
ConfigEntry (it is read
+ // directly in TransportConf), so it is spelled out literally here and in
+ // isRpcEncryptionEnabled.
+ val remediation = s"Enable RPC encryption via
spark.ssl.rpc.enabled=true, or " +
Review Comment:
Fixed both. The remediation string is now a single shared value,
`SecurityManager.rpcEncryptionRemediation`, built once from the config keys
(`SSL_RPC_ENABLED_CONF` and the `*.key` values), so there's no stray `s"..."`
without interpolation and no key spelled out in two places.
--
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]
