dongjoon-hyun commented on code in PR #59185:
URL: https://github.com/apache/spark/pull/59185#discussion_r4163562371
##########
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 " +
+ s"${NETWORK_AUTH_ENABLED.key}=true together with one of " +
+ s"${NETWORK_CRYPTO_ENABLED.key} / ${SASL_ENCRYPTION_ENABLED.key}."
+ logWarning(log"OIDC credential propagation is enabled " +
+ log"(${MDC(LogKeys.CONFIG, SECURITY_OIDC_ENABLED.key)}=true) but RPC
channel encryption " +
+ log"is not configured. Credentials will be transmitted to executors
over an unencrypted " +
+ log"channel. ${MDC(LogKeys.REASON, remediation)}")
Review Comment:
Thank you, @sarutak. The proposed `spark.security.oidc.requireRpcEncryption`
(default `false`) looks good to me. Please go ahead and include it in this PR.
--
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]
