ssh: fix support for partial success authentication responses in client

The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.

This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
   returns  when underlying auth method fails with partial success
   set to true.
2) Book keeping of tried (and failed) auth methods in
   clientAuthenticate() does not mark an auth method failed if it
   fails with partial success set to true.

Fixes golang/go#23461

Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
Reviewed-on: https://go-review.googlesource.com/88035
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
This commit is contained in:
Sami Pönkänen
2018-02-08 15:07:01 +00:00
committed by Han-Wen Nienhuys
parent d9133f5469
commit 9334d73e5f
5 changed files with 450 additions and 57 deletions
+55 -40
View File
@@ -11,6 +11,14 @@ import (
"io"
)
type authResult int
const (
authFailure authResult = iota
authPartialSuccess
authSuccess
)
// clientAuthenticate authenticates with the remote server. See RFC 4252.
func (c *connection) clientAuthenticate(config *ClientConfig) error {
// initiate user auth session
@@ -37,11 +45,12 @@ func (c *connection) clientAuthenticate(config *ClientConfig) error {
if err != nil {
return err
}
if ok {
if ok == authSuccess {
// success
return nil
} else if ok == authFailure {
tried[auth.method()] = true
}
tried[auth.method()] = true
if methods == nil {
methods = lastMethods
}
@@ -82,7 +91,7 @@ type AuthMethod interface {
// If authentication is not successful, a []string of alternative
// method names is returned. If the slice is nil, it will be ignored
// and the previous set of possible methods will be reused.
auth(session []byte, user string, p packetConn, rand io.Reader) (bool, []string, error)
auth(session []byte, user string, p packetConn, rand io.Reader) (authResult, []string, error)
// method returns the RFC 4252 method name.
method() string
@@ -91,13 +100,13 @@ type AuthMethod interface {
// "none" authentication, RFC 4252 section 5.2.
type noneAuth int
func (n *noneAuth) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
func (n *noneAuth) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
if err := c.writePacket(Marshal(&userAuthRequestMsg{
User: user,
Service: serviceSSH,
Method: "none",
})); err != nil {
return false, nil, err
return authFailure, nil, err
}
return handleAuthResponse(c)
@@ -111,7 +120,7 @@ func (n *noneAuth) method() string {
// a function call, e.g. by prompting the user.
type passwordCallback func() (password string, err error)
func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
type passwordAuthMsg struct {
User string `sshtype:"50"`
Service string
@@ -125,7 +134,7 @@ func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand
// The program may only find out that the user doesn't have a password
// when prompting.
if err != nil {
return false, nil, err
return authFailure, nil, err
}
if err := c.writePacket(Marshal(&passwordAuthMsg{
@@ -135,7 +144,7 @@ func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand
Reply: false,
Password: pw,
})); err != nil {
return false, nil, err
return authFailure, nil, err
}
return handleAuthResponse(c)
@@ -178,7 +187,7 @@ func (cb publicKeyCallback) method() string {
return "publickey"
}
func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
// Authentication is performed by sending an enquiry to test if a key is
// acceptable to the remote. If the key is acceptable, the client will
// attempt to authenticate with the valid key. If not the client will repeat
@@ -186,13 +195,13 @@ func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand
signers, err := cb()
if err != nil {
return false, nil, err
return authFailure, nil, err
}
var methods []string
for _, signer := range signers {
ok, err := validateKey(signer.PublicKey(), user, c)
if err != nil {
return false, nil, err
return authFailure, nil, err
}
if !ok {
continue
@@ -206,7 +215,7 @@ func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand
Method: cb.method(),
}, []byte(pub.Type()), pubKey))
if err != nil {
return false, nil, err
return authFailure, nil, err
}
// manually wrap the serialized signature in a string
@@ -224,24 +233,24 @@ func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand
}
p := Marshal(&msg)
if err := c.writePacket(p); err != nil {
return false, nil, err
return authFailure, nil, err
}
var success bool
var success authResult
success, methods, err = handleAuthResponse(c)
if err != nil {
return false, nil, err
return authFailure, nil, err
}
// If authentication succeeds or the list of available methods does not
// contain the "publickey" method, do not attempt to authenticate with any
// other keys. According to RFC 4252 Section 7, the latter can occur when
// additional authentication methods are required.
if success || !containsMethod(methods, cb.method()) {
if success == authSuccess || !containsMethod(methods, cb.method()) {
return success, methods, err
}
}
return false, methods, nil
return authFailure, methods, nil
}
func containsMethod(methods []string, method string) bool {
@@ -318,28 +327,31 @@ func PublicKeysCallback(getSigners func() (signers []Signer, err error)) AuthMet
// handleAuthResponse returns whether the preceding authentication request succeeded
// along with a list of remaining authentication methods to try next and
// an error if an unexpected response was received.
func handleAuthResponse(c packetConn) (bool, []string, error) {
func handleAuthResponse(c packetConn) (authResult, []string, error) {
for {
packet, err := c.readPacket()
if err != nil {
return false, nil, err
return authFailure, nil, err
}
switch packet[0] {
case msgUserAuthBanner:
if err := handleBannerResponse(c, packet); err != nil {
return false, nil, err
return authFailure, nil, err
}
case msgUserAuthFailure:
var msg userAuthFailureMsg
if err := Unmarshal(packet, &msg); err != nil {
return false, nil, err
return authFailure, nil, err
}
return false, msg.Methods, nil
if msg.PartialSuccess {
return authPartialSuccess, msg.Methods, nil
}
return authFailure, msg.Methods, nil
case msgUserAuthSuccess:
return true, nil, nil
return authSuccess, nil, nil
default:
return false, nil, unexpectedMessageError(msgUserAuthSuccess, packet[0])
return authFailure, nil, unexpectedMessageError(msgUserAuthSuccess, packet[0])
}
}
}
@@ -381,7 +393,7 @@ func (cb KeyboardInteractiveChallenge) method() string {
return "keyboard-interactive"
}
func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
type initiateMsg struct {
User string `sshtype:"50"`
Service string
@@ -395,20 +407,20 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
Service: serviceSSH,
Method: "keyboard-interactive",
})); err != nil {
return false, nil, err
return authFailure, nil, err
}
for {
packet, err := c.readPacket()
if err != nil {
return false, nil, err
return authFailure, nil, err
}
// like handleAuthResponse, but with less options.
switch packet[0] {
case msgUserAuthBanner:
if err := handleBannerResponse(c, packet); err != nil {
return false, nil, err
return authFailure, nil, err
}
continue
case msgUserAuthInfoRequest:
@@ -416,18 +428,21 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
case msgUserAuthFailure:
var msg userAuthFailureMsg
if err := Unmarshal(packet, &msg); err != nil {
return false, nil, err
return authFailure, nil, err
}
return false, msg.Methods, nil
if msg.PartialSuccess {
return authPartialSuccess, msg.Methods, nil
}
return authFailure, msg.Methods, nil
case msgUserAuthSuccess:
return true, nil, nil
return authSuccess, nil, nil
default:
return false, nil, unexpectedMessageError(msgUserAuthInfoRequest, packet[0])
return authFailure, nil, unexpectedMessageError(msgUserAuthInfoRequest, packet[0])
}
var msg userAuthInfoRequestMsg
if err := Unmarshal(packet, &msg); err != nil {
return false, nil, err
return authFailure, nil, err
}
// Manually unpack the prompt/echo pairs.
@@ -437,7 +452,7 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
for i := 0; i < int(msg.NumPrompts); i++ {
prompt, r, ok := parseString(rest)
if !ok || len(r) == 0 {
return false, nil, errors.New("ssh: prompt format error")
return authFailure, nil, errors.New("ssh: prompt format error")
}
prompts = append(prompts, string(prompt))
echos = append(echos, r[0] != 0)
@@ -445,16 +460,16 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
}
if len(rest) != 0 {
return false, nil, errors.New("ssh: extra data following keyboard-interactive pairs")
return authFailure, nil, errors.New("ssh: extra data following keyboard-interactive pairs")
}
answers, err := cb(msg.User, msg.Instruction, prompts, echos)
if err != nil {
return false, nil, err
return authFailure, nil, err
}
if len(answers) != len(prompts) {
return false, nil, errors.New("ssh: not enough answers from keyboard-interactive callback")
return authFailure, nil, errors.New("ssh: not enough answers from keyboard-interactive callback")
}
responseLength := 1 + 4
for _, a := range answers {
@@ -470,7 +485,7 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
}
if err := c.writePacket(serialized); err != nil {
return false, nil, err
return authFailure, nil, err
}
}
}
@@ -480,10 +495,10 @@ type retryableAuthMethod struct {
maxTries int
}
func (r *retryableAuthMethod) auth(session []byte, user string, c packetConn, rand io.Reader) (ok bool, methods []string, err error) {
func (r *retryableAuthMethod) auth(session []byte, user string, c packetConn, rand io.Reader) (ok authResult, methods []string, err error) {
for i := 0; r.maxTries <= 0 || i < r.maxTries; i++ {
ok, methods, err = r.authMethod.auth(session, user, c, rand)
if ok || err != nil { // either success or error terminate
if ok != authFailure || err != nil { // either success, partial success or error terminate
return ok, methods, err
}
}