proposal: x/crypto/ssh: dynamic auth method selection in ServerConfig

[awly]

Proposal Details

Today, when an auth method fails, server returns a SSH_MSG_USERAUTH_FAILURE with a set of possible methods that the client can attempt next. This set of methods is based on what auth callbacks are set in the ServerConfig.

The proposal is to add a new callback that allows for customizing this set of auth methods per connection, based on previous errors:

type ServerConfig struct
	...

	// NextAuthMethodCallback, if non-nil, is called whenever an authentication
	// method fails. It's called after AuthLogCallback, if set. The return
	// values are the SSH auth types (from
	// https://www.iana.org/assignments/ssh-parameters/ssh-parameters.xhtml#ssh-parameters-10
	// such as "password", "publickey", "keyboard-interactive", etc)
	// to suggest for the client to try next. If empty, authentication fails.
	NextAuthMethodCallback func(conn ConnMetadata, prevErrors []error) []string
}

An example use case: using NoClientAuthCallback to learn some information about the client and then selectively enable a subset of auth methods that could succeed for them.
Another use case: when using an out-of-band authentication mechanism (such as an authenticated tunnel that carries the SSH connection), NextAuthMethodCallback can return a custom auth method name to the client, as an indication of where authentication failed.

cc @bradfitz @maisem

[bradfitz]

As background for @rolandshoemaker @FiloSottile et al, this is one of the patches we're carrying in our x/crypto/ssh fork and we're trying to unfork. We're flexible on the API naming/docs/promises but we'd need something dynamic here.

[seankhliao]

cc @drakkan @golang/security

[drakkan]

@awly @bradfitz can you please take a look at this CL? Could this be useful for your use case too?

I think the API you suggest should also allow multi-step authentication, so we should converge on a proposal and implement it. Thank you

[awly]

@drakkan thanks for the CL link!
It handles one half of our use-case.

The other half, which is a little unconventional, is to return a custom auth method name (one that doesn't map to existing callbacks) as a hint to the client.
One option would be to add AdditonalMethods []string to PartialSuccessError in your PR.
Another option would be #64962, so we could send a helpful error message via a banner from one of the auth callbacks.

Ideally, I'd like the latter. It handles the "please authenticate using this URL in your browser" flow in our setup.

[drakkan]

It handles one half of our use-case.

It would be helpful if you could share your thoughts on linked CL, does it simplify or complicate your application code compared to the NextAuthMethodCallback method? Thank you.

Another option would be #64962, so we could send a helpful error message via a banner from one of the auth callbacks.

This weekend I'll take a more in-depth look at this proposal. It seems very useful!

Ideally, I'd like the latter. It handles the "please authenticate using this URL in your browser" flow in our setup.

At first glance I also prefer a separate proposal for sending dynamic banner messages to clients from authentication callbacks.

[awly]

Left some comments on the CL, but yes: PartialSuccessError simplifies what we're trying to do here 👍

[gopherbot]

Change https://go.dev/cl/516355 mentions this issue: ssh: add server side multi-step authentication

[FiloSottile]

To confirm, a combination of (the latest iteration of) #61447 and #64962 would address this, correct?

[awly]

Correct, I'll close this proposal in favor of the other two.