[
https://issues.apache.org/jira/browse/KAFKA-21043?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Seokmo Yoo updated KAFKA-21043:
-------------------------------
Component/s: clients
> LoginManager cache omits the class loader, so the credential refresh thread
> outlives its deployment (SASL/OAUTHBEARER)
> ----------------------------------------------------------------------------------------------------------------------
>
> Key: KAFKA-21043
> URL: https://issues.apache.org/jira/browse/KAFKA-21043
> Project: Kafka
> Issue Type: Bug
> Components: clients
> Reporter: Seokmo Yoo
> Priority: Major
>
> {{LoginManager}} caches instances by JAAS configuration, login class, login
> callback class and the {{sasl.*}} configs. The caller's class loader is not
> part
> of the key. In containers that give each deployment its own class loader
> (Connect, Flink, Spark, application servers), a {{Login}} can outlive the
> deployment that created it. For SASL/OAUTHBEARER the credential refresh then
> fails permanently.
> h3. Sequence
> # Deployment A creates a client. Its LoginManager is cached and
> {{ExpiringCredentialRefreshingLogin}} starts a refresh thread. The thread
> inherits A's context class loader, because {{KafkaThread}} does not set one.
> # Deployment B creates a client with the same JAAS configuration, receives
> A's
> LoginManager, and raises the reference count to two.
> # Deployment A stops and releases its reference. The count drops to one
> rather
> than zero, so {{login.close()}} never runs and the refresh thread keeps going
> with A's class loader.
> # The container discards A's class loader. {{LoginContext}} captures the
> context
> class loader at construction and resolves login modules through
> {{{}ServiceLoader{}}}. {{reLogin()}} builds its {{LoginContext}} on the same
> thread,
> so the class loader never changes and every retry fails the same way.
> # The credential expires and clients that need to re-authenticate are
> rejected.
> The last step lags the rest. Authentication happens only when a connection is
> established, so open connections keep working with an expired token. Which
> client breaks first depends on reconnection timing, which makes the failure
> hard
> to trace back to the redeployment.
> h3. Observed failure
> {code:java}
> javax.security.auth.login.LoginException: java.lang.IllegalStateException:
> Trying to access closed classloader
> at javax.security.auth.login.LoginContext.invoke(LoginContext.java:691)
> at javax.security.auth.login.LoginContext.login(LoginContext.java:587)
> at
> org.apache.kafka.common.security.oauthbearer.internals.expiring.ExpiringCredentialRefreshingLogin.reLogin(ExpiringCredentialRefreshingLogin.java:390)
> {code}
> The broker then rejects the client:
> {code:java}
> {"status":"invalid_token"}
> {code}
> h3. Reproduction
> A standalone test that uses only Kafka APIs, with no container framework,
> asserts three things:
> # Two callers on different class loaders with the same JAAS string receive
> the
> same {{LoginManager}} and share one refresh thread.
> # That thread's context class loader belongs to whichever caller acquired
> first.
> # After the first caller releases and its class loader is discarded, the
> thread
> is still alive and still holds that class loader, and building a
> {{LoginContext}} on it fails.
> Reproduces on trunk.
> h3. Proposed fix
> Add the context class loader to the cache key. Each deployment then owns its
> LoginManager, the reference count reaches zero when the deployment stops, and
> the refresh thread ends with it.
> This adds no new retention path. A cached LoginManager already holds the class
> loader strongly through the {{Login}} and its login module instances.
> Applications with a single class loader, brokers included, behave exactly as
> before, because the context class loader is then the same for every caller.
> Three lines of production code in {{{}LoginManager{}}}, plus a regression
> test.
> Reverting the fix makes that test fail.
> h3. The key has been widened twice for the same reason
> Each time, a dimension that must not be shared turned out to be missing from
> it.
> ||Change||Key||
> |KIP-83 / KAFKA-4180 (0.10.2.0)|JAAS config|
> |KIP-86 / KAFKA-4292 (2.0.0)|+ login class, login callback class|
> |KAFKA-14676 (3.3.3, 3.4.1, 3.5.0)|+ all {{sasl.*}} configs|
> The commit for KAFKA-14676 states the assumption the earlier key rested on:
> {quote}This implementation is based on the assumption clients that require
> different
> logins (e.g. username/password) use different JAAS configs, because login
> properties are included in the JAAS config rather than as separate top-level
> configs.
> {quote}
> That assumption broke once OIDC put the token endpoint URL in a top-level
> config, so the key grew to cover it. The class loader breaks it in a different
> way: two deployments can be identical in every configuration value and still
> must not share a {{{}Login{}}}, because the {{Login}} owns a thread whose
> context
> class loader belongs to whichever deployment created it first. No
> configuration
> value can express that.
> h3. Question
> Before treating this as a defect I would like to confirm the intent:
> # Is sharing a LoginManager between callers with identical configuration
> meant
> to hold regardless of the caller's class loader, or was the class loader not a
> consideration when the key was defined?
> # Is there a requirement that one JAAS configuration maps to exactly one
> {{Login}} per JVM, which adding the class loader to the key would break?
> KIP-83 says only that "LoginManager caching will be keyed on the jaas
> configuration object" and does not mention class loaders, so I could not
> settle
> this from the design documents. If the sharing is deliberate and must hold
> across class loaders, the fix belongs somewhere else, most likely in having
> {{ExpiringCredentialRefreshingLogin}} set the context class loader on its own
> thread instead of inheriting one.
> h3. Related
> * KAFKA-4180 and KIP-83 introduced sharing of logins between clients in one
> JVM.
> * KAFKA-14676 widened the key for the same class of problem.
> * KAFKA-13519 reports a related symptom, still open.
> I will attach a pull request.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
