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

GH-533: Fix ClientUserAuthService iteration through methods #547

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

tomaswolf
Copy link
Member

Fix publickey authentication in multi-authentication: if a server is configured with "publickey,password,publickey", a client must authenticate with two different keys. Also, if a publickey authentication fails and the server sends back a list of further acceptable methods that does not contain publickey, then the client must not continue with publickey authentication.

Fixes #533.

The ClientUserAuthService iterates through the configured authentication
methods and tries them each in order, skipping those that the server did
not list as acceptable.

However, it only set the "server acceptable" on a partial success, or on
the initial failure for the "none" request. This was wrong: if a pubkey
auth fails and the server replies with SSH_MSG_USERAUTH_FAILURE and an
acceptable methods list not containing "pubkey", then pubkey should not
continue even if it had more keys available (or more signatures to try
for an RSA key).

This case actually occurs, for instance with an XFB.Gateway SSH server
and RSA keys, if an rsa-sha2* signature is used. That server apparently
knows only ssh-rsa signatures and consequently may reply with
SSH_MSG_USERAUTH_FAILURE, partialSuccess=false, allowed=password.
(Presumably if the key was right but the signature was unknown, and the
user has only a single authorized key, and password auth is enabled
server-side.)

So reset the "server acceptable" list on any SSH_MSG_USERAUTH_FAILURE,
even if partialSuccess = false. Check whether the current method is
still allowed, and if not start over instead of continuing with the
current method.

A second problem was with the handling of multiple required
authentication methods. A server may require multiple authentications,
for instance first a pubkey auth, then a password auth, and then again
a pubkey auth with another key. Such continuations are indicated by
SSH_MSG_USERAUTH_FAILURE with partialSuccess = true and a list of
methods to try next. For pubkey,password,pubkey the client would get a
message SSH_MSG_USERAUTH_FAILURE partialSuccess=true allowed=password
after the successful first pubkey authentication, and then a message
SSH_MSG_USERAUTH_FAILURE partialSuccess=true allowed=pubkey after the
successful password log-in. On that second use of pubkey auth, the
previously successful key must not be re-tried, and there's also no
point in re-trying previously rejected keys. Thus our code cannot simply
create a new UserAuthPublicKey instance again as this would re-load the
key iterator from scratch. Instead we must remember the pubkey auth
instance once created and re-use that same instance on the second use,
so that we keep on with any remaining keys that might be available.

See also https://issues.apache.org/jira/browse/SSHD-1229, which may be
related.

Bug: apache#533
@tomaswolf tomaswolf merged commit 4b6f809 into apache:master Jul 29, 2024
7 checks passed
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.

Client auth: "authentication methods that can continue" is mishandled
1 participant