Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NoneAuthenticator Race Condition Causes Authentication Failure on PublicKey #13

Closed
dandalf opened this issue Mar 4, 2021 · 4 comments
Closed

Comments

@dandalf
Copy link
Contributor

dandalf commented Mar 4, 2021

I am trying to test connecting various private key formats to the server and I am running into an occasional issue that says that publickey authentication failed.

I've dug into the code and have found that this is caused by the server sending a response to an attempt at "none" authentication, but the client code thinks the failure message is from the publickey authenticator.

I have traced this to a race condition in the SshClient constructor. This only happens occasionally but the pattern for the failed cases is:

  1. none authentication is attempted via the AuthenticationProtocolClient class' start() method.
  2. public key authentication is attempted, which sets the currentAuthenticator in AuthenticationProtocolClient to the public key authenticator
  3. A SSH_MSG_USERAUTH_FAILURE message is received from the server for the none authentication and processed in AuthenticationProtocolClient.processMessage. Since the "currentAuthenticator" is set to the public key authenticator, the client reports the failure incorrectly on the publickey.

I have been trying to find a way to configure the client to not send the NoneAuthentication but I think it is not possible.

@ludup
Copy link
Contributor

ludup commented Mar 4, 2021

I'll fix the race condition. It's not advisable to stop none authentication from being sent.

@dandalf
Copy link
Contributor Author

dandalf commented Mar 4, 2021

I think it may be happening because my tests are creating and destroying SshClient too quickly. I think this queue may not be cleared by the time I create another SshClient:
private static final Integer ACTIVE_SERVICE_IN = ExecutorOperationQueues.generateUniqueQueue("TransportProtocol.activeService.in");

@ludup
Copy link
Contributor

ludup commented Mar 7, 2021

It's a problem in AuthenticationProtocolClient. Now that I'm aware of it you can see the issue quite clearly, currentAuthenticator variable gets overridden by the next authenticator so as you suspected received the failure message that should have been routed to the NoneAuthenticator.

I've done a little refactor and improved the logic here. I've also added a check and some synchronization to ensure that the none authentication is complete.

I need to test it a little more but have committed so you can check too.

@dandalf
Copy link
Contributor Author

dandalf commented Mar 17, 2021

Thank you! That seems like it was the problem, my rapid connect/disconnect loop tests are passing with flying colors now.

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

No branches or pull requests

2 participants