Skip to content

Conversation

@loserwang1024
Copy link
Contributor

Purpose

Linked issue: close #844

Brief change log

Tests

com.alibaba.fluss.rpc.netty.authenticate.AuthenticationTest#testRetirableAuthenticateException

API and Format

Documentation

if (!authenticator.isCompleted()) {
byte[] token = authenticateRequest.getToken();
byte[] challenge = authenticator.evaluateResponse(token);
if (!authenticator.isCompleted() && challenge != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Is the removal of !authenticator.isCompleted() necessary? I though the authenticator.evaluateResponse may change status of authenticator into complete and don't need to send challenge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some authentication protocol, the server will be complete before the client becomes complete.

Take SCRAM-SHA-256 of kafka for example,

  1. sasl server recieves RECEIVE_CLIENT_FINAL_MESSAGE, and then mark authenticator server status as COMPLETE, then still send serverFinalMessage
// org.apache.kafka.common.security.scram.internals.ScramSaslServer#evaluateResponse
case RECEIVE_CLIENT_FINAL_MESSAGE:
                    try {
                        ClientFinalMessage clientFinalMessage = new ClientFinalMessage(response);
                        verifyClientProof(clientFinalMessage);
                        byte[] serverKey = scramCredential.serverKey();
                        byte[] serverSignature = formatter.serverSignature(serverKey, clientFirstMessage, serverFirstMessage, clientFinalMessage);
                        ServerFinalMessage serverFinalMessage = new ServerFinalMessage(null, serverSignature);
                        clearCredentials();
                        setState(State.COMPLETE);
                        return serverFinalMessage.toBytes();
                    } catch (InvalidKeyException e) {
                        throw new SaslException("Authentication failed: Invalid client final message", e);
                    }
  1. Then sasl client recieve this RECEIVE_SERVER_FINAL_MESSAGE, then will set status and State.COMPLETE and then return null(no longer send anymore)
org.apache.kafka.common.security.scram.internals.ScramSaslClient#evaluateChallenge

 case RECEIVE_SERVER_FINAL_MESSAGE:
                    ServerFinalMessage serverFinalMessage = new ServerFinalMessage(challenge);
                    if (serverFinalMessage.error() != null)
                        throw new SaslException("Sasl authentication using " + mechanism + " failed with error: " + serverFinalMessage.error());
                    handleServerFinalMessage(serverFinalMessage.serverSignature());
                    setState(State.COMPLETE);
                    return null;

I add same logic in com.alibaba.fluss.rpc.netty.authenticate.MutualAuthenticationPlugin.

@loserwang1024 loserwang1024 force-pushed the retry-auth branch 2 times, most recently from d2e00e7 to 4d5ec5b Compare May 12, 2025 08:51
@loserwang1024
Copy link
Contributor Author

Rebase this pr with three modification:

  1. Add ExponentialBackoff to calculate the backoff time.
  2. Retain authentication logic in ServerConnection.
  3. Add initialize method for ServerAuthenticator and ClientAuthenticator to reset the status of authenticator.

@luoyuxia
Copy link
Contributor

Maybe we can verify this pr in our env before merge that to avoid go back and forth. Maybe in this afternoon or tomorrow to verify this pr.

@loserwang1024
Copy link
Contributor Author

Maybe we can verify this pr in our env before merge that to avoid go back and forth. Maybe in this afternoon or tomorrow to verify this pr.

ok, I will apply the modification of this pr to env test.

@wuchong
Copy link
Member

wuchong commented May 15, 2025

@loserwang1024 please ping me when you have verified in internal env.

@wuchong
Copy link
Member

wuchong commented May 19, 2025

It is passed in our internal env.

@wuchong wuchong merged commit 22f6015 into apache:main May 19, 2025
5 of 6 checks passed
caozhen1937 pushed a commit to caozhen1937/fluss that referenced this pull request May 21, 2025
polyzos pushed a commit to polyzos/fluss that referenced this pull request Jun 6, 2025
polyzos pushed a commit to polyzos/fluss that referenced this pull request Jun 6, 2025
@loserwang1024 loserwang1024 deleted the retry-auth branch June 17, 2025 03:43
ZmmBigdata pushed a commit to ZmmBigdata/fluss that referenced this pull request Jun 20, 2025
polyzos pushed a commit to polyzos/fluss that referenced this pull request Aug 30, 2025
polyzos pushed a commit to Alibaba-HZY/fluss that referenced this pull request Aug 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add retriable authentication exception

3 participants