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

x/crypto/ssh: allow retrying AuthMethods #16077

Closed
jbeverly opened this issue Jun 15, 2016 · 4 comments
Closed

x/crypto/ssh: allow retrying AuthMethods #16077

jbeverly opened this issue Jun 15, 2016 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@jbeverly
Copy link

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    1.6.1
  2. What operating system and processor architecture are you using (go env)?
    GOARCH="amd64"
    GOBIN=""
    GOEXE=""
    GOHOSTARCH="amd64"
    GOHOSTOS="linux"
    GOOS="linux"
    GOPATH="/go"
    GORACE=""
    GOROOT="/usr/local/go"
    GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
    GO15VENDOREXPERIMENT="1"
    CC="gcc"
    GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
    CXX="g++"
    CGO_ENABLED="1"
  3. What did you do?
    Mistyped password when using ssh.Client
  4. What did you expect to see?
    A prompt to re-enter password
  5. What did you see instead?
    A failed authentication and immediate termination of the connection
@adg adg changed the title Allow retrying AuthMethods x/crypto/ssh: allow retrying AuthMethods Jun 15, 2016
@adg adg added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 15, 2016
@jbeverly
Copy link
Author

a proposed patch for what I would like: https://go-review.googlesource.com/#/c/24156/

@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jun 16, 2016
@hanwen
Copy link
Contributor

hanwen commented Jun 16, 2016

can you give more background? Is this something that openSSH normally supports? I think most servers use keyboard-interactive auth nowadays, which lets the server itself decide how many prompts it wants to do.

@jbeverly
Copy link
Author

jbeverly commented Jun 16, 2016

I'd be happy to provide more background.

To the first question, yes, this is something openssh supports. Particularly for password authentication, but also for keyboard interactive authentication (and could potentially for other auth methods; though it handles pubkey differently).

With keyboard-interactive authentication, a server expects the number of answers it receives to exactly match the number of prompts it sent. x/crypto/ssh already checks this itself ( https://github.com/golang/crypto/blob/master/ssh/client_auth.go#L419 ). So if a server sends 3 prompts, it expects 3 answers. If any of those answers are incorrect, it would fail that authentication attempt. In a typical keyboard-interactive scenario being used for "password-auth", the servers sends 1 question (e.g. "Password: ") and expects 1 answer. Normally, the server responds to circumstances where the answer is wrong by providing a list of authentications that can continue that still includes the method that just failed. The client can then choose any of the methods that can continue, and (re)attempt one of them.

You can see this behavior if you use the following snippet to connect to an sshd (openssh in my experiments). (with minor hardcoded tweaks of doom) https://play.golang.org/p/F-od_urg3G

The way x/crypto/ssh currently handles this is, as soon as a method is tried, it is immediately placed in the tried map, and never tried again. My patch allows this behavior in the default case (so as not to break any existing users of x/crypto/ssh), but allows a user of x/crypto/ssh to alternatively obtain the behavior clients like openssh/etc exhibit, where failed authentication methods are retryable if you typo, but can after some number of attempts (or other condition) stop allowing a particular method, even if the server lists it as a method that can continue (e.g. NumberOfPasswordPrompts). This enables a more familiar user-experience for human-interactive clients written using x/crypto/ssh.

The unit-tests in my patch are probably better at demonstrating the behavior than my explanation here, so I'll refer to them:
https://go-review.googlesource.com/#/c/24156/1/ssh/client_auth_test.go

The first test shows what happens in the case of a typo, and then successful entry. The second demonstrates the behavior when a client tries password-auth first (which would be silly, but is sometimes how people have things configured to behave) and finally falls back to publickey authentication after password auth fails twice.

Note also, this does not change the behavior of servers demanding multiple forms of authentication must succeed (sshd_config's AuthenticationMethods option). A server requiring multiple authentication methods exhibits the same behavior when the user makes a typo; either offering the client the opportunity to retry the method again by including it in the list of methods that may continue, or not, at the server's discretion.

Thanks again!

@hanwen
Copy link
Contributor

hanwen commented Jul 11, 2016

@hanwen hanwen closed this as completed Jul 11, 2016
@golang golang locked and limited conversation to collaborators Jul 11, 2017
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Nov 24, 2019
Adds a new AuthMethod called "RetryableAuthMethod" which decorates any
other authmethod, allowing it to be retried up to maxTries before
aborting.

Fixes golang#16077

Change-Id: Ie310c24643e53dca4fa452750a69936674906484
Reviewed-on: https://go-review.googlesource.com/24156
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants