x/crypto/ssh: add support for multi-step authentication #17889
Comments
thinxer
changed the title
quentinmit
added
the
NeedsDecision
|
the FR seems valid in principle. The 1st approach seems sane, but I need a code review to look at it in more detail. |
|
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. |
|
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. |
|
Any process now ? |
|
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 |
|
@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. WHYthere 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 |
|
@drakkan ok |
|
Here is my attempt. My patch should be RFC compliant and allows to define per-user partial authentication methods. |
|
@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 |
|
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. |
|
Change https://go.dev/cl/516355 mentions this issue: |
Add support for sending back partial success to the client while handling authentication in the server. This is implemented by a special error that can be returned by any of the authentication methods, which contains the authentication methods to offer next. This patch is based on CL 399075 with some minor changes and the addition of test cases. Fixes golang/go#17889 Fixes golang/go#61447 Fixes golang/go#64974 Change-Id: I05c8f913bb407d22c2e41c4cbe965e36ab4739b0
Add support for sending back partial success to the client while handling authentication in the server. This is implemented by a special error that can be returned by any of the authentication methods, which contains the authentication methods to offer next. This patch is based on CL 399075 with some minor changes and the addition of test cases. Fixes golang/go#17889 Fixes golang/go#61447 Fixes golang/go#64974 Change-Id: I05c8f913bb407d22c2e41c4cbe965e36ab4739b0
dmitshur
added
NeedsFix
Previous thread here: https://groups.google.com/forum/#!topic/golang-nuts/rxrYhntkQtI.
Currently the
x/crypto/sshpackage uses callbacks inServerConfigto 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
publickeyfirst, thenkeyboard-interactiveto verify OTP tokens. When a client attempts to login, the server must first respond with onlypublickeyavailable. When the client successfully completes the first stage, the server will respond with an authentication error withPartialSuccessset, and with the next available methodkeyboard-interactive. The client then knows it can continue with the second stage.I'd propose to add a
NextAuthMethodsCallbacktossh.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.
The text was updated successfully, but these errors were encountered: