Add a SaslTermination filter

[tombentley]

Summary

This proposal adds a SaslTermination filter that authenticates Kafka clients directly at the proxy, without forwarding SASL exchanges to the upstream broker. This enables credential isolation, authentication protocol translation, and centralized credential management.

Motivation

Some routing implementations depend on knowing the client's Subject, but mechanisms like SCRAM-SHA are incompatible with the existing SASL inspection technique (each server issues a unique challenge, so SASL Authenticate can't be fanned out). SASL termination solves this by having the proxy perform the full SASL exchange itself.

Scope

The proposal covers:

• Support for SCRAM-SHA-256/512, and OAUTHBEARER
• A state machine managing the SASL exchange lifecycle
• Timing side-channel mitigation via fixedAuthDelay
• Reauthentication support

DoS protection (rate limiting / exponential backoff) and validation of legal SASL filter compositions within filter chains are out of scope.

Part of kroxylicious/kroxylicious#4391

[tombentley]

Please can someone review this PR? @k-wall @SamBarker @robobario

[SamBarker]

Thanks for putting this together Tom — there's clearly a lot of thorough thinking here.

I'm going to be honest: I've spent a good amount of time on this and I haven't made it much past the SaslTerminationConfig section. There's a wall of detail that makes it hard to form a high-level picture of the design before getting pulled into component-level specifics.

I think the proposal would benefit from a 40,000 ft overview early on — something showing how the components compose, what calls what, and where the module boundaries are. Without that, I'm left assembling the big picture from seven component descriptions, and I'm not confident I have it right.

I'll continue working through the rest of the proposal, but wanted to get these comments posted rather than sit on them.

[SamBarker]

The name MechanismHandler doesn't convey what this component actually does. From Kroxylicious's perspective, the filter's job is to validate client credentials — whether that takes one round (OAUTHBEARER) or multiple (SCRAM) is an implementation detail of how the validation happens, not a different responsibility.

I understand "mechanism" comes from the SASL spec, but in our codebase the meaningful concept is credential validation. Something like CredentialValidator / CredentialValidatorFactory would make the role immediately clear to someone reading the code without SASL RFC context.

I'm also a bit wary of "Handler" given our Netty context — it invites confusion with ChannelHandler, which is a different thing entirely.

[tombentley]

The thing which defined and uses the MechanismHandler interface is the SaslTermination filter. That's the relevant perspective by which we should judge the suitability of names. From that PoV SASL is a given, and so using SASL specific terms, like 'mechanism', in named seems entirely appropriate.

I'm open to suggestions about 'handler'. 'Callback' is okayish, but doesn't quite feel right given how it's used.

[SamBarker]

I still think naming in terms of roles (what the component does) rather than protocol concepts (where it comes from) would serve readers better, but I don't feel strongly enough to hold up the proposal. A few alternatives to "Handler" if you're open to it:

• MechanismAuthenticator — direct role-based, it authenticates
• MechanismExchange — leans into SASL spec language, one instance per connection maps to one exchange per connection
• MechanismValidator — what it ultimately decides: are these credentials valid?

For what it's worth, "Mechanism" feels redundant in the impl names — you already know it's SCRAM-SHA-256:

Interface	Impl
MechanismAuthenticator	ScramSha256Authenticator
MechanismExchange	ScramSha256Exchange
MechanismValidator	ScramSha256Validator

[tombentley]

Noted. If we drop the service loading part these are gonna be internal interfaces (not public API), so we're at liberty to decide when a code PR shows up.

[tombentley]

Thanks for taking a look @SamBarker, I'll try to address your comments next week, but:

I think the proposal would benefit from a 40,000 ft overview early on — something showing how the components compose, what calls what, and where the module boundaries are.

I added a UML diagram which renders when you view the rendered page (but not in the code review view):

[SamBarker]

Thanks Tom. I was working from a local checkout and only found the updates after I'd submitted my comments.

I found the single UML diagram hard to parse — it covers module boundaries, inheritance hierarchies, and runtime interactions all in one. I had a go at breaking it down into separate views at different levels of detail: https://gist.github.com/SamBarker/eddb68a6797c43bda4cbaee4bf9043bf

I think some of these would be worth including in the proposal but not necessarily all of them. Have a look and pick whichever ones you think are strongest?

[SamBarker]

Continuing the review — comments on the remaining components, credential store implications, and system-level failure modes.

[robobario]

Might be useful to have a counter for unauthorized RPCs that arrive before the user is authenticated.

[robobario]

we mention security quite a bit, should we put something up top near the summary as a statement about our overall security posture for this solution. It feels like implicitly we value this solution being secure because of the Authn/Authz realm it sits in, and we are choosing to diverge from what Kafka offers (SCRAM password minimum lengths, SASL Plain) because of this unwritten posture. I think it's worth being explicit.

[tombentley]

It feels like implicitly we value this solution being secure because of the Authn/Authz realm it sits in

Well, yes, there's not point string to terminate SASL if we do so with glaring flaws.

and we are choosing to diverge from what Kafka offers (SCRAM password minimum lengths, SASL Plain)

FWIW:

• Minimum password length is just enforcing NIST recommendations, IIRC. We don't have to do that, but I think it moves us closer to being able to claim "secure by default". Also, since NIST now recommends against forced password rotation the JSON lacks a last_changed_date. But these things are always trade-offs. It seems possible that someone will come along with a compliance requirement to enforce password ageing. It doesn't have to be perfect right now.
• The choice to use a larger iteration count comes from OWASP. It basically boils down to making brute force attacks (whether online or offline) both and slower and more mostly.
• SASL Plain: We could offer it, of course. It's not ruled out by this proposal. But I'd rather keep this proposal small. SCRAM provides a simple, password-based mechanism without requiring additional infra (c.f. OAUTHBEARER) which is just as usable from the PoV of a app developer writing a Kafka application. So I don't see a good use case for PLAIN.

[robobario]

👍 I should have been clearer, I agree with the proposal diverging from Kafka, using more up to date security requirements than whatever Kafka chose back in the day. Was just asking for a sentence somewhere saying, "since this is in the security domain we're going to adopt current/stronger security advice rather than support everything Apache Kafka does"

[tombentley]

Added an explicit security posture statement at line 62. Addressed in 30f58af.

[robobario]

might be worth adding the unsupported mechanism case stating we remain in RequiringHandshake and returns UNSUPPORTED_SASL_MECHANISM with supported list. Which looks like what's in the PoC

[k-wall]

+1

[robobario]

something claude picked out of your PoC is that we should be careful with logging or returning error messages to the client which could contain details like "JWT expired at...", "wrong audience", or "wrong issuer" . Giving the client information about whether they've got the right IdP but wrong claims and so on could be a risk. Similar to scram I think these should be hidden behind a generic "Authentication failed".

[tombentley]

Added to the OAUTHBEARER threats table: the filter returns a generic "Authentication failed" to the client, matching the SCRAM approach. Addressed in 2e29fe2.

[robobario]

In the PoC the oauth handler factory is manipulating the org.apache.kafka.sasl.oauthbearer.allowed.urls system property as part of the interaction with the apache kafka components, I think this is worth calling out somewhere as a challenge of integrating with the kafka internals and why we diverge from using our own sys prop or other proxy-wide configuration.

[k-wall]

The proposal probably ought to say the validating legal compositions of SASL Filters within the filter chains is not in scope for this proposal.

[k-wall]

Clients in authentication fail loops are common place in production systems. There's also DoS issue possibility. As the proposal stands the client's reuath attempt will be slowed by fixedAuthDelay. Is this sufficient? Should we be thinking about exponential delay strategy (username or token based, the latter like io.kroxylicious.filter.oauthbearer.OauthBearerValidationFilter#rateLimiter)

[k-wall]

Very thorough work @tombentley. I've left some comments to consider.

[tombentley]

Thanks for the reviews @SamBarker @robobario @k-wall, I think I've addressed many of them, so please take another look when you have time.

[k-wall]

Thanks @tombentley, happy to approve but for the absence of a response on the DoS possibility. It would be good to see unsupport mech handling in the state machine too.

Can I also ask you add a very quick summary to the description field of this PR too explain the motivation etc?

[tombentley]

Sorry @k-wall:

Clients in authentication fail loops are common place in production systems. There's also DoS issue possibility. As the proposal stands the client's reuath attempt will be slowed by fixedAuthDelay. Is this sufficient? Should we be thinking about exponential delay strategy (username or token based, the latter like io.kroxylicious.filter.oauthbearer.OauthBearerValidationFilter#rateLimiter)

fixedAuthDelay is specifically a timing side-channel mitigation, not a DoS countermeasure. I think DoS protection via rate limiting or quotas would be better served by a dedicated filter that can be reused outside of SASL termination.

[k-wall]

Nice work @tombentley. Happy to see this merged.