ssh: add ssh partial success for ssh server 2FA #89

ibuler · 2019-05-08T12:17:25Z

Adds ssh partial success error, and next auth methods call back, When auth methods call back return ErrPartialSuccess, then let client continue use nextAuthMethodsCallback return methods auth farther

I don't know if match RFC or not, Please check

#golang/go#17889
#golang/go#17934

…ethods

googlebot · 2019-05-08T12:17:29Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

ibuler · 2019-05-08T12:19:36Z

I signed it!

googlebot · 2019-05-08T12:19:38Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

gopherbot · 2019-05-08T12:35:02Z

This PR (HEAD: 40738d4) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/175978 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2019-05-08T12:36:11Z

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

Please don’t reply on this GitHub thread. Visit golang.org/cl/175978.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2019-05-08T14:05:56Z

Message from Benny Siegert:

Patch Set 1: Run-TryBot+1

(3 comments)

Can you please add a test?

Please don’t reply on this GitHub thread. Visit golang.org/cl/175978.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2019-05-08T14:05:57Z

Message from Gobot Gobot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=19bc885e

Please don’t reply on this GitHub thread. Visit golang.org/cl/175978.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2019-05-08T14:11:10Z

Message from Gobot Gobot:

Patch Set 1: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/175978.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2019-05-09T06:13:08Z

This PR (HEAD: ef0d1a6) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/175978 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

ibuler · 2019-05-09T06:41:59Z

Test case added

gopherbot · 2019-05-09T10:12:27Z

This PR (HEAD: a7099ee) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/175978 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2019-05-13T16:23:48Z

Message from Filippo Valsorda:

Patch Set 3:

#17889 should be turned into a proposal.

Please don’t reply on this GitHub thread. Visit golang.org/cl/175978.
After addressing review feedback, remember to publish your drafts!

retry next Auth when auth failed

googlebot · 2019-07-15T09:26:54Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

gopherbot · 2019-07-15T09:27:34Z

This PR (HEAD: 911d13b) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/175978 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

yanshuf0 · 2019-07-18T02:02:20Z

Really wish we could use this...

zachfi · 2019-09-13T04:28:10Z

I too want for something to help with 2fa.

liuzheng · 2019-09-30T22:36:17Z

Here is the way to include 2FA or any other questions you want
source code you can follow this project https://github.com/kennylevinsen/sshmux/blob/master/forward.go#L134

// SessionForward performs a regular forward, providing the user with an
// interactive remote host selection if necessary. This forwarding type
// requires agent forwarding in order to work.
func (s *Server) SessionForward(S *session.Session, newChannel ssh.NewChannel, chans <-chan ssh.NewChannel) {

	// Okay, we're handling this as a regular session
	sshChan, sessReqs, err := newChannel.Accept()
	if err != nil {
		log.Error(LogFilter, "%v", err)
		return
	}

	stderr := sshChan.Stderr()
	term := terminal.NewTerminal(sshChan, "")
	check := false
	for _, q := range s.Questions {
		count := 0
		if S.GetSeed() == "" {
			continue
		}
		for count < q.Retry {
			count++
			term.Write([]byte(q.Question))
			line, err := term.ReadLine()
			if err != nil {
				log.Error("term.ReadLine: %v", err)
				sshChan.Close()
				return
			}
			if q.Checker(line, term) {
				check = true
				break
			}
		}
		if !check {
			sshChan.Close()
			return
		}
	}
...

create a []Question struct list under Server struct to follow all your questions

adampointer-form3 · 2019-10-09T10:05:09Z

I have found that the solution that uses NextAuthMethodsCallback does not work completely.

In the scenario where we have PublicKey followed by Password auth, there is a bug caused by the fact that the client calls the server twice, firstly to pre-auth the key and then secondly to actually auth. The response from candidate.perms, candidate.result = config.PublicKeyCallback(s, pubKey) is cached so the same result is used both for the pre-auth and for the auth. This breaks if you return ErrPartialSuccess as it fails the pre-auth stage and does not allow the auth chain to continue. And due to the caching you can not discriminate in your callback whether to send a nil error or a PartialSuccess.

I worked around it with this. But I don't know if it is the ideal solution:

--- a/vendor/golang.org/x/crypto/ssh/server.go
+++ b/vendor/golang.org/x/crypto/ssh/server.go
@@ -480,7 +480,7 @@ userAuthLoop:
                                        return nil, parseError(msgUserAuthRequest)
                                }
 
-                               if candidate.result == nil {
+                               if candidate.result == nil || candidate.result == ErrPartialSuccess {
                                        okMsg := userAuthPubKeyOkMsg{
                                                Algo:   algo,
                                                PubKey: pubKeyData,

LeeEirc · 2019-10-14T04:59:14Z

@adampointer-form3 The ssh client will try the methods listed by the server, but if you configure NextAuthMethodsCallback as "publickey", then the client will not try "publickey" method until the client get ErrPartialSuccess err. It will not trigger publickey method cached problem.

slifty · 2022-08-30T01:12:49Z

It has been quite some time, and this functionality is sorely needed -- any chance @LeeEirc could sign the CLA since that seems to be the hangup!

slifty · 2022-08-31T15:34:39Z

Apologies for two posts but a related question: if this PR is forever dead due to CLA, what is the appropriate next step? Would it be useful for someone else to open a separate PR with @ibuler's code but removing the commit from @LeeEirc? Lee's code makes a fix but, for instance, I could make the same type of fix and sign the CLA to unstick it.

ibuler · 2022-09-05T03:45:45Z

I signed it!

slifty · 2022-09-06T14:21:01Z

@ibuler I think the issue is that in your PR you accepted a commit from a third party (@LeeEirc ) and that means they have to sign it to. If you know them maybe you could give a nudge?

Otherwise (and I am not a lawyer) I imagine one thing you could do is rebase your PR to remove his commit, and potentially re-implement their portion of the overall PR in your own way?

kfogel · 2022-09-12T18:52:27Z

Hi, @ibuler. I think what @slifty says above is accurate -- the issue is the inclusion of @LeeEirc's commit in your PR, because it means that now @LeeEirc would also be a contributor and would need to sign the CLA.

So our options are:

Hope that @LeeEirc sees this and signs the CLA too, or
Remove his commit and replace it with one by you that does basically the same thing. That is actually probably the easiest solution here -- @LeeEirc's commit 8ce6a60 is just a tiny number of lines of difference, and even some of those are whitespace changes that you can just leave out. (IMHO it's questionable whether commit 8ce6a60 is substantial enough to even be subject to copyright licensing at all, really -- but we can't have that discussion with the CLA bot, so it's easiest just to replace the commit.)

tt · 2022-09-12T20:49:04Z

This pull request does not work with public-key authentication as pointed out by @adampointer-form3. There should probably be a test to validate that.

The current implementation only allows public-key authentication as the last method. If you try to return ssh.PartialSuccess, it will fail. This is evident in the Go client code where the PartialSuccess field is not checked in the key query request:

crypto/ssh/client_auth.go

Lines 378 to 379 in c86fa9a

    
           case msgUserAuthFailure: 
        
           	return false, nil

There are (at least) two other implementations that don't have this issue, #130 and #214. I like the proposed interface of the latter to avoid passing strings around.

ibuler added 2 commits May 8, 2019 19:35

[Update] Support for ssh server paratial success, and try next auth m…

fbdad15

…ethods

[Update] Avoid call NextAuthMethodsCallback many times

40738d4

googlebot added the cla: no label May 8, 2019

googlebot added cla: yes and removed cla: no labels May 8, 2019

[Bugfix] fix client only request the next auth methods

ef0d1a6

[Update] Remove unreachable code

a7099ee

LeeEirc and others added 2 commits July 15, 2019 17:07

retry next Auth when auth failed

8ce6a60

Merge pull request #1 from LeeEirc/master

911d13b

retry next Auth when auth failed

googlebot added cla: no and removed cla: yes labels Jul 15, 2019

liuzheng mentioned this pull request Sep 30, 2019

x/crypto/ssh: add support for multi-step authentication golang/go#17889

Closed

ncw mentioned this pull request May 19, 2022

SFTP: Failes to connect to OpenSSH using ChallengeResponseAuthentication rclone/rclone#6181

Open

appleboy mentioned this pull request Dec 6, 2024

Feature request: Support TOTP SSH 2FA authentication appleboy/ssh-action#314

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ssh: add ssh partial success for ssh server 2FA #89

ssh: add ssh partial success for ssh server 2FA #89

ibuler commented May 8, 2019

googlebot commented May 8, 2019

ibuler commented May 8, 2019

googlebot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 9, 2019

ibuler commented May 9, 2019

gopherbot commented May 9, 2019

gopherbot commented May 13, 2019

googlebot commented Jul 15, 2019

gopherbot commented Jul 15, 2019

yanshuf0 commented Jul 18, 2019

zachfi commented Sep 13, 2019

liuzheng commented Sep 30, 2019 •

edited

Loading

adampointer-form3 commented Oct 9, 2019 •

edited

Loading

LeeEirc commented Oct 14, 2019

slifty commented Aug 30, 2022

slifty commented Aug 31, 2022 •

edited

Loading

ibuler commented Sep 5, 2022

slifty commented Sep 6, 2022 •

edited

Loading

kfogel commented Sep 12, 2022

tt commented Sep 12, 2022

ssh: add ssh partial success for ssh server 2FA #89

Are you sure you want to change the base?

ssh: add ssh partial success for ssh server 2FA #89

Conversation

ibuler commented May 8, 2019

googlebot commented May 8, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

ibuler commented May 8, 2019

googlebot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 8, 2019

gopherbot commented May 9, 2019

ibuler commented May 9, 2019

gopherbot commented May 9, 2019

gopherbot commented May 13, 2019

googlebot commented Jul 15, 2019

gopherbot commented Jul 15, 2019

yanshuf0 commented Jul 18, 2019

zachfi commented Sep 13, 2019

liuzheng commented Sep 30, 2019 • edited Loading

adampointer-form3 commented Oct 9, 2019 • edited Loading

LeeEirc commented Oct 14, 2019

slifty commented Aug 30, 2022

slifty commented Aug 31, 2022 • edited Loading

ibuler commented Sep 5, 2022

slifty commented Sep 6, 2022 • edited Loading

kfogel commented Sep 12, 2022

tt commented Sep 12, 2022

liuzheng commented Sep 30, 2019 •

edited

Loading

adampointer-form3 commented Oct 9, 2019 •

edited

Loading

slifty commented Aug 31, 2022 •

edited

Loading

slifty commented Sep 6, 2022 •

edited

Loading