michaeljmarshall opened a new pull request, #19626: URL: https://github.com/apache/pulsar/pull/19626
### Motivation While reviewing the `ServerCnx`, I noticed the broker and client compatibility could have a missing edge case. In this case, the concept of multi-stage authentication was introduced with version 14 of the protocol by https://github.com/apache/pulsar/pull/3677. However, we send an `AuthChallenge` to the client without verifying it has a sufficient protocol version to support such a challenge. This change is unlikely to fix any real issues because the `AuthChallenge` was introduced a while ago. However, I think we should add this logic to further support our claim that any version of the client can connect to any version of the broker. Another reason we might choose not to merge this PR is that it's possible https://github.com/apache/pulsar/pull/3677 introduced mutual authentication in the client and the proxy at the same time, so this case isn't possible. My main response is that the protocol is an open spec, so we should use our versioning system to ensure that there isn't even a third party client library that receives protocol messages it is unable to handle. ### Modifications * When an old client tries to connect using an authentication provider that generates an auth challenge, send an `AuthenticationError` and close the connection for both broker and proxy. * Only send the `AuthChallenge` that is triggered by authentication refresh when the client knows how to handle the `AuthChallenge` protocol message. This is arguably covered by the `FeatureFlag`, but given what our protocol versioning means, I think we should cover this unlikely edge case. ### Verifying this change Includes a test for `ServerCnx` and I created an issue for https://github.com/apache/pulsar/issues/19624 the proxy. ### Does this pull request potentially affect one of the following parts: This enforces the existing protocol. ### Documentation - [x] `doc-not-needed` This is part of the protocol spec. ### Matching PR in forked repository PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/32 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
