Backport of: 2132 Fix support for enhanced authentication #2171
Conversation
|
@dotnet-policy-service agree company="Quest Software" |
| var receivedPacket = await Receive(cancellationToken).ConfigureAwait(false); | ||
|
|
||
| switch (receivedPacket) | ||
| for (; ; ) |
There was a problem hiding this comment.
What is the purpose of this loop? In v5 is also no loop required.
There was a problem hiding this comment.
Thanks for the review.
The loop is needed because when when the client sends the initial connection request, depending on what type of authentication is requested the server response will be either a MqttConnAckPacket or a MqttAuthPacket. The server will send a MqttAuthPacket if it needs to request additional authentication token data. Depending on the authentication mechanism, the server may send several MqttAuthPacket responses that the client will need to handle.
The client must respond to the MqttAuthPacket server requests until it receives a MqttConnAckPacket response.
In the Kerberos authentication sample in the MQTT v5 specification, the server sends two AUTH responses back to the client before finally sending a CONNACK response.
There was a problem hiding this comment.
From the MQTT specification under Enhanced Authentication:
The Client and Server exchange AUTH packets as needed until the Server accepts the authentication by sending a CONNACK with a Reason Code of 0.
There was a problem hiding this comment.
Sorry I was confused because the original code is 4.x. In 5.x we have a while(true) at the same place. But I am sure we should also check the cancellation token instead of building an endless loop.
There was a problem hiding this comment.
cancellationToken.ThrowIfCancellationRequested();
is the first line within the loop.
This PR is a backport of #2132 for the version 4 library.
This PR has a number of changes as compared to the PR on which it is based. Since this PR is a fix for the v4 library I stuck with the original "ExtendedAuthenticationExchange" naming convention that was used in the v4 library instead of the more correct "EnhancedAuthentication" naming convention used in the v5 library.
Additionally I tried to avoid any changes to existing interfaces/method signatures that would cause breaking changes. The PR on which these changes are based has many breaking changes as compared to the v4 library.
Another significant difference between this PR and the original is in the client handler. The original PR allowed you to both send and receive packets from inside the client handler until the exchange is complete. This PR uses the pre-existing handler context that only supports sending a response. As a result, a multi-step authentication exchange will result in the handler being called multiple times. This works fine, but it is a key difference between this PR and the v5 PR. I made this choice in an effort to avoid any breaking changes.
My company is unable to use the v5 library because we are constrained by having to support the .Net Framework, but we need to be able to authenticate clients using Enhanced Authentication.
This PR addresses the following issues: