Skip to content

Conversation

@luozhenyu
Copy link
Contributor

Type of change

  • Bugfix

Description

I set up an SASL authentication with the KafkaAuthnHandler. The ApiVersions and SaslHandshake request work just fine. However, the SaslAuthenticate request is sent but not received by netty. After some debugging, I found that the auto-read config is disabled by the KafkaProxyFrontendHandler.

Kafka Traffic

After authentication, kafka client will send an ApiVersions request again that will transition the state from AUTHN_SUCCESS to API_VERSIONS, which I think is inappropriate.

An OpaqueRequestFrame will also be treated as an unexpected message.

    @Override
    public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
        if (msg instanceof BareSaslRequest) {
            handleBareRequest(ctx, (BareSaslRequest) msg);
        }
        else if (msg instanceof DecodedRequestFrame) {
            handleFramedRequest(ctx, (DecodedRequestFrame<?>) msg);
        }
        else {
            throw new IllegalStateException("Unexpected message " + msg.getClass());
        }
    }

Additional Context

Set up custom SASL mechanisms with the KafkaAuthnHandler.

Checklist

  • Write tests
  • Make sure all tests pass
  • Review performance test results. Ensure that any degradations to performance numbers are understood and justified.
  • Make sure all Sonar-Lint warnings are addressed or are justifiably ignored.
  • Update documentation
  • Reference relevant issue(s) and close them after merging
  • For user facing changes, update CHANGELOG.md (remember to include changes affecting the API of the test artefacts too).

@luozhenyu luozhenyu changed the title Authn Fix KafkaAuthnHandler Dec 27, 2023
@luozhenyu luozhenyu changed the title Fix KafkaAuthnHandler Fix problems in KafkaAuthnHandler Dec 27, 2023
@luozhenyu luozhenyu changed the title Fix problems in KafkaAuthnHandler Fix issues in KafkaAuthnHandler Dec 27, 2023
Copy link
Member

@tombentley tombentley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @luozhenyu!

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 4, 2024

@tombentley tombentley merged commit db1282f into kroxylicious:main Jan 4, 2024
@luozhenyu luozhenyu deleted the authn branch January 4, 2024 02:46
@robobario
Copy link
Member

Thanks for the contribution @luozhenyu

@SamBarker
Copy link
Member

Thanks for taking the time to test kroxylicious and find this @luozhenyu, very much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants