From fbdad15bb78187fbf9262a6bb722e5531e2728af Mon Sep 17 00:00:00 2001 From: ibuler Date: Wed, 8 May 2019 19:35:15 +0800 Subject: [PATCH 1/5] [Update] Support for ssh server paratial success, and try next auth methods --- ssh/server.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/ssh/server.go b/ssh/server.go index e86e89661a..086c4ff38e 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -85,6 +85,11 @@ type ServerConfig struct { // unknown. KeyboardInteractiveCallback func(conn ConnMetadata, client KeyboardInteractiveChallenge) (*Permissions, error) + // NextAuthMethodsCallback if non-nil, is called when auth methods + // return PartialSuccess error, then these methods will be + // methods auth can continue + NextAuthMethodsCallback func(conn ConnMetadata) []string + // AuthLogCallback, if non-nil, is called to log all authentication // attempts. AuthLogCallback func(conn ConnMetadata, method string, err error) @@ -320,6 +325,11 @@ func (l ServerAuthError) Error() string { // It is returned in ServerAuthError.Errors from NewServerConn. var ErrNoAuth = errors.New("ssh: no auth passed yet") +// ErrPartialSuccess is the error some auth method partially successful +// but need more auth method confirm, If return, then config.NextAuthMethodsCallback +// will be call, and check it +var ErrPartialSuccess = errors.New("authenticated with partial success") + func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, error) { sessionID := s.transport.getSessionID() var cache pubKeyCache @@ -513,13 +523,15 @@ userAuthLoop: authFailures++ var failureMsg userAuthFailureMsg - if config.PasswordCallback != nil { + + nextAuthMethods := strings.Join(config.NextAuthMethodsCallback(s), ",") + if config.PasswordCallback != nil && !strings.Contains(nextAuthMethods, "password") { failureMsg.Methods = append(failureMsg.Methods, "password") } - if config.PublicKeyCallback != nil { + if config.PublicKeyCallback != nil && !strings.Contains(nextAuthMethods, "publickey") { failureMsg.Methods = append(failureMsg.Methods, "publickey") } - if config.KeyboardInteractiveCallback != nil { + if config.KeyboardInteractiveCallback != nil && !strings.Contains(nextAuthMethods, "keyboard-interactive") { failureMsg.Methods = append(failureMsg.Methods, "keyboard-interactive") } @@ -527,6 +539,11 @@ userAuthLoop: return nil, errors.New("ssh: no authentication methods configured but NoClientAuth is also false") } + if authErr == ErrPartialSuccess { + failureMsg.PartialSuccess = true + failureMsg.Methods = config.NextAuthMethodsCallback(s) + } + if err := s.transport.writePacket(Marshal(&failureMsg)); err != nil { return nil, err } From 40738d426814df40c4d54d5e7017f7af7725da47 Mon Sep 17 00:00:00 2001 From: ibuler Date: Wed, 8 May 2019 19:51:01 +0800 Subject: [PATCH 2/5] [Update] Avoid call NextAuthMethodsCallback many times --- ssh/server.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ssh/server.go b/ssh/server.go index 086c4ff38e..4c8c60d470 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -338,6 +338,7 @@ func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, err authFailures := 0 var authErrs []error var displayedBanner bool + var nextAuthMethods []string userAuthLoop: for { @@ -524,14 +525,17 @@ userAuthLoop: var failureMsg userAuthFailureMsg - nextAuthMethods := strings.Join(config.NextAuthMethodsCallback(s), ",") - if config.PasswordCallback != nil && !strings.Contains(nextAuthMethods, "password") { + if len(nextAuthMethods) == 0 && config.NextAuthMethodsCallback != nil { + nextAuthMethods = config.NextAuthMethodsCallback(s) + } + authMethods := strings.Join(nextAuthMethods, ",") + if config.PasswordCallback != nil && !strings.Contains(authMethods, "password") { failureMsg.Methods = append(failureMsg.Methods, "password") } - if config.PublicKeyCallback != nil && !strings.Contains(nextAuthMethods, "publickey") { + if config.PublicKeyCallback != nil && !strings.Contains(authMethods, "publickey") { failureMsg.Methods = append(failureMsg.Methods, "publickey") } - if config.KeyboardInteractiveCallback != nil && !strings.Contains(nextAuthMethods, "keyboard-interactive") { + if config.KeyboardInteractiveCallback != nil && !strings.Contains(authMethods, "keyboard-interactive") { failureMsg.Methods = append(failureMsg.Methods, "keyboard-interactive") } @@ -539,9 +543,9 @@ userAuthLoop: return nil, errors.New("ssh: no authentication methods configured but NoClientAuth is also false") } - if authErr == ErrPartialSuccess { + if authErr == ErrPartialSuccess && len(nextAuthMethods) > 0 { failureMsg.PartialSuccess = true - failureMsg.Methods = config.NextAuthMethodsCallback(s) + failureMsg.Methods = nextAuthMethods } if err := s.transport.writePacket(Marshal(&failureMsg)); err != nil { From ef0d1a6f5b87067803518089d4cbc349777a56bd Mon Sep 17 00:00:00 2001 From: ibuler Date: Thu, 9 May 2019 14:12:40 +0800 Subject: [PATCH 3/5] [Bugfix] fix client only request the next auth methods --- ssh/server.go | 27 ++- ssh/test/next_auth_methods_test.go | 277 +++++++++++++++++++++++++++++ 2 files changed, 297 insertions(+), 7 deletions(-) create mode 100644 ssh/test/next_auth_methods_test.go diff --git a/ssh/server.go b/ssh/server.go index 4c8c60d470..41c4c08a28 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -338,7 +338,9 @@ func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, err authFailures := 0 var authErrs []error var displayedBanner bool + var firstAuthOk bool var nextAuthMethods []string + var nextAuthMethodsPlain string userAuthLoop: for { @@ -386,6 +388,16 @@ userAuthLoop: perms = nil authErr := ErrNoAuth + // get next auth methods + if len(nextAuthMethods) == 0 && config.NextAuthMethodsCallback != nil { + nextAuthMethods = config.NextAuthMethodsCallback(s) + nextAuthMethodsPlain = strings.Join(nextAuthMethods, ",") + } + // If user request auth method in next auth method, should be deny + if strings.Contains(nextAuthMethodsPlain, userAuthReq.Method) && !firstAuthOk { + authErr = errors.New(fmt.Sprintf("ssh: %v auth should be first auth success", nextAuthMethods)) + break + } switch userAuthReq.Method { case "none": @@ -525,17 +537,16 @@ userAuthLoop: var failureMsg userAuthFailureMsg - if len(nextAuthMethods) == 0 && config.NextAuthMethodsCallback != nil { - nextAuthMethods = config.NextAuthMethodsCallback(s) - } - authMethods := strings.Join(nextAuthMethods, ",") - if config.PasswordCallback != nil && !strings.Contains(authMethods, "password") { + // if password in next auth methods, should not be tell client + if config.PasswordCallback != nil && !strings.Contains(nextAuthMethodsPlain, "password") { failureMsg.Methods = append(failureMsg.Methods, "password") } - if config.PublicKeyCallback != nil && !strings.Contains(authMethods, "publickey") { + // if publickey in next auth methods, should not be tell client + if config.PublicKeyCallback != nil && !strings.Contains(nextAuthMethodsPlain, "publickey") { failureMsg.Methods = append(failureMsg.Methods, "publickey") } - if config.KeyboardInteractiveCallback != nil && !strings.Contains(authMethods, "keyboard-interactive") { + // if keyboard-interactive in next auth methods, should not be tell client + if config.KeyboardInteractiveCallback != nil && !strings.Contains(nextAuthMethodsPlain, "keyboard-interactive") { failureMsg.Methods = append(failureMsg.Methods, "keyboard-interactive") } @@ -543,7 +554,9 @@ userAuthLoop: return nil, errors.New("ssh: no authentication methods configured but NoClientAuth is also false") } + // if auth error is partial success, so need next auth if authErr == ErrPartialSuccess && len(nextAuthMethods) > 0 { + firstAuthOk = true failureMsg.PartialSuccess = true failureMsg.Methods = nextAuthMethods } diff --git a/ssh/test/next_auth_methods_test.go b/ssh/test/next_auth_methods_test.go new file mode 100644 index 0000000000..91fd9d4683 --- /dev/null +++ b/ssh/test/next_auth_methods_test.go @@ -0,0 +1,277 @@ +package test + +import ( + "crypto/rand" + "crypto/rsa" + "fmt" + "log" + "net" + "testing" + "errors" + + "golang.org/x/crypto/ssh" +) + +func generateSigner() (ssh.Signer, error) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, err + } + return ssh.NewSignerFromKey(key) +} + +func handleChannels(chans <-chan ssh.NewChannel) { + // Service the incoming Channel channel in go routine + for newChannel := range chans { + go handleChannel(newChannel) + } +} + +func handleChannel(newChan ssh.NewChannel) { + // Since we're handling a shell, we expect a + // channel type of "session". The also describes + // "x11", "direct-tcpip" and "forwarded-tcpip" + // channel types. + if t := newChan.ChannelType(); t != "session" { + newChan.Reject(ssh.UnknownChannelType, fmt.Sprintf("unknown channel type: %s", t)) + return + } + + // At this point, we have the opportunity to reject the client's + // request for another logical connection + ch, reqs, err := newChan.Accept() + if err != nil { + return + } + for req := range reqs { + switch req.Type { + case "shell", "exec": + go func(ch ssh.Channel) { + ch.Write([]byte("Hello world \r\n")) + ch.Close() + }(ch) + } + } +} + +func runDemoServer(conf *ssh.ServerConfig) net.Listener { + listener, err := net.Listen("tcp", "0.0.0.0:2200") + if err != nil { + log.Fatal("Failed to listen on 2200", err) + } + log.Print("Listening on :2200") + go func() { + for { + tcpConn, err := listener.Accept() + if err != nil { + if _, ok := err.(net.Error); ok{ + return + } + log.Printf("Failed to accept incoming connection (%s)", err) + continue + } + // Before use, a handshake must be performed on the incoming net.Conn. + sshConn, chans, reqs, err := ssh.NewServerConn(tcpConn, conf) + if err != nil { + log.Printf("Failed to handshake (%s)", err) + continue + } + + log.Printf("New SSH connection from %s (%s)", sshConn.RemoteAddr(), sshConn.ClientVersion()) + // Discard all global out-of-band Requests + go ssh.DiscardRequests(reqs) + // Accept all channels + go handleChannels(chans) + } + }() + return listener +} + +func originPasswordCallback(conn ssh.ConnMetadata, password []byte) (permissions *ssh.Permissions, e error) { + if conn.User() == "foo" && string(password) == "bar" { + return &ssh.Permissions{}, nil + } + e = errors.New("password not match") + return +} + +func originKeyboardInteractiveCallback(conn ssh.ConnMetadata, client ssh.KeyboardInteractiveChallenge) (permissions *ssh.Permissions, e error) { + answers, e := client("foo", "Write MFA code ", []string{"> "}, []bool{true}) + if e != nil { + return + } + if len(answers) != 1 { + e = errors.New("no mfa input") + return + } + if answers[0] != "123456" { + e = errors.New("mfa code not match") + } + return +} + + +func TestGeneralRun(t *testing.T) { + signer, err := generateSigner() + if err != nil { + t.Error("Generate signer error", err) + } + serverConf := &ssh.ServerConfig{ + PasswordCallback: originPasswordCallback, + KeyboardInteractiveCallback: originKeyboardInteractiveCallback, + } + serverConf.AddHostKey(signer) + l := runDemoServer(serverConf) + defer l.Close() + + var authMethods []ssh.AuthMethod + var clientConfig *ssh.ClientConfig + var client *ssh.Client + + // Error password + authMethods = []ssh.AuthMethod{ssh.Password("bar123")} + clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} + client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + if err == nil { + t.Error("Password not right, but connected") + return + } + + // Right password + authMethods = []ssh.AuthMethod{ssh.Password("bar")} + clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} + client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + if err != nil { + t.Error("General password connect ssh server error: ", err) + return + } + client.Close() + + // Error keyboard interactive code response + keyboardInteractiveChallenge := func(user, instruction string, questions []string, echos []bool, ) (answers []string, err error) { + if len(questions) == 0 { + return []string{}, nil + } + return []string{"12345678"}, nil + } + authMethods = []ssh.AuthMethod{ssh.KeyboardInteractive(keyboardInteractiveChallenge)} + clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} + client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + if err == nil { + t.Error("Interactive code not right, but connected") + return + } + + // Right keyboard interactive code response + keyboardInteractiveChallenge = func(user, instruction string, questions []string, echos []bool, ) (answers []string, err error) { + if len(questions) == 0 { + return []string{}, nil + } + return []string{"123456"}, nil + } + authMethods = []ssh.AuthMethod{ssh.KeyboardInteractive(keyboardInteractiveChallenge)} + clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} + client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + if err != nil { + t.Error("General keyboard interactive connect ssh server error: ", err) + return + } + client.Close() +} + +func nextPasswordCallback(conn ssh.ConnMetadata, password []byte) (permissions *ssh.Permissions, e error) { + if conn.User() == "foo" && string(password) == "bar" { + return &ssh.Permissions{}, ssh.ErrPartialSuccess + } + e = errors.New("password not match") + return +} + +func TestNextAuthMethods(t *testing.T) { + signer, err := generateSigner() + if err != nil { + t.Error("Generate signer error", err) + } + serverConf := &ssh.ServerConfig{ + PasswordCallback: nextPasswordCallback, + KeyboardInteractiveCallback: originKeyboardInteractiveCallback, + NextAuthMethodsCallback: func(conn ssh.ConnMetadata) []string { + return []string{"keyboard-interactive"} + }, + } + serverConf.AddHostKey(signer) + l := runDemoServer(serverConf) + defer l.Close() + + var authMethods []ssh.AuthMethod + var clientConfig *ssh.ClientConfig + var client *ssh.Client + + // Error password + authMethods = []ssh.AuthMethod{ssh.Password("bar123")} + clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} + client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + if err == nil { + t.Error("Password not right, but connected") + return + } + + // Right password, but should not login success + authMethods = []ssh.AuthMethod{ssh.Password("bar")} + clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} + client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + if err == nil { + t.Error("Password right but should not connect, because we set partial success ") + return + } + + // Error keyboard interactive code response + keyboardInteractiveChallenge := func(user, instruction string, questions []string, echos []bool, ) (answers []string, err error) { + if len(questions) == 0 { + return []string{}, nil + } + return []string{"12345678"}, nil + } + authMethods = []ssh.AuthMethod{ssh.KeyboardInteractive(keyboardInteractiveChallenge)} + clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} + client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + if err == nil { + t.Error("Interactive code not right, but connected") + return + } + + // Right keyboard interactive code response, but should login success + keyboardInteractiveChallenge = func(user, instruction string, questions []string, echos []bool, ) (answers []string, err error) { + if len(questions) == 0 { + return []string{}, nil + } + return []string{"123456"}, nil + } + authMethods = []ssh.AuthMethod{ssh.KeyboardInteractive(keyboardInteractiveChallenge)} + clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} + client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + if err == nil { + t.Error("Interactive code right but should not connect, because interactive code in next methods", err) + return + } + + // Right password and with right keyboard interactive code + keyboardInteractiveChallenge = func(user, instruction string, questions []string, echos []bool, ) (answers []string, err error) { + if len(questions) == 0 { + return []string{}, nil + } + return []string{"123456"}, nil + } + authMethods = []ssh.AuthMethod{ssh.KeyboardInteractive(keyboardInteractiveChallenge), ssh.Password("bar")} + clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} + client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + if err != nil { + t.Error("Password and interactive code right but connect error", err) + return + } + client.Close() +} + + + + From a7099eef26a7fdc97f3ac5f5b2b61f9f136dd16f Mon Sep 17 00:00:00 2001 From: ibuler Date: Thu, 9 May 2019 18:12:00 +0800 Subject: [PATCH 4/5] [Update] Remove unreachable code --- ssh/server.go | 19 ++++++++----------- ssh/test/next_auth_methods_test.go | 12 ++++++------ 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/ssh/server.go b/ssh/server.go index 41c4c08a28..afc11bb2d9 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -338,7 +338,6 @@ func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, err authFailures := 0 var authErrs []error var displayedBanner bool - var firstAuthOk bool var nextAuthMethods []string var nextAuthMethodsPlain string @@ -389,15 +388,10 @@ userAuthLoop: perms = nil authErr := ErrNoAuth // get next auth methods - if len(nextAuthMethods) == 0 && config.NextAuthMethodsCallback != nil { + if config.NextAuthMethodsCallback != nil && len(nextAuthMethods) == 0 { nextAuthMethods = config.NextAuthMethodsCallback(s) nextAuthMethodsPlain = strings.Join(nextAuthMethods, ",") } - // If user request auth method in next auth method, should be deny - if strings.Contains(nextAuthMethodsPlain, userAuthReq.Method) && !firstAuthOk { - authErr = errors.New(fmt.Sprintf("ssh: %v auth should be first auth success", nextAuthMethods)) - break - } switch userAuthReq.Method { case "none": @@ -555,10 +549,13 @@ userAuthLoop: } // if auth error is partial success, so need next auth - if authErr == ErrPartialSuccess && len(nextAuthMethods) > 0 { - firstAuthOk = true - failureMsg.PartialSuccess = true - failureMsg.Methods = nextAuthMethods + if authErr == ErrPartialSuccess { + if len(nextAuthMethods) > 0 { + failureMsg.PartialSuccess = true + failureMsg.Methods = nextAuthMethods + } else { + return nil, errors.New("ssh: no next authentication methods configured but first auth return partial success") + } } if err := s.transport.writePacket(Marshal(&failureMsg)); err != nil { diff --git a/ssh/test/next_auth_methods_test.go b/ssh/test/next_auth_methods_test.go index 91fd9d4683..d73cba2942 100644 --- a/ssh/test/next_auth_methods_test.go +++ b/ssh/test/next_auth_methods_test.go @@ -3,13 +3,12 @@ package test import ( "crypto/rand" "crypto/rsa" + "errors" "fmt" + "golang.org/x/crypto/ssh" "log" "net" "testing" - "errors" - - "golang.org/x/crypto/ssh" ) func generateSigner() (ssh.Signer, error) { @@ -206,6 +205,7 @@ func TestNextAuthMethods(t *testing.T) { var authMethods []ssh.AuthMethod var clientConfig *ssh.ClientConfig var client *ssh.Client + var keyboardInteractiveChallenge ssh.KeyboardInteractiveChallenge // Error password authMethods = []ssh.AuthMethod{ssh.Password("bar123")} @@ -226,7 +226,7 @@ func TestNextAuthMethods(t *testing.T) { } // Error keyboard interactive code response - keyboardInteractiveChallenge := func(user, instruction string, questions []string, echos []bool, ) (answers []string, err error) { + keyboardInteractiveChallenge = func(user, instruction string, questions []string, echos []bool, ) (answers []string, err error) { if len(questions) == 0 { return []string{}, nil } @@ -240,7 +240,7 @@ func TestNextAuthMethods(t *testing.T) { return } - // Right keyboard interactive code response, but should login success + // Right keyboard interactive code response, but should not login success keyboardInteractiveChallenge = func(user, instruction string, questions []string, echos []bool, ) (answers []string, err error) { if len(questions) == 0 { return []string{}, nil @@ -249,7 +249,7 @@ func TestNextAuthMethods(t *testing.T) { } authMethods = []ssh.AuthMethod{ssh.KeyboardInteractive(keyboardInteractiveChallenge)} clientConfig = &ssh.ClientConfig{User: "foo", Auth:authMethods, HostKeyCallback: ssh.InsecureIgnoreHostKey()} - client, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) + _, err = ssh.Dial("tcp", net.JoinHostPort("127.0.0.1", "2200"), clientConfig) if err == nil { t.Error("Interactive code right but should not connect, because interactive code in next methods", err) return From 8ce6a60537c033a923aa17c0b2cb87ac62cd8327 Mon Sep 17 00:00:00 2001 From: Eric Date: Mon, 15 Jul 2019 17:07:07 +0800 Subject: [PATCH 5/5] retry next Auth when auth failed --- ssh/server.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ssh/server.go b/ssh/server.go index afc11bb2d9..c66583e0ab 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -340,6 +340,7 @@ func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, err var displayedBanner bool var nextAuthMethods []string var nextAuthMethodsPlain string + var nextAuthLoaded bool userAuthLoop: for { @@ -547,13 +548,17 @@ userAuthLoop: if len(failureMsg.Methods) == 0 { return nil, errors.New("ssh: no authentication methods configured but NoClientAuth is also false") } + if nextAuthLoaded { + failureMsg.Methods = nextAuthMethods + } // if auth error is partial success, so need next auth if authErr == ErrPartialSuccess { if len(nextAuthMethods) > 0 { + nextAuthLoaded = true failureMsg.PartialSuccess = true failureMsg.Methods = nextAuthMethods - } else { + } else { return nil, errors.New("ssh: no next authentication methods configured but first auth return partial success") } }