Seokmo Yoo created KAFKA-21043:
----------------------------------
Summary: 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
Reporter: Seokmo Yoo
{{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)
