x/crypto/ssh: add support for multi-step authentication

[thinxer]

Previous thread here: https://groups.google.com/forum/#!topic/golang-nuts/rxrYhntkQtI.

Currently the x/crypto/ssh package uses callbacks in ServerConfig to do the autentication. Supported callbacks including "password,publickey,keyboard-interactive", and the client authenticates successfully with any of the callbacks.

This makes it impossible to implement multi-step authentication correctly. An example multi-step authentication process is to do publickey first, then keyboard-interactive to verify OTP tokens. When a client attempts to login, the server must first respond with only publickey available. When the client successfully completes the first stage, the server will respond with an authentication error with PartialSuccess set, and with the next available method keyboard-interactive. The client then knows it can continue with the second stage.

I'd propose to add a NextAuthMethodsCallback to ssh.ServerConfig. An example implementation is here: https://gist.github.com/thinxer/637acd43480174fede118704f27530a6#file-authmethods-patch.

If this change looks good, I will add tests and submit a patch for code review.

[hanwen]

the FR seems valid in principle. The 1st approach seems sane, but I need a code review to look at it in more detail.

[danp]

We are doing publickey + keyboard-interactive currently with a workaround:

We generate a ServerConfig per client with callbacks to methods on the client. Then as KeyboardInteractiveCallback and PublicKeyCallback are called we note what has succeeded so far, with either callback returning success if everything has been satisfied.

Probably would be nicer to explicitly start with publickey, get that taken care of, then explicitly move to keyboard-interactive.

[hanwen]

what danp is doing is OK, but nothing sets PartialSuccess in the return message. I think we could have a specialized error type that an auth callback could hand back that causes PartialSuccess to be set. Possibly that should also error should also contain the next desired auth method.

[ibuler]

Any process now ?

[liuzheng]

I think this is a user side question, see this solution golang/crypto#89 (comment)

[drakkan]

I think this is a user side question, see this solution golang/crypto#89 (comment)

Hi,

based on RFC 4252, multi step authentication requires to return partial success, I don't see a partial success response in the code you linked, am I missing something? Thanks

[liuzheng]

@drakkan I'm not to do that, my way is you can define those multi step authentication requires as questions, and use question checker to verify it. If verified, keep the connection, if not, close it.

WHY

there are too many authentication methods in the world, no way to support it by a frame work, so I think use that question way is better.

[drakkan]

@drakkan I'm not to do that, my way is you can define those multi step authentication requires as questions, and use question checker to verify it. If verified, keep the connection, if not, close it.

WHY

there are too many authentication methods in the world, no way to support it by a frame work, so I think use that question way is better.

Well, a more cleaner way to ask for multiple questions is to use keyboard interactive authentication, it works well.

This issue seems focused on multi step authentication (please take a look at the forum link in the initial post). Multi step authentication, based on RFC 4252, requires to return a partial success after each step, so you are providing a workaround and not a proper solution, am I wrong?

I could be interested to send a pull request and/or to improve the existing one, but I don't want to waste my time if it cannot be merged, thanks

[liuzheng]

@drakkan ok

[drakkan]

Here is my attempt. My patch should be RFC compliant and allows to define per-user partial authentication methods.

[crazed]

@drakkan thank you for your PR, do you think this will get merged soon here?

[drakkan]

@drakkan thank you for your PR, do you think this will get merged soon here?

I don't know. This patch is included in SFTPGo 1.0.0 and allows to me to enable multi-step auth per-user (and not globally). It seems to work fine but only after a proper review I can understand if I missed something

[shanyou]

@drakkan thank you for your PR, do you think this will get merged soon here?

I don't know. This patch is included in SFTPGo 1.0.0 and allows to me to enable multi-step auth per-user (and not globally). It seems to work fine but only after a proper review I can understand if I missed something

If you use multi-step authentication, the permission must be inherited or merged. For example, when using ssh certificate as public key authentication first, some permission information will be extracted from the certificate, but it will be discarded in the later authentication process. You should know that the permission information in the certificate will be used by the server to judge the user's permission. such as "permit agent forwarding" or "permit port forwarding"

[drakkan]

@drakkan thank you for your PR, do you think this will get merged soon here?

I don't know. This patch is included in SFTPGo 1.0.0 and allows to me to enable multi-step auth per-user (and not globally). It seems to work fine but only after a proper review I can understand if I missed something

If you use multi-step authentication, the permission must be inherited or merged. For example, when using ssh certificate as public key authentication first, some permission information will be extracted from the certificate, but it will be discarded in the later authentication process. You should know that the permission information in the certificate will be used by the server to judge the user's permission. such as "permit agent forwarding" or "permit port forwarding"

Hi, we could reuse or extend the pubKeyCache for this. I don't need this feature myself for now. I have no problem to try to add this support but I would like to receive a review/direction from someone with repository write access. Upstream seems not to be very interested in this feature: no comments or reviews until now

[slifty]

It's been a few years so I wanted to check to see if this might be able to get resolved -- a project I'm working on would really benefit from support for multifactor SSH auth.

[gopherbot]

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